Support periodInMinutes key for chrome.alarms.

RESOLVED FIXED in Firefox 44


4 years ago
11 months ago


(Reporter: bwinton, Assigned: bwinton)


Dependency tree / graph
Bug Flags:
blocking-webextensions +

Firefox Tracking Flags

(firefox44 fixed)


(Whiteboard: [alarms])


(1 attachment)

Currently, we pass the periodInMinutes down into the Alarm object, but entirely ignore it when setting the scheduledTime (as shown in )  :)

I am a little surprised that the "timer.init(this, delay, Ci.nsITimer.TYPE_ONE_SHOT);" call doesn't complain when getting NaN as the delay…

Johann: Did you want to try fixing this one, or should I give it a shot?


4 years ago
Whiteboard: [alarms]
The documentation here is really weird. Here's what the Chrome docs say:

"Describes when the alarm should fire. The initial time must be specified by either when or delayInMinutes (but not both). If periodInMinutes is set, the alarm will repeat every periodInMinutes minutes after the initial event. If neither when or delayInMinutes is set for a repeating alarm, periodInMinutes is used as the default for delayInMinutes."

The second sentence implies that one of |when| or |delayInMinutes| must be defined. But then the last sentence suggests that isn't the case. I guess the bug happens if they're both undefined?
Yeah, that's when the bug happens.  Chrome fires as per the second sentence.  Also, I think the fix will be pretty easy, but I'm not sure how to run multiple tests from the test_ext_alarms.html in bug 1215201.  Do I need to put everything into Promises?
No, you can add a separate add_task(...) block. It will run after the first one has finished.
(In reply to Bill McCloskey (:billm) from comment #3)
> No, you can add a separate add_task(...) block. It will run after the first
> one has finished.

Oh, cool!  Okay, here we go with a little refactoring of the previous test.  Lemme know what should be changed.  Thanks!  :D
Assignee: nobody → bwinton
Attachment #8675005 - Flags: review?(wmccloskey)


4 years ago
Blocks: webext
Comment on attachment 8675005 [details] [diff] [review]
The first attempt.

Review of attachment 8675005 [details] [diff] [review]:

::: toolkit/components/extensions/test/mochitest/test_ext_alarms.html
@@ +18,2 @@
>      browser.test.log("running alarm script");
> +    chrome.alarms.clearAll();

This shouldn't be needed. It will only clear the alarms associated with this extension, and there aren't any yet. If you're having problems with alarms firing after the test ends, we should investigate more.

@@ +47,5 @@
> +add_task(function* test_periodic_alarm_fires() {
> +  function backgroundScript() {
> +    var ALARM_NAME = "test_ext_alarms";
> +    browser.test.log("running alarm script");
> +    chrome.alarms.clearAll();

Same here.
Attachment #8675005 - Flags: review?(wmccloskey) → review+
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44


4 years ago
Flags: blocking-webextensions+


11 months ago
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.