1057973 - Receiving Carrier Download MMS instead of the actual message

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[Florin Strugariu [:Bebe]]

Attachment: screenshot from jenkins

We are getting a error when receiving a MMS.

"Attachment can be downloaded until Thursday, Aug 28. Download"

The first time we saw this error is build:

http://jenkins1.qa.scl3.mozilla.com/job/flame.b2g-inbound.ui.functional.smoke/954/testReport/

Tested this locally and all works as expected.

It looks like a local carrier issue.

Comment 1

[Florin Strugariu [:Bebe]]

Raymond can you take a look over this please.

Until then I will xfail the test.

Comment 2

[Florin Strugariu [:Bebe]]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/23260

Comment 3

[Florin Strugariu [:Bebe]]

Looks like this is not a carrier issue as I first thought.

This test is working fine on 2.0 on the same devices.


From the B2G-i :

last good build:
application_buildid: 20140822045656
application_changeset: 7f46b03873c3
application_display_name: B2G
application_name: B2G
application_repository: https://hg.mozilla.org/integration/b2g-inbound
application_version: 34.0a1
build_changeset: 7eef86294cd794ab9e6a53d218c238bfc63c3a6d
device_firmware_date: 1403855878
device_firmware_version_incremental: 110
device_firmware_version_release: 4.3
device_id: flame
gaia_changeset: 33e471f952876677d770a16842a9f50135e3d9ed
gaia_date: 1408706654
platform_buildid: 20140822045656
platform_changeset: 7f46b03873c3
platform_repository: https://hg.mozilla.org/integration/b2g-inbound



First bad build:
application_buildid: 20140822064156
application_changeset: ab6ec16fcdf6
application_display_name: B2G
application_name: B2G
application_repository: https://hg.mozilla.org/integration/b2g-inbound
application_version: 34.0a1
build_changeset: 7eef86294cd794ab9e6a53d218c238bfc63c3a6d
device_firmware_date: 1406125546
device_firmware_version_incremental: eng.cltbld.20140723.102536
device_firmware_version_release: 4.3
device_id: flame
gaia_changeset: ecb4dc352e709b58da52428b543222d1d5ac8cc2
gaia_date: 1408713888
platform_buildid: 20140822064156
platform_changeset: ab6ec16fcdf6
platform_repository: https://hg.mozilla.org/integration/b2g-inbound



Git diff:
https://github.com/mozilla-b2g/gaia/compare/33e471f952876677d770a16842a9f50135e3d9ed...ecb4dc352e709b58da52428b543222d1d5ac8cc2


Geko diff:
http://hg.mozilla.org/integration/b2g-inbound/pushloghtml?fromchange=7f46b03873c3&tochange=ab6ec16fcdf6

Comment 4

[Florin Strugariu [:Bebe]]

STR:

1. Open the messages app
2. Send a MMS to your number containing a photo took with the camera
3. Wait for the MMS.

Expected:
3. The MMS is recived

Actual:
3. A message to download the MMS is recived

We can't reproduce this in RO with 2 different carriers but we can see it reproducing on the grid on b2g-i and master without any issues

Comment 5

[Jason Smith [:jsmith]]

There's a bunch of RIL landings in the push log here, so let's start there (bug 1056618, bug 1056522, bug 1055994, bug 1032097).

Hsin-Yi - Any ideas on why this is happening?

Comment 6

[Jason Smith [:jsmith]]

[Blocking Requested - why for this release]:

Causes our smoketest for MMS to permafail, which makes this a qablocker.

Marking qawanted to see if we can reproduce this in the US. Including qaurgent cause this could be a smoketest issue.

Comment 7

[raymond [:retornam] (inactive)]

