mac Reporter
	Description • 9 years ago

User Agent: Mozilla/5.0 (X11; OpenBSD amd64; rv:35.0) Gecko/20100101 Firefox/35.0
Build ID: 20150307020558

Steps to reproduce:

1. Open SMS thread.
2. Type in message.
3. Press 'Send' button.


Actual results:

'There was a problem sending the message. Please try again' appeard. Not possible to send the message.


Expected results:

It should be possible to send messages.

	mac Reporter
	Comment 1 • 9 years ago

From logcat:

I/Gecko   ( 1119): InputContextDOMRequestIpcHelper received message without context attached.

Found on:

TCT Flame (got from Foxtrot Programme)
B2G version: 2.5.0.0-prerelease master
Firmware: v18D_nightly_v4
Platform version: 43.0a1
Build Identifier: 20150908030224
Git commit info: 2015-09-07 02:49:56 891798f1

	Steve Chung [:steveck]
	Comment 2 • 9 years ago

Requesting QA to verify this, although gaia side didn't have big update recently. Maybe Bevis will know something from gecko.

Hi mac, could you please try to enable the RIL output in ADB for more information from ADB log(in the settings -> developer menu)? Thanks!

	mac Reporter
	Comment 3 • 9 years ago

Here you are:

--------- beginning of /dev/log/main
I/Gecko   (  209): SmsService: Error! Fail to save sending message! aRv = 2147500037
I/Gecko   (  209): SmsService: Broadcasting the SMS system message: 3
I/Gecko   (  209): SmsService: Failed to _broadcastSmsSystemMessage: TypeError: aDomMessage is null
I/Gecko   ( 1207): InputContextDOMRequestIpcHelper received message without context attached.
I/Gecko   (  209): RIL Worker: Received 68 bytes.
I/Gecko   (  209): RIL Worker: Already read 0
I/Gecko   (  209): RIL Worker: New incoming parcel of size 64
I/Gecko   (  209): RIL Worker: Parcel (size 64): 1,0,0,0,241,3,0,0,10,0,0,0,0,0,0,0,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,99,0,0,0,255,255,255,127,255,255,255,127,255,255,255,127,255,255,255,127,255,255,255,127,255,255,255,127
I/Gecko   (  209): RIL Worker: We have at least one complete parcel.
I/Gecko   (  209): RIL Worker: [0] Handling parcel as UNSOLICITED_SIGNAL_STRENGTH
I/Gecko   (  209): RIL Worker: [0] Received signal network info.
I/Gecko   (  209): RIL Worker: [0] signal strength: {"gsmSignalStrength":10,"gsmBitErrorRate":0,"cdmaDBM":-1,"cdmaECIO":-1,"evdoDBM":-1,"evdoECIO":-1,"evdoSNR":-1,"lteSignalStrength":99,"lteRSRP":2147483647,"lteRSRQ":2147483647,"lteRSSNR":2147483647,"lteCQI":2147483647}
I/Gecko   (  209): -*- RadioInterface[0]: Received message from worker: {"voice":{"signalStrength":-93,"relSignalStrength":68},"data":{"signalStrength":-93,"relSignalStrength":68},"rilMessageType":"signalstrengthchange","rilMessageClientId":0}
I/Gecko   (  209): MobileConnectionService: notifySignalStrengthChanged for 0: {"voice":{"signalStrength":-93,"relSignalStrength":68},"data":{"signalStrength":-93,"relSignalStrength":68},"rilMessageType":"signalstrengthchange","rilMessageClientId":0}
I/Gecko   (  209): RIL Worker: Parcel handler didn't consume whole parcel, 8 bytes left over
I/Gecko   (  209): RIL Worker: Next parcel size unknown, going to sleep.
I/Gecko   (  209): RIL Worker: Received 68 bytes.
I/Gecko   (  209): RIL Worker: Already read 0
I/Gecko   (  209): RIL Worker: New incoming parcel of size 64
I/Gecko   (  209): RIL Worker: Parcel (size 64): 1,0,0,0,241,3,0,0,9,0,0,0,0,0,0,0,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,255,99,0,0,0,255,255,255,127,255,255,255,127,255,255,255,127,255,255,255,127,255,255,255,127,255,255,255,127
I/Gecko   (  209): RIL Worker: We have at least one complete parcel.
I/Gecko   (  209): RIL Worker: [0] Handling parcel as UNSOLICITED_SIGNAL_STRENGTH
I/Gecko   (  209): RIL Worker: [0] Received signal network info.
I/Gecko   (  209): RIL Worker: [0] signal strength: {"gsmSignalStrength":9,"gsmBitErrorRate":0,"cdmaDBM":-1,"cdmaECIO":-1,"evdoDBM":-1,"evdoECIO":-1,"evdoSNR":-1,"lteSignalStrength":99,"lteRSRP":2147483647,"lteRSRQ":2147483647,"lteRSSNR":2147483647,"lteCQI":2147483647}
I/Gecko   (  209): -*- RadioInterface[0]: Received message from worker: {"voice":{"signalStrength":-95,"relSignalStrength":60},"data":{"signalStrength":-95,"relSignalStrength":60},"rilMessageType":"signalstrengthchange","rilMessageClientId":0}
I/Gecko   (  209): MobileConnectionService: notifySignalStrengthChanged for 0: {"voice":{"signalStrength":-95,"relSignalStrength":60},"data":{"signalStrength":-95,"relSignalStrength":60},"rilMessageType":"signalstrengthchange","rilMessageClientId":0}
I/Gecko   (  209): RIL Worker: Parcel handler didn't consume whole parcel, 8 bytes left over
I/Gecko   (  209): RIL Worker: Next parcel size unknown, going to sleep.

	:gerard-majax
	Comment 4 • 9 years ago

