1037030 - Followup to bug 1024513

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Runtime, defect)

Description

[[:fabrice] Fabrice Desré]

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.

Comment 1

[[:fabrice] Fabrice Desré]

Attachment: gecko patch

The real issue was on the gaia side, but we were not very robust on the gecko side either.

Comment 2

[[:fabrice] Fabrice Desré]

Attachment: gaia patch

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.

Comment 3

[Gregor Wagner [:gwagner]]

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?

Comment 4

[[:fabrice] Fabrice Desré]

(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 5

[(inactive after 6/18) Alive Kuo [:alive]]

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.

Comment 6

[(inactive after 6/18) Alive Kuo [:alive]]

Fred, please let me know if you have time to investigate since you are taking care permission manager now.

Comment 7

[[e-mail]]

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.

Comment 8

[[:fabrice] Fabrice Desré]

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

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Backed out for mochitest failures.
https://hg.mozilla.org/integration/b2g-inbound/rev/ae4192ba2824

https://tbpl.mozilla.org/php/getParsedLog.php?id=43760187&tree=B2g-Inbound

Comment 10

[[:fabrice] Fabrice Desré]

fixed at https://tbpl.mozilla.org/?tree=Try&rev=e7073f9ba795

https://hg.mozilla.org/integration/b2g-inbound/rev/6ac0f97c0c09

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out for Gu failures: https://tbpl.mozilla.org/php/getParsedLog.php?id=43773875&tree=B2g-Inbound

https://hg.mozilla.org/integration/b2g-inbound/rev/2b340d6bf8ff

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to Wes Kocher (:KWierso) from comment #11)
> Backed out for Gu failures:
> https://tbpl.mozilla.org/php/getParsedLog.php?id=43773875&tree=B2g-Inbound
> 
> https://hg.mozilla.org/integration/b2g-inbound/rev/2b340d6bf8ff

There was also this mochitest failure: https://tbpl.mozilla.org/php/getParsedLog.php?id=43777201&tree=B2g-Inbound

Comment 13

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/integration/b2g-inbound/rev/8d70f13112e2

green try at https://tbpl.mozilla.org/?tree=Try&rev=c190d6dec7d3

Comment 14

[[:fabrice] Fabrice Desré]

Gaia part on master: https://github.com/mozilla-b2g/gaia/commit/4e6422d86cdf4d11dd5f5d36963bd32f465416ea

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8d70f13112e2

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/69030cc2f178

v2.0: https://github.com/mozilla-b2g/gaia/commit/8cb1a949f2e9650bb2c5598e78a6f24a58bbaf97