Closed Bug 1014916 Opened 6 years ago Closed 6 years ago

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

Categories

(Core :: DOM: Geolocation, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla32
blocking-b2g 1.3T+
Tracking Status
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: garvan, Assigned: garvan)

References

Details

Attachments

(1 file, 6 obsolete files)

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).
Depends on: 1014924
Attached patch bug-1014916.diff (obsolete) — Splinter Review
Loop through the radio interfaces until one returns valid results, storing the index for later use.
Attachment #8427383 - Flags: review?(dougt)
Attached patch bug-1014916.diff (obsolete) — Splinter Review
Loop through the radio interfaces until one returns valid results, storing the index for later use.
Attachment #8427383 - Attachment is obsolete: true
Attachment #8427383 - Flags: review?(dougt)
Attachment #8427396 - Flags: review?(dougt)
Attached patch bug-1014916.diff (obsolete) — Splinter Review
Loop through the radio interfaces until one returns valid results, storing the index for later use.
Attachment #8427396 - Attachment is obsolete: true
Attachment #8427396 - Flags: review?(dougt)
Attachment #8427398 - Flags: review?(dougt)
Attached patch bug1014916.diff (obsolete) — Splinter Review
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.
Attachment #8427398 - Attachment is obsolete: true
Attachment #8427398 - Flags: review?(dougt)
Attachment #8427473 - Flags: review?(dougt)
blocking-b2g: --- → 1.3T?
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.
Attachment #8427473 - Flags: review?(dougt)
Flags: needinfo?(dougt)
Blocks: 1014924
No longer depends on: 1014924
Depends on: 1007953
Attached patch bug-1014916.diff (obsolete) — Splinter Review
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)
Attachment #8429302 - Flags: review?(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.
Attachment #8429302 - Flags: review?(dougt) → review+
Attached patch bug-1014916.diff (obsolete) — Splinter Review
As per review, removed limit of 10 max radios.
Attachment #8427473 - Attachment is obsolete: true
Attachment #8429302 - Attachment is obsolete: true
Attachment #8429374 - Flags: review?(dougt)
Flags: needinfo?(dougt)
Attached patch bug-1014916.diffSplinter Review
As per review, removed limit of 10 max radios.
Attachment #8429374 - Attachment is obsolete: true
Attachment #8429374 - Flags: review?(dougt)
Attachment #8429378 - Flags: review?(dougt)
Attachment #8429374 - Attachment is obsolete: false
Attachment #8429378 - Flags: review?(dougt) → review+
Keywords: checkin-needed
Attachment #8429374 - Attachment is obsolete: true
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?
https://hg.mozilla.org/mozilla-central/rev/73326956f225
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
blocking-b2g: 1.3T? → 1.3T+
Blocks: 1009592
Blocks: 971637
You need to log in before you can comment on or make changes to this bug.