772369 - Alarm API - Follow-Up Fix for System Message Integration

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Gene Lian (I already quit Mozilla)]

Need to add sanity checks to avoid adding alarms when the app's manifestURL is null. Please see Fabrice's suggestion at https://bugzilla.mozilla.org/show_bug.cgi?id=749551#c130.

Comment 1

[Gene Lian (I already quit Mozilla)]

Attachment: WIP Patch

Fabrice,

May I invite you to be the reviewer of the System Message integration for Alarm APIs? This following summarizes what I've changed in this patch:

1. In the content process (AlarmsManager.js), the aWindow.document.nodePrincipal.URI.spec is saved and sent to parent process when adding alarms.

2. In the parent process (AlarmService.jsm), when alarm is fired, I used Services.io.newURI() to create a new manifestURI for sending system messages.

3. Add a sanity check to avoid adding alarms from a page without valid URI.

4. s/manifestURL/URISpec


Questions:

1. As discussed in the e-mail, it seems the the aManifestURI.spec passed into SystemMessageInternal.sendMessage() is still http://clock.gaiamobile.org/, instead of http://clock.gaiamobile.org/manifest.webapp as expected. Is it correct to use aWindow.document.nodePrincipal.URI.spec to get URI?

2. Although the aManifestURI.spec could be wrong, it seems the SystemMessageManager has never been started because the debug messages within the SystemMessageManager.init() didn't show (debug is already turned on). That's why SystemMessageManager.receiveMessage() cannot receive any messages sent from ppmm.sendAsyncMessage("SystemMessageManager:Message", ...). How should we correctly launch the SystemMessageManager?

Thanks,
Gene

Comment 2

[Gene Lian (I already quit Mozilla)]

Attachment: Patch

Fabrice,

May I invite you to be the reviewer of the System Message integration for Alarm APIs? The following summarizes what I've changed in this patch:

1. In the content process (AlarmsManager.js), the |utils.getApp().manifestURL| is saved and sent to parent process when adding alarms.

2. In the parent process (AlarmService.jsm), when alarm is fired, I used Services.io.newURI() to create a new manifestURI for sending system messages. Don't need to check if the manifestURL is null or not because of item #3.

3. Disable the Alarm APIs (i.e. set navigator.mozAlarms to null) when the page has no valid manifestURL, so that clients won't have chances to add an alarm that had no manifestURL.

Thanks,
Gene

Comment 3

[Gene Lian (I already quit Mozilla)]

Comment on attachment 642892 [details] [diff] [review]
Patch

Oops... This one cannot be passed by Mochitest which doesn't always use installed apps for testing. I'll upload another solution later. Sorry for bugging.

Gene

Comment 4

[Gene Lian (I already quit Mozilla)]

Attachment: Patch, V2

Fabrice,

May I invite you to be the reviewer of the System Message integration for Alarm APIs? The following summarizes what I've changed in this patch:

1. In the content process (AlarmsManager.js), the |utils.getApp().manifestURL| is saved and sent to parent process when adding alarms.

2. In the parent process (AlarmService.jsm), when alarm is fired, I used Services.io.newURI() to create a new manifestURI for sending system messages. Don't need to check if the manifestURL is null or not because of item #3.

3. Add an sanity check to avoid adding alarms for an non-installed app which can ensure all the added alarms are associated with an valid manifestURL for sending system messages.

Thanks,
Gene

Comment 5

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

http://hg.mozilla.org/integration/mozilla-inbound/rev/44e8a79e4be3

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/44e8a79e4be3