09-09 16:22:40.151   320  2833 E GeckoConsole: [JavaScript Error: "IndexedDB UnknownErr: ActorsParent.cpp:895"]
09-09 16:22:40.151   320   902 E GeckoConsole: [JavaScript Error: "IndexedDB UnknownErr: ActorsParent.cpp:593"]
09-09 16:22:40.151   320   320 E GeckoConsole: [JavaScript Error: "UnknownError" {file: "resource://gre/modules/MobileMessageDB.jsm" line: 2910}]
09-09 16:22:40.161   320   320 E GeckoConsole: [JavaScript Error: "AbortError" {file: "resource://gre/modules/MobileMessageDB.jsm" line: 2866}]

Reproduced on Z3 Compact, and I got reports on nightly on Open C too.

	Johan Lorenzo [:jlorenzo]
	Comment 5 • 9 years ago

This failure wasn't caught by automation[1]. After checking with Alexandre, this occurs only with existing message threads. If he created a new thread to a new contact, there is no bug. That'd explain why automation didn't catch it.

STR
1. Send an SMS to somebody
2. OTA to the latest update
3. Send an SMS to the same person as step 1.

Regarding the errors thrown and the time where this bug has been found, this looks like a fallout of bug 1200004. Kyle, does it sound possible to you?

[Blocking Requested - why for this release]: This bug prevents a dogfooder from sending an SMS after the next OTA.

[1] http://jenkins1.qa.scl3.mozilla.com/view/UI/job/flame-kk-319.mozilla-central.nightly.ui.functional.smoke/427/testReport/

	Mahendra Potharaju [:mahe]
	Comment 7 • 9 years ago

Blocks 2.5. SMS functionality broken

	Naoki Hirata :nhirata (please use needinfo instead of cc)
	Comment 8 • 9 years ago

khuey is going on vacation, is there someone else we can ping?

	Johan Lorenzo [:jlorenzo]
	Comment 9 • 9 years ago

The patch just before was bug 871846. Maybe Reuben can help to diagnose the error shown in comment 4?

	Reuben Morais [:reuben] Assignee
	Comment 10 • 9 years ago

I don't have access to bug 1200004 so I can't tell if it's related, but bug 871846 definitely is (comment 4). The interaction with the OTA upgrade doesn't make a lot of sense to me, it looks like we'll have to backout bug 871846 for now.

	Reuben Morais [:reuben] Assignee
	Comment 11 • 9 years ago

