963258 - Alarm API throws exception when enabled in Firefox

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Alexandre Poirot [:ochameau]]

In Firefox Mulet (more info in bug 943878), the code from the Alarm API throws over here:
  http://mxr.mozilla.org/mozilla-central/source/dom/alarm/AlarmsManager.js#184
Because of `principal.URI` being undefined.
It throws and breaks the webconsole. It throws when the console tries to autocomplete.
It's not really clear why we fetch a the page URI from the principal whereas we have window/document objects. Also I don't know what we do exactly with this pageURL value.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Prevent exception in the webconsole in alarm code

But I know something! That patch fix this exception and make the console work again on the Mulet!

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 8364614 [details] [diff] [review]
Prevent exception in the webconsole in alarm code

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

Gene is probably a better reviewer :)

Comment 3

[Gene Lian (I already quit Mozilla)]

Comment on attachment 8364614 [details] [diff] [review]
Prevent exception in the webconsole in alarm code

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

Please make sure it's still working on the device and the try server before landing. Thanks! r=gene

Comment 4

[Alexandre Poirot [:ochameau]]

Attachment: Use window location instead of origin

Using the principal origin was a bad idea as it will be something like:
  app://alarm.gaiamobile.org
instead of
  app://alarm.gaiamobile.org/index.html

In the particular case of the builtin alarm app, it shouldn't make much difference.
But if an app registers from an html document other than index.html, it may break.

A try run:
https://tbpl.mozilla.org/?tree=Try&rev=7e66e021fe56

Gene, I tried it on my device but I wasn't able to dismiss the
attention screen. If I push Close button, the ring stops but
the alarm screen stays and then I can't kill the alarm app.

But it doesn't seem to be related to my patch?

(I'm testing on last gecko/gaia master, but I may have something wrong in my environmenent...)

Comment 5

[Gene Lian (I already quit Mozilla)]

(In reply to Alexandre Poirot (:ochameau) from comment #4)
> Created attachment 8365982 [details] [diff] [review]
> Use window location instead of origin
> 
> Using the principal origin was a bad idea as it will be something like:
>   app://alarm.gaiamobile.org
> instead of
>   app://alarm.gaiamobile.org/index.html

Yeap! We have to make sure it's the app://alarm.gaiamobile.org/index.html. Otherwise, the system message cannot be delivered to the correct page.

> 
> In the particular case of the builtin alarm app, it shouldn't make much
> difference.
> But if an app registers from an html document other than index.html, it may
> break.
> 
> A try run:
> https://tbpl.mozilla.org/?tree=Try&rev=7e66e021fe56
> 
> Gene, I tried it on my device but I wasn't able to dismiss the
> attention screen. If I push Close button, the ring stops but
> the alarm screen stays and then I can't kill the alarm app.
> 
> But it doesn't seem to be related to my patch?

Sounds not related to the patch. The page URL only determine which page to deliver the system message to:

  _fireSystemMessage: function _fireSystemMessage(aAlarm) {
    debug("Fire system message: " + JSON.stringify(aAlarm));

    let manifestURI = Services.io.newURI(aAlarm.manifestURL, null, null);
    let pageURI = Services.io.newURI(aAlarm.pageURL, null, null);

    messenger.sendMessage("alarm", this._publicAlarm(aAlarm),
                          pageURI, manifestURI);
  },

If the attention screen shows up, it means the system message has been delivered and the alarm has been fired.

Have you try to exclude your patch to see if this issue still happens?

> 
> (I'm testing on last gecko/gaia master, but I may have something wrong in my
> environmenent...)

I'll wait for you experiment to decide the review, although the patch seems to be working (has to be app://alarm.gaiamobile.org/index.html).

Comment 6

[Alexandre Poirot [:ochameau]]

Yes I tried without my patch and have seen the same breakage with dismiss buttons.
I'll try to find someone with 1.3 to see if I'm the only one facing this issue.

Comment 7

[Alexandre Poirot [:ochameau]]

s/1.3/master/

Comment 8

[Alexandre Poirot [:ochameau]]

It looks like it was another patch in my queue that broke it. It works fine now.
(I'm working on Firefox mulet and have various quite significant patch applied)

Comment 9

[Gene Lian (I already quit Mozilla)]

Comment on attachment 8365982 [details] [diff] [review]
Use window location instead of origin

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

Please go ahead to land (supposing this won't break the function on the device). Thank you for all the work! :)

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/72da2b2f029c

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/72da2b2f029c

Comment 12

[Gene Lian (I already quit Mozilla)]

This one causes the regression at bug 980682.