1215201 - callback is null in ext-alarms.

Status: RESOLVED FIXED

(WebExtensions :: Untriaged, defect)

Description

[Blake Winton (:bwinton) (:☕️)]

So, I'm digging in to why my alarms aren't firing, and I came across this.
It looks like we should be using for…of instead of for…in.

Comment 1

[Blake Winton (:bwinton) (:☕️)]

Attachment: The first attempt.

(I even added tests!  ;)

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8674395 [details] [diff] [review]
The first attempt.

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

Thanks!

::: toolkit/components/extensions/test/mochitest/test_ext_alarms.html
@@ +13,5 @@
> +<script type="application/javascript;version=1.8">
> +
> +add_task(function* test_contentscript() {
> +  function backgroundScript() {
> +    browser.test.log('running alarm script');

Please use double quotes for all string literals.

@@ +22,5 @@
> +    });
> +    chrome.alarms.create('test_ext_alarms', {delayInMinutes: 0.02});
> +    setTimeout(() => {
> +      browser.test.notifyFail('alarms test failed, took too long');
> +    }, 3000);

This doesn't give us a lot of margin for error on slow test machines. Please make this 10 seconds or remove it entirely.

Comment 3

[Blake Winton (:bwinton) (:☕️)]

Done and done.

I also changed the:
  add_task(function* test_contentscript() {
line to
  add_task(function* test_alarm_fires() {
cause we're not really testing the contentscript here.
(It looks like that line was copied and pasted in a bunch of files, but I don't think it's worth cleaning up in this patch… :)

Comment 4

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b0fc271e4897

Comment 5

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/b0fc271e4897