Trees are closed right now, feel free to land this. Otherwise I'll do it when I get home tonight.

	Reuben Morais [:reuben] Assignee
	Comment 13 • 9 years ago

Jan, what do you think is the best way to handle backing out the version upgrade and the changes to MakeCompressedIndexDataValues and ReadCompressedIndexDataValuesFromBlob, in a way that doesn't break nightly users who have data in the new format? Should I write an upgrade specifically for the backout?

	Jan Varga [:janv]
	Comment 14 • 9 years ago

Uhm, can you explain what is actually failing and how long it would take to fix it ?
Backing out this patch is not trivial obviously.

	Johan Lorenzo [:jlorenzo]
	Comment 15 • 9 years ago

Forgot to clear my NI in comment 9.

	Reuben Morais [:reuben] Assignee
	Comment 16 • 9 years ago

(In reply to Jan Varga [:janv] from comment #14)
> Uhm, can you explain what is actually failing and how long it would take to
> fix it ?

We're trying to read sort keys from the compressed index data values, but after an OTA there can be data there in the old format. Basically, the 18 -> 19 version upgrade should have upgraded object_data, but didn't. We can't easily parse from the data layout whether it has sort keys or not:

Version 18: | index id | key length | key data | index id | …
Version 19: | index id | key length | key data | sort key length | sort key data | index id | …

But hopefully we can just check if the number following the key data is a valid index id and upgrade accordingly. I'm going to try that.

	Johan Lorenzo [:jlorenzo]
	Comment 17 • 9 years ago

Issue identified. Regression window not needed anymore.

	Naoki Hirata :nhirata (please use needinfo instead of cc)
	Comment 18 • 9 years ago

Would it be possible if you can message out changes to the data format please?
We ( QA ) would then know to check OTA more so.  As far as I know we don't have automated tests around OTA/FOTA.

	Reuben Morais [:reuben] Assignee
	Comment 19 • 9 years ago

Can anyone share an affected database with me? My Flame gets all kinds of broken when I try to flash just the Gecko upgrade.

(In reply to Naoki Hirata :nhirata (please use needinfo instead of cc) from comment #18)
> Would it be possible if you can message out changes to the data format
> please?
> We ( QA ) would then know to check OTA more so.  As far as I know we don't
> have automated tests around OTA/FOTA.

Sure, I'll keep it in mind, but this is a question for the IDB peers.

	Naoki Hirata :nhirata (please use needinfo instead of cc)
	Comment 20 • 9 years ago

jlorenzo, ktucker : Can either of you share a sample data that exhibits the issue with the devs please?

	Pi Wei Cheng [:piwei] (inactive)
	Comment 21 • 9 years ago

Naoki or Reuben, could you explain what exactly is needed from QA and how we should go about to obtain it? We're unsure what 'affected database' or 'data' means in this context.

	Reuben Morais [:reuben] Assignee
	Comment 22 • 9 years ago

The database is the following file on your device, you can just |adb pull| it:

/data/local/storage/permanent/chrome/idb/226660312ssm.sqlite

This contains all your sent and received messages and their senders/recipients, so you might not want to attach it here (or share it at all). You can mail it to me directly.

	Reuben Morais [:reuben] Assignee
	Comment 23 • 9 years ago

Ok, the database file might not be needed anymore. Instead, it'd be great if you could try applying this patch and see if it helps.

	Reuben Morais [:reuben] Assignee
	Comment 24 • 9 years ago

Canceling this NI, this is unlikely to have anything to do with Kyle's patch.

	Jan Varga [:janv]
	Comment 25 • 9 years ago

Reuben, you could also add a test for this, see test_schema18upgrade.js
However, since this bug is urgent, you can do it in a followup bug.

I should have caught this during review. Anyway, I've got an idea how to write a test to prevent this kind of problems in future.

	Johan Lorenzo [:jlorenzo]
	Comment 26 • 9 years ago

I' testing it. I'm making one build without the patch and one build with, to get the differences.

	:gerard-majax
	Comment 27 • 9 years ago

(In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #26)
> I' testing it. I'm making one build without the patch and one build with, to
> get the differences.

I do confirm that an uptodate Gecko with this patch fixes the issue for me :)

	Reuben Morais [:reuben] Assignee
	Comment 29 • 9 years ago

(In reply to Alexandre LISSY :gerard-majax from comment #27)
> (In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #26)
> > I' testing it. I'm making one build without the patch and one build with, to
> > get the differences.
> 
> I do confirm that an uptodate Gecko with this patch fixes the issue for me :)

Thanks!

	Jan Varga [:janv]
	Comment 30 • 9 years ago

Reviewing now.

	Jan Varga [:janv]
	Comment 31 • 9 years ago

Comment on attachment 8659842 [details] [diff] [review]
Upgrade object_data

Review of attachment 8659842 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, but please write a test to make sure the upgrade works fine. See test_schema18upgrade.js and schema18upgrade_profile.zip
Basically, you need to create a database with some data in previous nightly and then import it to the profile and test that thing still work after the upgrade.
You can do it in a followup bug.

::: dom/indexedDB/ActorsParent.cpp
@@ +3857,5 @@
>  
> +class UpgradeIndexDataValuesFunction final
> +  : public mozIStorageFunction
> +{
> +  nsRefPtr<mozIStorageConnection> mConnection;

Is this member used at all ?

@@ +4015,5 @@
> +
> +  std::pair<const void *, int> data(static_cast<void*>(newIdv.get()),
> +                                    newIdvLength);
> +
> +  nsCOMPtr<nsIVariant> result = new mozilla::storage::BlobVariant(data);

What about |result = new storage::AdoptedBlobVariant(copiedBlobDataPair);|
I know there's some old code that still uses BlobVariant(), but we could try to do better here.

@@ +4022,5 @@
> +  return NS_OK;
> +}
> +
> +nsresult
> +UpgradeSchemaFrom20_0To21_0(mozIStorageConnection* aConnection)

I would maybe add a comment that this upgrade should have been a part of the 18 to 19 upgrade.

	Johan Lorenzo [:jlorenzo]
	Comment 32 • 9 years ago

Comment on attachment 8659842 [details] [diff] [review]
Upgrade object_data

Tested on top of gecko at revision [1]. Old threads as well as new are fine. You're able to send SMS with no issue.

Testing protocol:
1. Get a build before that regression occurred (m-c 20150902150202) and create an SMS thread => OK
2. Put gecko at revision [1] with no patch and send an SMS in the same thread => The bug happened.
3. Apply the patch and send an SMS in the same thread => No more issues
4. Create new threads and send SMS in there => OK.

[1] 22565071a2edc3aa53038a99068ebf1844fd4dac

	mac Reporter
	Comment 33 • 9 years ago

It shouldn't be an issue after the fix but could you check if it is possible to receive message in the old thread? In the time it was impossible not only to send but also to receive message in an old thread.

	Johan Lorenzo [:jlorenzo]
	Comment 34 • 9 years ago

I just checked, receiving SMS can be done with the patch applied.

	Pulsebot
	Comment 35 • 9 years ago

https://hg.mozilla.org/integration/b2g-inbound/rev/81627b348cd5

	Phil Ringnalda (:philor)
	Comment 36 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/81627b348cd5

	Adam Alldredge [:AdamA][Inactive]
	Comment 38 • 9 years ago

This issue is verified fixed in Aries 2.5.

Environmental Variables:
Device: Flame 2.5 [Full Flash]
BuildID: 20150911103006
Gaia: 758c75ee087ea3722213ea2c185cca1d952c8a29
Gecko: 19f806034f67e8fc84b352db10f1a21ab982670f
Gonk: c4779d6da0f85894b1f78f0351b43f2949e8decd
Version: 43.0a1 (2.5) 
Firmware Version: v18D
User Agent: Mozilla/5.0 (Mobile; rv:43.0) Gecko/43.0 Firefox/43.0

Result:
User is able to send SMS in threads that were created before OTA.

Notes:
I updated from RC4.