1169819 - add browser-chrome service worker test to validate refreshing

Status: RESOLVED FIXED

(Core :: DOM: Service Workers, defect)

Description

[Ben Kelly [:bkelly, not reviewing]]

Attachment: work-in-progress

I started a browser-chrome refresh test for bug 1157619, but ended up use an iframe mochitest instead.  The browser-chrome test revealed some assertions could be hit, though, so we should probably add it anyway.

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Add browser chrome test to validate SW force refresh. r=ehsan

Comment 2

[(no longer active)]

Comment on attachment 8620375 [details] [diff] [review]
Add browser chrome test to validate SW force refresh. r=ehsan

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

::: dom/workers/test/serviceworkers/browser.ini
@@ +4,5 @@
> +  browser_cached_force_refresh.html
> +  browser_force_refresh_worker.js
> +
> +[browser_force_refresh.js]
> +skip-if = e10s

Do you mind filing a bug for enabling this outside of e10s and put it on v3?  Thanks!

::: dom/workers/test/serviceworkers/browser_force_refresh.js
@@ +22,5 @@
> +    var url = gTestRoot + 'browser_base_force_refresh.html';
> +    var tab = gBrowser.addTab(url);
> +    gBrowser.selectedTab = tab;
> +
> +    ok(true);

Why this?

::: dom/workers/test/serviceworkers/browser_force_refresh_worker.js
@@ +1,1 @@
> +var name = 'browserRefresherCache';

Nit: please rename this so that it doesn't begin with "browser_" (that's the convention we use for test names in browser-chrome, although with test manifests it probably doesn't matter much in practice.)

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

> ::: dom/workers/test/serviceworkers/browser.ini
> > +skip-if = e10s
> 
> Do you mind filing a bug for enabling this outside of e10s and put it on v3?
> Thanks!

Actually, it works in e10s.  I think I added this when I was trying to use postMessage instead of events.

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=97df902c48d0

Comment 5

[Pulsebot]

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

Comment 6

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

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/dace20e4271c

Comment 7

[Ben Kelly [:bkelly, not reviewing]]

I requested this backout because I pushed stale patches without the review feedback addressed.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7b23ff6ac3a2

Comment 9

[Phil Ringnalda (:philor)]

Backed out again in https://hg.mozilla.org/integration/mozilla-inbound/rev/91a848ffec69 because one bug or the other caused the same https://treeherder.mozilla.org/logviewer.html#?job_id=10697453&repo=mozilla-inbound timeouts on OS X as it did the first time they landed.

Comment 10

[Ben Kelly [:bkelly, not reviewing]]

I think this is due to mac using the OS command key instead of ctrl for refreshing.

Comment 11

[Ben Kelly [:bkelly, not reviewing]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=304d3497099f

Comment 12

[(no longer active)]

(In reply to Ben Kelly [:bkelly] from comment #10)
> I think this is due to mac using the OS command key instead of ctrl for
> refreshing.

Oh, yes.  Sorry, my bad for not catching it.

Comment 13

[Ben Kelly [:bkelly, not reviewing]]

Attachment: Add browser chrome test to validate SW force refresh. r=ehsan

Use accelKey instead of ctrlKey in order to support mac.

Comment 14

[Pulsebot]

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

Comment 15

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/6f9177ab99d1