833711 - B2G RIL: Add lastKnownMcc into nsIDOMMobileConnectionInfo

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Although Bug 819560 adds lastKnownMcc, but it seemed that field was added into wrong place, it was added in MobileIccInfo.

The lastKnownMcc is from voice connection, so it should be added into nsIDOMMobileNetworkInfo.

Comment 1

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

CCing Gregor and Vicamo

Comment 2

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Gregor, as I have discussed with you during Work Week,
seems we don't have to modify the IDL, 
we could just write lastKnownMcc into settigs/pref, right?

Otherwise if the phone reboots it still cannot read lastKnownMcc from MobileConnection.

Comment 3

[Gregor Wagner [:gwagner]]

(In reply to Yoshi Huang[:yoshi][:allstars.chh] from comment #2)
> Gregor, as I have discussed with you during Work Week,
> seems we don't have to modify the IDL, 
> we could just write lastKnownMcc into settigs/pref, right?
> 
> Otherwise if the phone reboots it still cannot read lastKnownMcc from
> MobileConnection.

If you call navigator.mozMobileConnection you still call the constructor right? And in the constructor we read the lastKnownMcc from the pref. So it should be accessible. But I am open for another solution here.

Comment 4

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

As Bug 830164 will remove lastKnownMcc from IccInfo, I mark this bug 833711 depends on bug 830164.

Comment 5

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

As Vicamo suggests lastKnownMcc should not be in nsIDOMMobileNetworkInfo, because it doesn't belong to current network operator, but previous available network operator.
So I'll move lastKnownMcc into nsIDOMMobileConnectionInfo for voice connection.

Comment 6

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Part 1: Add lastKnownMcc into nsIDOMMobileConnectionInfo.

Comment 7

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Part 2: Add lastKnownMcc in RIL.

Comment 8

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Part 4: marionette for lastKnownMcc.

Comment 9

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Part 2: Move lastKnownMcc into voice connection. v2

Merged Part 5 patch from Bug 830164.

Comment 10

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 705789 [details] [diff] [review]
Part 1: Add lastKnownMcc into nsIDOMMobileConnectionInfo.

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

Hi, Jonas

This bug is filed for Bug 819560 adds lastKnownMcc to wrong place, it adds lastKnowMcc into nsIDOMMobileIccInfo
which is wrong, since SIM card should have only one MCC.

My first patch is to add it into nsIDOMMobileNetworkInfo.

But Vicamo suggests since lastKnownMcc is not from current network operator, we should not add it into nsIDOMMobileNetworkInfo.
Instead, we should add lastKnownMcc into voice connection,
that is, nsIDOMMobileConnectionInfo. Please also see Comment 5.

thanks

Comment 11

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Part 2: Move lastKnownMcc into voice connection. v3

I found many functions in RILContentHelper are just to copy object,
so I add an utility updateInfo for that.

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 705789 [details] [diff] [review]
Part 1: Add lastKnownMcc into nsIDOMMobileConnectionInfo.

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

How is this different from previousMcc? They seem identical?

Comment 13

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

previousMcc was added in bug 819560

Comment 14

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 705789 [details] [diff] [review]
Part 1: Add lastKnownMcc into nsIDOMMobileConnectionInfo.

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

Hmm... ok.. i see. I don't know the interfaces under .mobileconnection well enough so if vicamo thinks this is good then that's fine with me.

Comment 15

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Hi, Jonas
Actually It's the same with previousMcc,
but you commented in Bug 819560 comment 7 [1] to rename it to lastKnownMcc.

But at that time Gregor adds to wrong place, he adds to MobileIccInfo, which is for SIM card. 
Recall that SIM card has only one MCC, wherever you go.

So this bug is the move lastKnownMcc to correct place, into voice connection.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=819560#c7

Comment 16

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Hi, Jonas 
Also I'd like to remind that the patch in Bug 819560 is an older one, it still uses 'previousMcc', but you can check the actual commit in Bug 819560 comment 8, it has been modified to 'lastKnownMcc'.

Comment 17

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 706209 [details] [diff] [review]
Part 2: Move lastKnownMcc into voice connection. v3

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

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +773,5 @@
>        this.updateDataConnection(dataMessage);
>      }
>  
> +    if (operatorMessage) {
> +      this.handleOperatorChange(operatorMessage);

Here could be wrong, 
it will send a VoiceInfoChanged event.
Will address that.

Comment 18

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Part 2: Move lastKnownMcc into voice connection. v4

Comment 19

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Part 2: Move lastKnownMcc into voice connection. v4

Comment 20

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

Comment on attachment 706334 [details] [diff] [review]
Part 2: Move lastKnownMcc into voice connection. v4

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

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +773,5 @@
>        this.updateDataConnection(dataMessage);
>      }
>  
> +    if (operatorMessage) {
> +      operatorMessage.batch = true;

Please move these operator message handling changes into another patch or file another bug :)

Comment 21

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Part 2 (new) : Revise handleoperatorchange in RadioInterfaceLayer and updateInfo in RILContentHelper.

As Vicamo suggests to split code into another patch.
This patch is already r+

Comment 22

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Part 3: Move lastKnownMcc into voice connection. v4 (Former Part 2)

Comment 23

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Attachment: Part 1: Add lastKnownMcc into nsIDOMMobileConnectionInfo. v2

rebase

Comment 24

[Mozilla RelEng Bot]

Try run for 423ce09fd88a is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=423ce09fd88a
Results (out of 334 total builds):
    exception: 1
    success: 318
    warnings: 13
    failure: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-423ce09fd88a

Comment 25

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a4c130560ea6
https://hg.mozilla.org/integration/mozilla-inbound/rev/6d4a334a99bc
https://hg.mozilla.org/integration/mozilla-inbound/rev/da1f103142bd
https://hg.mozilla.org/integration/mozilla-inbound/rev/785366ece8a0

Comment 26

[Mozilla RelEng Bot]

Try run for 508f73f7c51f is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=508f73f7c51f
Results (out of 337 total builds):
    exception: 1
    success: 327
    warnings: 7
    failure: 2
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-508f73f7c51f

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a4c130560ea6
https://hg.mozilla.org/mozilla-central/rev/6d4a334a99bc
https://hg.mozilla.org/mozilla-central/rev/da1f103142bd
https://hg.mozilla.org/mozilla-central/rev/785366ece8a0