add browser-chrome service worker test to validate refreshing

RESOLVED FIXED in Firefox 41

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: bkelly, Assigned: bkelly)

Tracking

Trunk
mozilla41
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

Posted file work-in-progress (obsolete) —
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.
Depends on: 1173361
Attachment #8613125 - Attachment is obsolete: true
Attachment #8620375 - Flags: review?(ehsan)
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.)
Attachment #8620375 - Flags: review?(ehsan) → review+
> ::: 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.
I requested this backout because I pushed stale patches without the review feedback addressed.
Flags: needinfo?(bkelly)
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.
I think this is due to mac using the OS command key instead of ctrl for refreshing.
(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.
Use accelKey instead of ctrlKey in order to support mac.
Attachment #8620375 - Attachment is obsolete: true
Attachment #8621793 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/6f9177ab99d1
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
You need to log in before you can comment on or make changes to this bug.