1014916 - [B2G][Tarako][Geolocation] No cell geolocation with SIM card in slot 2

Status: RESOLVED FIXED

(Core :: DOM: Geolocation, defect)

Description

[:garvan]

In NetworkGeolocationProvider.js, we scan the cells using Cc["@mozilla.org/ril;1"].getService(Ci.nsIRadioInterfaceLayer).getRadioInterface(0);

However, the SIM card may be in slot 2, which is index 1.

With a SIM in slot 2 and cell data plan on, the device should fail to get a location (HERE Maps is good to test with).

Comment 1

[:garvan]

Attachment: bug-1014916.diff

Loop through the radio interfaces until one returns valid results, storing the index for later use.

Comment 2

[:garvan]

Attachment: bug-1014916.diff

Loop through the radio interfaces until one returns valid results, storing the index for later use.

Comment 3

[:garvan]

Attachment: bug-1014916.diff

Loop through the radio interfaces until one returns valid results, storing the index for later use.

Comment 4

[:garvan]

Attachment: bug1014916.diff

See previous comments for commit message. 
Just been trying to clean up this commit. Avoiding learning how to stage a hunk in hg has not served me well.

Comment 5

[:garvan]

Comment on attachment 8427473 [details] [diff] [review]
bug1014916.diff

Doug, I have a patch that concatenates all the radios' results. However code is based on bug 1007953. It is possible not to base it on that, but someone will have to deal with conflicts. What is the best approach?

(FYI, the code is here: http://pastebin.com/raw.php?i=vvE69t3X)

I'll submit the patch when I get info on how it is best to submit it.

Comment 6

[:garvan]

Attachment: bug-1014916.diff

This patch is applied after bug 1007953 (marked as a blocker of this). 

This patch loops through the available wireless interfaces (max of 10 to guard against a bad val from lower layer seizing main thread)

Comment 7

[Doug Turner (:dougt)]

Comment on attachment 8429302 [details] [diff] [review]
bug-1014916.diff

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

remove the safeguard.  Otherwise looks good.

::: dom/system/NetworkGeolocationProvider.js
@@ +199,5 @@
> +      let radioService = Cc["@mozilla.org/ril;1"]
> +                    .getService(Ci.nsIRadioInterfaceLayer);
> +      let numInterfaces = radioService.numRadioInterfaces;
> +      let result = [];
> +      for (let i = 0; i < numInterfaces && i < 10 /*safeguard*/; i++) {

I don't think we need this safe guard.  If there is a bug in the underlying subsystem, we should fix that.

Comment 8

[:garvan]

Attachment: bug-1014916.diff

As per review, removed limit of 10 max radios.

Comment 9

[:garvan]

Attachment: bug-1014916.diff

As per review, removed limit of 10 max radios.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/73326956f225

Should this have a test?

Comment 11

[:garvan]

It calls the API the gets the number of SIM slots, the only way I know to test that is manually on-device. 
I'm not sure of another way, any suggestion?

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/73326956f225

Comment 13

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/20ff18dbee48

Comment 14

[Doug Turner (:dougt)]

https://hg.mozilla.org/releases/mozilla-b2g30_v1_4/rev/e08c1bd28fb5