Closed Bug 1093688 Opened 6 years ago Closed 5 years ago

[PP] Adjustable Location Accuracy - Exception not working

Categories

(Firefox OS Graveyard :: General, defect, P1)

Tracking

(b2g-v2.0 wontfix, b2g-v2.0M wontfix, b2g-v2.1 wontfix, b2g-v2.1S wontfix, b2g-v2.2 affected, b2g-master affected)

RESOLVED WONTFIX
Tracking Status
b2g-v2.0 --- wontfix
b2g-v2.0M --- wontfix
b2g-v2.1 --- wontfix
b2g-v2.1S --- wontfix
b2g-v2.2 --- affected
b2g-master --- affected

People

(Reporter: erik.zitto, Assigned: huseby)

References

Details

(Keywords: privacy)

Attachments

(3 files, 3 obsolete files)

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:33.0) Gecko/20100101 Firefox/33.0
Build ID: 20141013195847

Steps to reproduce:

In Privacy Panel -> Location Accuracy I set an exception for an application. It should show blurred current location.


Actual results:

Displayed location remained the same as before setting the exception.


Expected results:

Displayed location should vary from the previous location according the set radius.
Agree with Erik, test failed. 

To reproduce this bug please try:

- gecko from  branch https://github.com/dhuseby/gecko/commits/privacy-panel-master, commit: a8ed2c287f
- gaia from https://github.com/kukomigo/gaia/tree/freeze-master, commit 197b30a814a 
(it is gaia master version from bebef1987 with latest patch from Privacy Panel - https://github.com/kukomigo/gaia/tree/privacy-panel-master-apps, commit 252ab5f4)
Today we had test failed again.

Tested versions:
- gecko from https://git.mozilla.org/releases/gecko.git, commit: 8bae192f289
- gaia from https://github.com/Wojtek-Lakomy/gaia/tree/privacy-panel-master-apps, commit: dc77aea67b6c
Present also with new build of Gecko - https://git.mozilla.org/releases/gecko.git - master branch, commit 8bae192f289ece626288df9b2f448d5b77dade49, Gaia remained same as before tested.
While testing this one I'm getting persistent crashes of b2g.  I've been investigating all day.  I built using the tip of gecko master and the latest version of Wojtek-Lakomy/privacy-panel-master-apps.
Summary: [PP] Adjustable Locacation Accuracy - Exception not working → [PP] Adjustable Location Accuracy - Exception not working
so here's more info on the random crashes.  it looks like it isn't geolocation's fault.  i'm bisecting it now:

> W/GPS     ( 5114): ../../../dom/geolocation/nsGeolocationSettings.cpp:160   mozsettings changed: geolocation.app_settings
> I/ServiceManager(  279): service 'android.security.keystore' died
> I/ServiceManager(  279): service 'media.resource_manager' died
> I/ServiceManager(  279): service 'SurfaceFlinger' died
> I/ServiceManager(  279): service 'permission' died
> I/ServiceManager(  279): service 'display.qservice' died
> D/nfcd    (  322):  8 of bytes to be sent... data=0x0 ret=0
> F/libc    ( 5334): Fatal signal 11 (SIGSEGV) at 0x00000000 (code=1), thread 5334 (Homescreen)
> F/libc    ( 5334): Unable to open connection to debuggerd: Permission denied
Attached patch Bug_1093688.patch (obsolete) — Splinter Review
This patches fixes both this bug and bug 1097636.
Assignee: nobody → huseby
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8523396 - Flags: review?(martin.thomson)
Duplicate of this bug: 1097636
Fixed and working. Gecko: master, apply patch https://bugzilla.mozilla.org/attachment.cgi?id=8523396
Gaia: https://github.com/martasect/gaia/tree/privacy-panel-master-apps commit: 541d8201f7ccec25f582ba1ee115f7c484477e48
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → WORKSFORME
Attached patch Bug 1093688.patch (obsolete) — Splinter Review
I forgot to initialize the JS compartment.  I just need a quick r+ to land this.
Attachment #8523396 - Attachment is obsolete: true
Attachment #8523396 - Flags: review?(martin.thomson)
Attachment #8528676 - Flags: review?(martin.thomson)
Re-opening so that the patch can be landed.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Comment on attachment 8528676 [details] [diff] [review]
Bug 1093688.patch

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

I really don't know that part of the code very well, but this looks right.
Attachment #8528676 - Flags: review?(martin.thomson) → review+
Keywords: checkin-needed
Can you please run this through Try first and update the patch to include a useful commit message?
https://developer.mozilla.org/en-US/docs/Developer_Guide/Committing_Rules_and_Responsibilities#Checkin_comment
Keywords: checkin-needed
Attached patch Bug 1093688.patch (obsolete) — Splinter Review
Updated patch with better commit message.
Attachment #8528676 - Attachment is obsolete: true
Try pass: http://mzl.la/1yiO127
Another try pass to see if the errors in the lass try are transient: http://mzl.la/1B3plgt
Attachment #8529372 - Attachment is obsolete: true
None of the failures in the last try pass are related to this patch.
Keywords: checkin-needed
dave, do you know if the failure in https://treeherder.mozilla.org/ui/logviewer.html#?job_id=3486743&repo=try is related to the patch seems this was in the last 2 try runs at least
Flags: needinfo?(huseby)
Keywords: checkin-needed
I believe it is unrelated.  It is a test for browser save image.
Flags: needinfo?(huseby)
Keywords: checkin-needed
(In reply to Dave Huseby [:huseby] from comment #19)
> I believe it is unrelated.  It is a test for browser save image.

seems it was not unrelated test failed after this checkin like https://treeherder.mozilla.org/ui/logviewer.html#?job_id=938642&repo=b2g-inbound so i had to back this out
Flags: needinfo?(huseby)
Alright, I'm on it.  I'll update soon.
Flags: needinfo?(huseby)
(In reply to Dave Huseby [:huseby] from comment #17)
> None of the failures in the last try pass are related to this patch.

+1, there is a long outstanding bug to resolve these intermittents, it is bug 1084288.
Depends on: 1107673
Depends on: 1107681
Blocks: 1057676
Blocks: 1111486
Blocks: 1114667
Depends on: 1116189
Blocks: 1111992
Blocks: Privacy
No longer blocks: 1114667
Depends on: 1114667
Status: REOPENED → NEW
blocking-b2g: --- → 2.2+
See Also: → 1111992
Dear Marta,
I am not sure Dave still working on this. Can you help to check this issue or anyone can help here?
Thank you!
Flags: needinfo?(marta)
NI? Huseby, not sure if he's working on it
Flags: needinfo?(marta) → needinfo?(huseby)
Comment on attachment 8530735 [details] [diff] [review]
Bug 1093688.patch

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

::: dom/geolocation/nsGeolocationSettings.cpp
@@ +350,5 @@
>  void
>  GeolocationSetting::HandleTypeChange(const JS::Value& aVal)
>  {
>    AutoJSAPI jsapi;
> +  jsapi.Init(&aVal.toObject());

This doesn't make sense. One of isObject and isString will be false; you cannot convert between them indiscriminately. It should assert in debug builds.

@@ +428,5 @@
>  void
>  GeolocationSetting::HandleFixedCoordsChange(const JS::Value& aVal)
>  {
>    AutoJSAPI jsapi;
> +  jsapi.Init(&aVal.toObject());

Same problem.
Dave, please could you finish this work as it is a 2.2 blocker please.
Flags: needinfo?(huseby)
Adding privacy keyword.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(pbylenga)
Keywords: privacy
Status: NEW → ASSIGNED
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(pbylenga)
Duplicate of this bug: 1118155
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][COM=Privacy Panel]
Hi Dave,
Do yu have any update for the bug? Thanks!
I just sent out patches for review for Bug 1115375 and Bug 1116189 and Bug 1114467 is waiting to be checked in.  The current build I have on my phone with all three of these patches has a working exceptions list.
Flags: needinfo?(huseby)
Dear Gerry,
Could yo help to check whether this issue still happen after Bug 111537, Bug 1116189 and Bug 1114467 all fixed? Thanks!
Flags: needinfo?(gchang)
Check on below build. 
Gaia-Rev        389542b71c89253c0d176d3b0bfb54e275c19bf1
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/9fd3441c8983
Build-ID        20150223002503
Version         37.0a2
Device-Name     flame
FW-Release      4.4.2


1. I add "Geoloc" app in exception list. It seems to work. But, when I want to add another app in exception list, I found another issue.
The app list in "Add exceptions" will become only 1 app. The rest apps disappear and I don't have any ways to bring them back but reset the whole device.

Please see the URL.
http://youtu.be/Bjj84aLSnyk

2. Because bug 1118155 is marked as duplicated as this bug, I also check the problem in bug 1118155. I still can't locate position in google map.
Flags: needinfo?(gchang) → needinfo?(huseby)
Per second finding in comment #32, the custom location can work now. The location can reflect to what I change.
I tried today's build and custom location can work.

Gaia-Rev        ca64f2fe145909f31af266b1730874051ba76c78
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/16804008c29f
Build-ID        20150224162516
Version         37.0
Device-Name     flame
FW-Release      4.4.2
I am now seeing that the exceptions list is broken with the latest code.  I'm seeing two things:

1. when I first go to the exceptions list, it properly lists all of the apps.  then I set an exception for geoloc.  then I close the settings app.  when I open the settings app and go back to the exceptions list, it only shows the camera app.  this is a problem with the privacy panel and maybe with gaia.

2. if I set a radius for an app exception, regardless of what I set, it treats it like I set the radius to 0, which is the same as "precise location".
Flags: needinfo?(huseby) → needinfo?(marta)
Dear Marta,
Could you please help to check comment 35 from Dave?
Thanks!
Severity: normal → major
Priority: -- → P1
I'm looking at it
Flags: needinfo?(marta)
Comment on attachment 8579361 [details] [review]
[gaia] martasect:Bug_1093688 > mozilla-b2g:master

Dave could you look if that solves the problem?
Attachment #8579361 - Flags: feedback?(huseby)
Hi Dave,
Could you help to check comment 39 from Marta? Thanks!
Flags: needinfo?(huseby)
I'll try to take a look today.
I just tested this with the latest v2.2 and marta's patch.  The exception list works great now.  The only thing broken is fuzzed locations.  Precise, no location, fixed location, and global settings all work.  I'll debug this ASAP.
Flags: needinfo?(huseby)
Now I'm seeing the exceptions list broken again and fuzzing doesn't work at all. :-/
Removed from 2.2 blocker as Privacy Control will not be FxOS 2.2 committed feature.
blocking-b2g: 2.2+ → ---
See Bug 1172129
Status: ASSIGNED → RESOLVED
Closed: 6 years ago5 years ago
Resolution: --- → WONTFIX
Comment on attachment 8579361 [details] [review]
[gaia] martasect:Bug_1093688 > mozilla-b2g:master

currently rethinking all of this code.
Attachment #8579361 - Flags: feedback?(huseby) → feedback-
You need to log in before you can comment on or make changes to this bug.