Integrate service worker manager thread by creativcoder · Pull Request #11727 · servo/servo

creativcoder · 2016-06-12T08:36:52Z

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes are part of Implement Service Workers API #11091

There are tests for these changes at my gh-pages branch to test the instantiation of service workers by their manager, but will need to discuss how that would integrate into master.

Changes:

Introduces a ServiceWorkerManager, which maintains an map of registered service workers as well as a map of active workers keyed by their scope_url.
Adds the initialization of ServiceWorkerManager, at the script::init(), which makes it available as a single entity listening for requests from different script threads.
Adds a timeout thread in serviceworkerglobalscope, which terminates the workers, after a timeout of 60 secs, thereby removing it from the active workers list.
Adds the matching of scope urls, in longest prefix way rather than path structural way, according to spec.
Make ServiceWorkerManager, the holder of network sender, instead of script thread, so it can send CustomResponse.

This change is

highfive · 2016-06-12T08:36:54Z

Heads up! This PR modifies the following files:

@asajeffrey: components/constellation/pipeline.rs, components/constellation/constellation.rs
@KiChjang: components/script/dom/worker.rs, components/script/lib.rs, components/script/serviceworker_manager.rs, components/script/dom/serviceworkerregistration.rs, components/script/dom/serviceworker.rs, components/script/Cargo.toml, components/script/dom/workerglobalscope.rs, components/script/dom/dedicatedworkerglobalscope.rs, components/script_traits/script_msg.rs, components/script_traits/script_msg.rs, components/script_traits/lib.rs, components/script_traits/lib.rs, components/script/script_thread.rs, components/script/dom/abstractworker.rs, components/script/dom/serviceworkerglobalscope.rs

highfive · 2016-06-12T08:36:55Z

Warning

These commits modify script code, but no tests are modified. Please consider adding a test!

creativcoder · 2016-06-12T08:37:13Z

@jdm r?

creativcoder · 2016-06-12T08:41:24Z

components/script/dom/serviceworkerglobalscope.rs

@@ -4,75 +4,43 @@



@jdm As suggested about serviceworkerglobalscopes, not having the parent sender, the worker instance, and the parent runtime, i have removed the code, which had interactions with them. But will need advice on the run_with_memory_reporting thing when listening for events, which needs a parent_sender. I think, at the very minimum, we will need a ServiceWorker object instance, to facilitate dispatching of events to them, but how would i send a ServiceWorker object to service worker manager from the script_thread ? I am open to improving this PR. Thoughts ?

creativcoder · 2016-06-12T09:06:15Z

The build succeeded but travis gives this The command "bash etc/ci/check_no_unwrap.sh" exited with 1.

Ms2ger · 2016-06-12T16:00:06Z

r? @jdm

jdm · 2016-06-12T16:34:50Z

Check_no_unwrap fails if there are unwrap calls detected in constellation code.

highfive · 2016-06-12T19:15:13Z

New code was committed to pull request.

bors-servo · 2016-06-15T00:23:33Z

