Closed Bug 1037030 Opened 5 years ago Closed 5 years ago

Followup to bug 1024513

Categories

(Firefox OS Graveyard :: Runtime, defect)

All
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:2.0+, firefox31 wontfix, firefox32 fixed, firefox33 fixed, b2g-v2.0 fixed, b2g-v2.1 fixed)

RESOLVED FIXED
2.1 S1 (1aug)
blocking-b2g 2.0+
Tracking Status
firefox31 --- wontfix
firefox32 --- fixed
firefox33 --- fixed
b2g-v2.0 --- fixed
b2g-v2.1 --- fixed

People

(Reporter: fabrice, Assigned: fabrice)

References

Details

(Keywords: qablocker, regression, Whiteboard: [xfail])

Attachments

(2 files)

We still have issues with b2g desktop builds:

We attempted enabling our tests on Travis today but struck problems.

I debugged it and found this error message in the console:
Error: perm.options is null
Source File: file:///home/zac/Mozilla/gaia/b2g/components/ContentPermissionPrompt.js
Line: 28

This is on TBPL mozilla-central desktopb2g builds.

STR are:
1. Download build from http://ftp.mozilla.org/pub/mozilla.org/b2g/tinderbox-builds/mozilla-central-linux64_gecko/1405001967/
2. make a gaia profile from master branch: "make"
3. Load ./b2g-bin -profile /path/to/gaia/profile/
4. Complete FTU
5. Open "UI tests - privileged" app
6. Tap Contacts
7. Tap Insert fake contacts
8. Note error message.
Blocks: 1024513
No longer depends on: 1024513
Assignee: nobody → fabrice
blocking-b2g: --- → 2.0+
Attached patch gecko patchSplinter Review
The real issue was on the gaia side, but we were not very robust on the gecko side either.
Attachment #8454268 - Flags: review?(anygregor)
Attached patch gaia patchSplinter Review
Alive, what I see happening is that when we have a pending prompt in queue, showNextPendingRequest() would be called before this.responseStatus has a chance to be set, and we end up sending a undefined type. Bad things then happened on the gecko side! The strange part is that I could only reproduce the issue on b2g desktop but not on device.
Attachment #8454270 - Flags: review?(alive)
Comment on attachment 8454268 [details] [diff] [review]
gecko patch

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

::: dom/contacts/ContactManager.js
@@ +277,5 @@
> +      cancel: aCancelCallback ||
> +              function() {
> +                if (DEBUG)
> +                  debug("Default cancel contacts callback. " + access +"\n");
> +              },

Why is this needed? Is this actually causing problems or just to be conservative?
Attachment #8454268 - Flags: review?(anygregor) → review+
(In reply to Gregor Wagner [:gwagner] from comment #3)
> Comment on attachment 8454268 [details] [diff] [review]
> gecko patch
> 
> Review of attachment 8454268 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/contacts/ContactManager.js
> @@ +277,5 @@
> > +      cancel: aCancelCallback ||
> > +              function() {
> > +                if (DEBUG)
> > +                  debug("Default cancel contacts callback. " + access +"\n");
> > +              },
> 
> Why is this needed? Is this actually causing problems or just to be
> conservative?

I saw it causing problems when the c++ code calls an undefined cancel() method. That makes xpconnect unhappy.
Comment on attachment 8454270 [details] [diff] [review]
gaia patch

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

I cannot tell what's happening but I suspect it's a design problem.
I could r+ this at first and I will ask someone in gaia to find out the root cause afterwards.
Attachment #8454270 - Flags: review?(alive) → review+
Fred, please let me know if you have time to investigate since you are taking care permission manager now.
Flags: needinfo?(gasolin)
thanks for the patch. 

We did not set this.responseStatus param while dispatchResponse in showNextPendingRequest. The param is used to fill into `response` object which will be send to gecko 

event.initCustomEvent('mozContentEvent', response)

In general path the dispatchResponse(yes/no/discard) callback always come with explicit `'permission-allow/deny` type.
Flags: needinfo?(gasolin)
https://hg.mozilla.org/mozilla-central/rev/8d70f13112e2
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S1 (1aug)
You need to log in before you can comment on or make changes to this bug.