(In reply to Florin Strugariu [:Bebe] from comment #1)
> Raymond can you take a look over this please.
> 
> Until then I will xfail the test.

http://jenkins1.qa.scl3.mozilla.com/job/flame.b2g-inbound.ui.functional.smoke/954/console


ran on B2G-15.1 . I took the device offline when I came in and couldn't send an MMS with it because it kept failing. Output of check version script



Gaia      169a465e71138b892799b8a8f188eaf7d1bf06d0
Gecko     https://hg.mozilla.org/integration/b2g-inbound/rev/c99f0f778f7a
BuildID   20140825075947
Version   34.0a1
ro.build.version.incremental=eng.cltbld.20140723.102536
ro.build.date=Wed Jul 23 10:25:46 EDT 2014

Comment 8

[Duane Dixon [:ddixon]]

Branch Checks 

Issue DOES occur in Flame 2.1, Open_C 2.1

(Flame device tested with 512 MB memory) 

Device: Flame Master
Build ID: 20140823032956
Gaia: e424c85eda87a40c0fa64d6a779c3fa368bf770b
Gecko: daa84204a11a
Version: 34.0a1 (Master)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
---------------------------------------------------------------------------------------
Device: Open_C Master
Build ID: 20140825085054
Gaia: a25ae14dbd2fe3e25144a7064efc0cc4e31042b8
Gecko: 4ca2bd0722d9
Version: 34.0a1 (Master)
Firmware Version: P821A10V1.0.0B06_LOG_DL
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0
---------------------------------------------------------------------------------------
---------------------------------------------------------------------------------------
Issue DOES NOT occur in Flame 2.0, 1.4

Device: Flame 2.0
BuildID: 20140823033754
Gaia: 4c8b5ced1966079086d86dec3098ecf340881306
Gecko: b0545e46d08b
Version: 32.0 (2.0) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0
---------------------------------------------------------------------------------------
Device: Flame 1.4
Build ID: 20140825062151
Gaia: cf9d74da6653efeb43d9653e81c61aa00e693a67
Gecko: cdcb73d0febc
Version: 30.0 (1.4)
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Comment 9

[Bevis Tseng[:bevis][:btseng](Exited)]

We have some change to fix the routing problem during mms transactions in bug 1032097.
However, we didn't see this error in local carrier.

Mark qawanted for:
1. Identify the regression to bug 1032097.
2. get adb logcat & corresponding tcpdump for further analysis by the instructions in the link:
   https://github.com/bevis-tseng/Debug_Tools

Comment 10

[Zac C (:zac)]

(In reply to Florin Strugariu [:Bebe] from comment #0)

> We are getting a error when receiving a MMS.
> 
> "Attachment can be downloaded until Thursday, Aug 28. Download"
> 

This is a bad grammatical/English bug we should raise separately too!

Comment 11

[Zac C (:zac)]

(In reply to Zac C (:zac) from comment #10)
> (In reply to Florin Strugariu [:Bebe] from comment #0)
> 
> > We are getting a error when receiving a MMS.
> > 
> > "Attachment can be downloaded until Thursday, Aug 28. Download"
> > 
> 
> This is a bad grammatical/English bug we should raise separately too!

Filed:
https://bugzilla.mozilla.org/show_bug.cgi?id=1058526

Comment 12

[Zac C (:zac)]

Comment on attachment 8478134 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/23260

label is spelt wrong but I won't block. r+, merged:
https://github.com/mozilla-b2g/gaia/commit/01d7d492df5160b2c4acf34da7f7ffe50975f3a5

This bug has now caused test coverage to be disabled.

Comment 13

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Zac C (:zac) from comment #10)
> (In reply to Florin Strugariu [:Bebe] from comment #0)
> 
> > We are getting a error when receiving a MMS.
> > 
> > "Attachment can be downloaded until Thursday, Aug 28. Download"
> > 
> 
> This is a bad grammatical/English bug we should raise separately too!

IMHO, this message could be created from carrier-side and is received as a text message for the notification instead.

Comment 14

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #13)
> (In reply to Zac C (:zac) from comment #10)
> > (In reply to Florin Strugariu [:Bebe] from comment #0)
> > 
> > > We are getting a error when receiving a MMS.
> > > 
> > > "Attachment can be downloaded until Thursday, Aug 28. Download"
> > > 
> > 
> > This is a bad grammatical/English bug we should raise separately too!
> 
> IMHO, this message could be created from carrier-side and is received as a
> text message for the notification instead.

After checking the screenshot, I just realized that it's the MMS-Notification to be displayed in current UI design:
https://github.com/mozilla-b2g/gaia/blob/aa72df680c56bad1507ca0ee2d640d300fe91a1d/apps/sms/locales/sms.en-US.properties#L224

Comment 15

[Duane Dixon [:ddixon]]

Attachment: logcat_mms_malfunction.txt

Comment 16

[Duane Dixon [:ddixon]]

Attachment: capture.pcap

Comment 17

[Duane Dixon [:ddixon]]

(In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #9)
> We have some change to fix the routing problem during mms transactions in
> bug 1032097.
> However, we didn't see this error in local carrier.
> 
> Mark qawanted for:
> 1. Identify the regression to bug 1032097.
> 2. get adb logcat & corresponding tcpdump for further analysis by the
> instructions in the link:
>    https://github.com/bevis-tseng/Debug_Tools

I added the adb logcat and the capture.pcap file in the Attachments field.  I'm unsure what you mean by your first request: "1. Identify the regression to bug 1032097."

Comment 18

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Duane Dixon [:ddixon] from comment #17)
> (In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #9)
> > We have some change to fix the routing problem during mms transactions in
> > bug 1032097.
> > However, we didn't see this error in local carrier.
> > 
> > Mark qawanted for:
> > 1. Identify the regression to bug 1032097.
> > 2. get adb logcat & corresponding tcpdump for further analysis by the
> > instructions in the link:
> >    https://github.com/bevis-tseng/Debug_Tools
> 
> I added the adb logcat and the capture.pcap file in the Attachments field. 
> I'm unsure what you mean by your first request: "1. Identify the regression
> to bug 1032097."

Thanks for capturing these information.
According to the log, there are 3 incoming MMS messages.
Only the 1st one is retrieved successfully, the 2nd/3rd are failed due to "Invalid network interface"
There seems something wrong in the patch of bug 1032097.
I'll try to see if this symptom can be reproduced locally for further analysis.

Comment 19

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Patch Part 1 v1: Ensure the initialization is done before setupDataCallByType.

The root cause of this bug is that, 
1. for shared APN, the result of setupDataCallByType() comes synchronously when default APN is already enabled.
2. This cause the init actions right after setupDataCallByType() are done unexpectedly after the mms transaction is done. That means this.networkInterface will be set to null unexpectedly even the connection is still alive and cause the upcoming MMS transactions failed.

The formal solution to have consistent behavior of setupDataCallByType() has been filed in new bug 1059110.

This patch is to:
1. Do the init actions before setupDataCallByType().
2. Remove |Clear hostsToRoute/networkInterface| which has been covered in 
http://dxr.mozilla.org/mozilla-central/source/dom/mobilemessage/src/gonk/MmsService.js#536
and 
http://dxr.mozilla.org/mozilla-central/source/dom/mobilemessage/src/gonk/MmsService.js#300

Hi Vicamo,

Sorry to say that this is a regression of the fix in bug 1032097.
May I have your review for the fix of this regression?

Thanks!

Comment 20

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Patch Part 2: QueryInterface(Ci.nsIRilNetworkInterface) before accessing serviceId property.

This is to ensure that QueryInterface(Ci.nsIRilNetworkInterface) will be done before accessing the properties defined in nsIRilNetworkInterface.

Hi Edgar,

May I have your review for this change?

Thanks!

Comment 21

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8480318 [details] [diff] [review]
Patch Part 2: QueryInterface(Ci.nsIRilNetworkInterface) before accessing serviceId property.

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

::: dom/system/gonk/NetworkManager.js
@@ +288,1 @@
>        id = "ril" + network.serviceId;

Don't assign something of another type to a existing variable.

  let rilNetwork = network.QueryInterface(Ci.nsIRilNetworkInterface);
  id = "ril" + rilNetwork.serviceId;

@@ +803,2 @@
>          if (serviceId != undefined && this.isNetworkTypeMobile(network.type) &&
>              network.serviceId != serviceId) {

ditto.

  if (serviceId != undefined &&
      this.isNetworkTypeMobile(network.type) &&
      network instanceof Ci.nsIRilNetworkInterface) {
    let rilNetwork = network.QueryInterface(Ci.nsIRilNetworkInterface);
    if (rilNetwork.serviceId != serviceId) {
      continue;
    }
  }

Comment 22

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 8479683 [details] [diff] [review]
Patch Part 1 v1: Ensure the initialization is done before setupDataCallByType.

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

Thank you!

Comment 23

[Bevis Tseng[:bevis][:btseng](Exited)]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #21)
> Comment on attachment 8480318 [details] [diff] [review]
> Patch Part 2: QueryInterface(Ci.nsIRilNetworkInterface) before accessing
> serviceId property.
> 
> Review of attachment 8480318 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/NetworkManager.js
> @@ +288,1 @@
> >        id = "ril" + network.serviceId;
> 
> Don't assign something of another type to a existing variable.
> 
>   let rilNetwork = network.QueryInterface(Ci.nsIRilNetworkInterface);
>   id = "ril" + rilNetwork.serviceId;

Will do! :)
> 
> @@ +803,2 @@
> >          if (serviceId != undefined && this.isNetworkTypeMobile(network.type) &&
> >              network.serviceId != serviceId) {
> 
> ditto.
> 
>   if (serviceId != undefined &&
>       this.isNetworkTypeMobile(network.type) &&
>       network instanceof Ci.nsIRilNetworkInterface) {
>     let rilNetwork = network.QueryInterface(Ci.nsIRilNetworkInterface);
>     if (rilNetwork.serviceId != serviceId) {
>       continue;
>     }
>   }

Comment 24

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Patch Part 2 v2: QueryInterface(Ci.nsIRilNetworkInterface) before accessing serviceId property.

1. Address the problem in comment 21.
2. We only have to check (network instanceof Ci.nsIRilNetworkInterface) if we want to access network.serviceId, so I remove uncessary checking of isNetworkTypeMobile(network.type).

Hi Vicamo,

May I have your review again for the change?

Thanks!

Comment 25

[Bevis Tseng[:bevis][:btseng](Exited)]

Hi Zac,

As mentioned in comment 19, This shall be the regression of bug 1032097.
Shall I obsolete your patch of attachment 8478134 [details] [review]?

Comment 26

[Zac C (:zac)]

Bevis, yes if you can revert that commit with your fix then we'll get this coverage back for devices.

Comment 27

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Patch Part 1: Ensure the initialization is done before setupDataCallByType. r=vyang

update final patch with positive try server result.

Comment 28

[Bevis Tseng[:bevis][:btseng](Exited)]

Attachment: Patch Part 2: QueryInterface(Ci.nsIRilNetworkInterface) before accessing serviceId property. r=vyang

update final patch with positive try server result.

Comment 29

[Bevis Tseng[:bevis][:btseng](Exited)]

update try server result:
https://tbpl.mozilla.org/?tree=Try&rev=32de6b0b49ad

Comment 30

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/b2g-inbound/rev/e229a3641ada
https://hg.mozilla.org/integration/b2g-inbound/rev/91bebdf15f48

Comment 31

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e229a3641ada
https://hg.mozilla.org/mozilla-central/rev/91bebdf15f48

Comment 33

[Bevis Tseng[:bevis][:btseng](Exited)]

[Blocking Requested - why for this release]:
This shall be 2.1+ because it cause downloading MMS become abnormal in the carrier where shared APN is applied and it's a regression of bug 1032097.

Comment 34

[Bevis Tseng[:bevis][:btseng](Exited)]

It seems that the fix already included in 2.1 branch. remove 2.1? flag.

Comment 35

[Jason Smith [:jsmith]]

(In reply to Bevis Tseng [:bevistseng] ([e-mail]) from comment #34)
> It seems that the fix already included in 2.1 branch. remove 2.1? flag.

Right - that's why I cleared the flag.