Closed Bug 1130570 Opened 5 years ago Closed 5 years ago

trained-to-thrill Service Worker registration throws DOMException in e10s mode

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
e10s + ---
firefox38 --- fixed

People

(Reporter: bkelly, Assigned: nsm)

Details

Attachments

(3 files, 1 obsolete file)

Testing with our current best patches in this queue:

  https://github.com/wanderview/gecko-patches/tree/serviceworkers

In non-e10s mode the Service Worker successfully registers.  In e10s mode, though, we get a DOMException.
Assignee: nobody → nsm.nikhil
Status: NEW → ASSIGNED
Waiting for b2g tests to be green https://treeherder.mozilla.org/#/jobs?repo=try&revision=75fd40f7be86
This problem for e10s does not arise in treeherder because we run other worker tests so we have a runtime service around.
It does arise in e10s on individual examples as bkelly pointed out. On b2g we seem to be always running into it.
Attachment #8563015 - Flags: review?(amarchesini)
Comment on attachment 8563015 [details] [diff] [review]
Ensure we have a valid runtime service, and clear updating scopes on early return

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

::: dom/workers/ServiceWorkerManager.cpp
@@ +2006,5 @@
>      return rv;
>    }
>  
>    nsRefPtr<ServiceWorker> serviceWorker;
> +  RuntimeService* rs = RuntimeService::GetOrCreateService();

we already have this, right?
Attachment #8563015 - Flags: review?(amarchesini) → review+
Attached patch iframe vs window.open (obsolete) — Splinter Review
Another reason why we fail in b2g desktop/emulator is that we don't have the support for window.open when the code runs into an app. Here we use an iframe.
Attachment #8563306 - Flags: review?(nsm.nikhil)
Comment on attachment 8563306 [details] [diff] [review]
iframe vs window.open

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

::: dom/workers/test/serviceworkers/mochitest.ini
@@ +1,2 @@
>  [DEFAULT]
> +skip-if = android_version == "10" # bug 1056702

FYI, I removed this in my patches since we use 11 on treeherder now. They pass on 11, not sure if we still need this.
Attachment #8563306 - Flags: review?(nsm.nikhil) → review+
Do you want to land my patch with yours?
Attachment #8563306 - Attachment is obsolete: true
Flags: needinfo?(nsm.nikhil)
Yes, about to push to try.
Flags: needinfo?(nsm.nikhil)
also needs Bug 1130065 before this can land.
Ugh, I'm not sure why we continue to have b2g failures. baku, can you give this a look since you are currently surrounded by b2g experts? Thanks!
Flags: needinfo?(amarchesini)
sorry i had to to back this out in https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=491e81cd9cf8 - seems one of this changes was causing on multiple desktop platforms test failures like 

https://treeherder.mozilla.org/logviewer.html#?job_id=6670670&repo=mozilla-inbound

that became frequent to perma failures starting with this csets. Could you take a look at this, thanks!
https://hg.mozilla.org/mozilla-central/rev/71df4e762c58
https://hg.mozilla.org/mozilla-central/rev/1b86fdc1e49f
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
Flags: needinfo?(amarchesini)
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.