☔ The latest upstream changes (presumably #11745) made this pull request unmergeable. Please resolve the merge conflicts.

creativcoder · 2016-06-17T18:28:11Z

@jdm I removed the need for sending navigation message from constellation to script thread. You were right, it was redundant. Instead, am matching for the load_url, when the script thread has to interact with network sender, and during that match I am sending, the the ScopeThings to constellation, which further sends it to SW-Manager, to activate the respective worker.
I have a question:
For the origin http://localhost:8000, navigating to http://localhost:8000/dashboard-page.html, by clicking a button, will reuse the same script thread which was created for http://localhost:8000; right ?

jdm · 2016-06-17T18:41:33Z

Same-origin navigation do not reuse the script thread right now, unfortunately.

creativcoder · 2016-06-17T18:53:11Z

@jdm okay, so that's the reason, why manual click registering a sw for http://localhost:8000/dashboard-page.html, and then navigating to that url, does not activate the sw-dashboard.js, because the registration was stored in previous script thread.

jdm · 2016-06-17T18:55:53Z

This is why we need the actual registration data (note: orthogonal to the JS object) to be stored in the SW manager.

creativcoder · 2016-06-23T07:13:58Z

@jdm I made the changes. The ScopeThings is stored in SW-manager, but then again, we have to remove its entry in manager when passing it to run_serviceworker_scope, since it cannot be cloned (as there is a devtools receiver there) and i wonder, at what place the devtools_receiver could be decoupled from ScopeThings. It is hooked to the WorkerGlobalScopeInit Suggestions ?

bors-servo · 2016-06-23T16:50:58Z

☔ The latest upstream changes (presumably #11735) made this pull request unmergeable. Please resolve the merge conflicts.

bors-servo · 2016-07-12T17:51:35Z

💔 Test failed - mac-rel-wpt

creativcoder · 2016-07-13T03:44:02Z

Test logs shows this (60 tests timeout):

▶ TIMEOUT [expected PASS] /_mozilla/css/borders_a.html
  │ 
  └ Shutting down the Constellation after generating an output file or exit flag specified

  ▶ TIMEOUT [expected PASS] /_mozilla/css/canvas_radial_gradient_a.html
  │ 
  └ Shutting down the Constellation after generating an output file or exit flag specified

  ▶ TIMEOUT [expected PASS] /_mozilla/css/data_img_a.html
  │ 
  └ Shutting down the Constellation after generating an output file or exit flag specified

and this ( there are more instances of this kind of Timeout)

▶ TIMEOUT [expected PASS] /_mozilla/mozilla/canvas/drawimage_html_image_1.html
  │ 
  │ updating composition_request (NewFrameTree)
  │ not ready to composite: NotReadyToPaintImage(JustNotifiedConstellation)
  │ got ready to save image query, result is DocumentLoading
  │ sent response
  │ resetting ready_to_save_state!
  │ composition_request is already CompositeNow(NewFrameTree)
  │ not ready to composite: NotReadyToPaintImage(JustNotifiedConstellation)
  │ got ready to save image query, result is DocumentLoading
  │ sent response
  │ resetting ready_to_save_state!
  │ composition_request is already CompositeNow(NewFrameTree)
  │ got ready to save image query, result is DocumentLoading
  │ not ready to composite: NotReadyToPaintImage(JustNotifiedConstellation)
  │ sent response
  │ resetting ready_to_save_state!
  │ composition_request is already CompositeNow(NewFrameTree)
  │ not ready to composite: NotReadyToPaintImage(JustNotifiedConstellation)
  │ got ready to save image query, result is DocumentLoading
  │ sent response
  │ resetting ready_to_save_state!
  │ composition_request is already CompositeNow(NewFrameTree)
  │ not ready to composite: NotReadyToPaintImage(JustNotifiedConstellation)
  │ got ready to save image query, result is DocumentLoading
  │ sent response
  │ resetting ready_to_save_state!
  │ composition_request is already CompositeNow(NewFrameTree)
  │ not ready to composite: NotReadyToPaintImage(JustNotifiedConstellation)
  │ got ready to save image query, result is DocumentLoading
  │ sent response
  │ resetting ready_to_save_state!
  │ composition_request is already CompositeNow(NewFrameTree)
  │ not ready to composite: NotReadyToPaintImage(JustNotifiedConstellation)
  │ got ready to save image query, result is DocumentLoading
  │ sent response
  │ resetting ready_to_save_state!
  │ composition_request is already CompositeNow(NewFrameTree)
  │ not ready to composite: NotReadyToPaintImage(JustNotifiedConstellation)
  │ got ready to save image query, result is DocumentLoading
  │ sent response
  │ resetting ready_to_save_state!
  │ composition_request is already CompositeNow(NewFrameTree)
  │ not ready to composite: NotReadyToPaintImage(JustNotifiedConstellation)
  │ got ready to save image query, result is DocumentLoading
  │ sent response
  │ resetting ready_to_save_state!
  │ composition_request is already CompositeNow(NewFrameTree)
  │ not ready to composite: NotReadyToPaintImage(JustNotifiedConstellation)
  │ got ready to save image query, result is DocumentLoading
  │ sent response

@jdm Could that be an intermittent ? Most Timeouts appear to happen when rendering images or canvas related tests. Full Log

jdm · 2016-07-13T10:06:00Z

Have you run those tests locally? I suspect it's a reproducible problem.

bors-servo · 2016-07-15T19:24:00Z

☔ The latest upstream changes (presumably #12441) made this pull request unmergeable. Please resolve the merge conflicts.

jdm · 2016-07-17T18:16:54Z

I did some debugging and figured out the problem. When we're running reftests (ie. using --output foo.png or --exit), we block on retrieving image data in the layout thread (https://dxr.mozilla.org/servo/source/components/layout/context.rs#136). Meanwhile, the compositor asks the constellation if it's time to take a screenshot, so the constellation sends a synchronous query to the layout thread to get some information about what the most recent operation was (https://dxr.mozilla.org/servo/source/components/constellation/constellation.rs#2043). Meanwhile, the HTTP loader thread for the image sends a message to the constellation to ensure that any service worker is properly handled, but that message sits in a queue until after the constellation is finished dealing with the request from the compositor.

We've got a deadlock between the layout, constellation, and HTTP loader threads in this case. The easiest way to break it is to make the HTTP loader communicate directly with the service worker manager, rather than passing through the constellation as an intermediary.

creativcoder · 2016-07-18T13:42:01Z

@jdm Oh, great. Thanks for giving time in investigating this. Updated with the required changes, and tested the wpt-tests locally and there are no timeouts now. Let me know if the latest commit looks good to you.

jdm · 2016-07-18T16:16:12Z

@bors-servo: r+

bors-servo · 2016-07-18T16:16:14Z

📌 Commit eff3e01 has been approved by jdm

bors-servo · 2016-07-18T16:50:59Z

⌛ Testing commit eff3e01 with merge 513811f...

bors-servo · 2016-07-18T17:53:48Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows

highfive assigned Ms2ger Jun 12, 2016

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jun 12, 2016

creativcoder reviewed Jun 12, 2016
View reviewed changes

highfive assigned jdm and unassigned Ms2ger Jun 12, 2016

creativcoder force-pushed the swmanager branch from e076765 to ca850ee Compare June 12, 2016 19:15

highfive added the S-needs-rebase There are merge conflict errors. label Jun 15, 2016

creativcoder force-pushed the swmanager branch 3 times, most recently from f8a337c to f794ad4 Compare June 17, 2016 17:24

creativcoder force-pushed the swmanager branch 3 times, most recently from c398863 to d88d3b3 Compare June 22, 2016 18:38

highfive added the S-needs-rebase There are merge conflict errors. label Jun 23, 2016

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jul 12, 2016

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jul 12, 2016

highfive added the S-needs-rebase There are merge conflict errors. label Jul 15, 2016

Integrate service worker manager thread

1e6293e

creativcoder force-pushed the swmanager branch from b43f5a3 to 1e6293e Compare July 16, 2016 18:01

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Jul 16, 2016

jdm added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jul 17, 2016

make resource_thread talk to sw-manager

eff3e01

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Jul 18, 2016

highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Jul 18, 2016

creativcoder mentioned this pull request Aug 17, 2016

Implement Service Workers API #11091

Open

8 tasks

Uh oh!

Conversation

creativcoder commented Jun 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

highfive commented Jun 12, 2016

Uh oh!

highfive commented Jun 12, 2016

Uh oh!

creativcoder commented Jun 12, 2016

Uh oh!

creativcoder Jun 12, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

creativcoder commented Jun 12, 2016

Uh oh!

Ms2ger commented Jun 12, 2016

Uh oh!

jdm commented Jun 12, 2016

Uh oh!

highfive commented Jun 12, 2016

Uh oh!

bors-servo commented Jun 15, 2016

Uh oh!

creativcoder commented Jun 17, 2016

Uh oh!

jdm commented Jun 17, 2016

Uh oh!

creativcoder commented Jun 17, 2016

Uh oh!

jdm commented Jun 17, 2016

Uh oh!

creativcoder commented Jun 23, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bors-servo commented Jun 23, 2016

Uh oh!

bors-servo commented Jul 12, 2016

Uh oh!

creativcoder commented Jul 13, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jdm commented Jul 13, 2016

Uh oh!

bors-servo commented Jul 15, 2016

Uh oh!

jdm commented Jul 17, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

creativcoder commented Jul 18, 2016

Uh oh!

jdm commented Jul 18, 2016

Uh oh!

bors-servo commented Jul 18, 2016

Uh oh!

bors-servo commented Jul 18, 2016

Uh oh!

bors-servo commented Jul 18, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

creativcoder commented Jun 12, 2016 •

edited

Loading

creativcoder Jun 12, 2016 •

edited

Loading

creativcoder commented Jun 23, 2016 •

edited

Loading

creativcoder commented Jul 13, 2016 •

edited

Loading

jdm commented Jul 17, 2016 •

edited

Loading