New issue
Advanced search Search tips

Issue 627034 link

Starred by 2 users
Status: Fixed
Owner:
kinuko@chromium.org
Closed: Oct 2016
Cc:
haraken@chromium.org
altimin@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 624689



Sign in to add a comment

Workers should have a way to post tasks to a per-frame scheduler

Project Member Reported by haraken@chromium.org, Jul 11 2016 
Context: We're moving as many tasks to per-frame schedulers as possible.

Currently workers post tasks to the document through MainThreadTaskRunner, which just uses the default task runner of the main thread. We should provide a way for workers to post tasks to the per-frame scheduler of the document associated with the worker.

Comment 1 by kinuko@chromium.org, Jul 21 2016

Status: Assigned (was: Untriaged)
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 29 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/66b07a326160fed57046fbb454f424a302d4cc6b

commit 66b07a326160fed57046fbb454f424a302d4cc6b
Author: kinuko <kinuko@chromium.org>
Date: Fri Jul 29 16:28:07 2016

Pass per-frame task runners to Workers (when possible)

- For in-process workers (i.e. dedicated/compositor workers): always use
  the associated document's task runners
- For out-of-process workers (i.e. service/shared workers): keep using the
  default task runner of the main thread, but via TaskRunnerHelper so
  we could still start using different task runners for different tasks
  (e.g. loading vs timer) if we want.

BUG= 627034 

Review-Url: https://codereview.chromium.org/2163983004
Cr-Commit-Position: refs/heads/master@{#408652}

[modify] https://crrev.com/66b07a326160fed57046fbb454f424a302d4cc6b/third_party/WebKit/Source/core/core.gypi
[modify] https://crrev.com/66b07a326160fed57046fbb454f424a302d4cc6b/third_party/WebKit/Source/core/workers/InProcessWorkerMessagingProxy.cpp
[modify] https://crrev.com/66b07a326160fed57046fbb454f424a302d4cc6b/third_party/WebKit/Source/core/workers/InProcessWorkerMessagingProxy.h
[modify] https://crrev.com/66b07a326160fed57046fbb454f424a302d4cc6b/third_party/WebKit/Source/core/workers/InProcessWorkerObjectProxy.cpp
[modify] https://crrev.com/66b07a326160fed57046fbb454f424a302d4cc6b/third_party/WebKit/Source/core/workers/InProcessWorkerObjectProxy.h
[add] https://crrev.com/66b07a326160fed57046fbb454f424a302d4cc6b/third_party/WebKit/Source/core/workers/ParentFrameTaskRunners.cpp
[add] https://crrev.com/66b07a326160fed57046fbb454f424a302d4cc6b/third_party/WebKit/Source/core/workers/ParentFrameTaskRunners.h
[modify] https://crrev.com/66b07a326160fed57046fbb454f424a302d4cc6b/third_party/WebKit/Source/web/WebEmbeddedWorkerImpl.cpp
[modify] https://crrev.com/66b07a326160fed57046fbb454f424a302d4cc6b/third_party/WebKit/Source/web/WebEmbeddedWorkerImpl.h
[modify] https://crrev.com/66b07a326160fed57046fbb454f424a302d4cc6b/third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp
[modify] https://crrev.com/66b07a326160fed57046fbb454f424a302d4cc6b/third_party/WebKit/Source/web/WebSharedWorkerImpl.h

Comment 3 by kinuko@chromium.org, Jul 29 2016

Status: Fixed (was: Assigned)
Project Member

Comment 4 by bugdroid1@chromium.org, Aug 1 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/043b1d5eb0223ec6cf40afac5cb591112a5006d7

commit 043b1d5eb0223ec6cf40afac5cb591112a5006d7
Author: tzik <tzik@chromium.org>
Date: Mon Aug 01 08:58:32 2016

Revert of Pass per-frame task runners to Workers (when possible) (patchset #5 id:100001 of https://codereview.chromium.org/2163983004/ )

Reason for revert:
This CL seems to cause a test failure on "WebKit Linux Leak" bot.
https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Linux%20Leak/builds/21436

An example of the failure is:
http/tests/serviceworker/foreign-fetch-workers.html
({"numberOfLiveActiveDOMObjects":[2,3],"numberOfLiveDocuments":[1,2],"numberOfLiveNodes":[4,15],"numberOfLiveResources":[0,1]})

Original issue's description:
> Pass per-frame task runners to Workers (when possible)
>
> - For in-process workers (i.e. dedicated/compositor workers): always use
>   the associated document's task runners
> - For out-of-process workers (i.e. service/shared workers): keep using the
>   default task runner of the main thread, but via TaskRunnerHelper so
>   we could still start using different task runners for different tasks
>   (e.g. loading vs timer) if we want.
>
> BUG= 627034 
>
> Committed: https://crrev.com/66b07a326160fed57046fbb454f424a302d4cc6b
> Cr-Commit-Position: refs/heads/master@{#408652}

TBR=skyostil@chromium.org,hiroshige@chromium.org,haraken@chromium.org,nhiroki@chromium.org,kinuko@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 627034 

Review-Url: https://codereview.chromium.org/2199633002
Cr-Commit-Position: refs/heads/master@{#408932}

[modify] https://crrev.com/043b1d5eb0223ec6cf40afac5cb591112a5006d7/third_party/WebKit/Source/core/core.gypi
[modify] https://crrev.com/043b1d5eb0223ec6cf40afac5cb591112a5006d7/third_party/WebKit/Source/core/workers/InProcessWorkerMessagingProxy.cpp
[modify] https://crrev.com/043b1d5eb0223ec6cf40afac5cb591112a5006d7/third_party/WebKit/Source/core/workers/InProcessWorkerMessagingProxy.h
[modify] https://crrev.com/043b1d5eb0223ec6cf40afac5cb591112a5006d7/third_party/WebKit/Source/core/workers/InProcessWorkerObjectProxy.cpp
[modify] https://crrev.com/043b1d5eb0223ec6cf40afac5cb591112a5006d7/third_party/WebKit/Source/core/workers/InProcessWorkerObjectProxy.h
[delete] https://crrev.com/d0e9a0766b7c360fb4524e226b9f6bbdf5e68333/third_party/WebKit/Source/core/workers/ParentFrameTaskRunners.cpp
[delete] https://crrev.com/d0e9a0766b7c360fb4524e226b9f6bbdf5e68333/third_party/WebKit/Source/core/workers/ParentFrameTaskRunners.h
[modify] https://crrev.com/043b1d5eb0223ec6cf40afac5cb591112a5006d7/third_party/WebKit/Source/web/WebEmbeddedWorkerImpl.cpp
[modify] https://crrev.com/043b1d5eb0223ec6cf40afac5cb591112a5006d7/third_party/WebKit/Source/web/WebEmbeddedWorkerImpl.h
[modify] https://crrev.com/043b1d5eb0223ec6cf40afac5cb591112a5006d7/third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp
[modify] https://crrev.com/043b1d5eb0223ec6cf40afac5cb591112a5006d7/third_party/WebKit/Source/web/WebSharedWorkerImpl.h

Comment 5 by kinuko@chromium.org, Aug 1 2016

Status: Assigned (was: Fixed)
Reopening :(
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 16 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a1e7e84803441a117280bbf0d20e8d7479453691

commit a1e7e84803441a117280bbf0d20e8d7479453691
Author: kinuko <kinuko@chromium.org>
Date: Tue Aug 16 21:48:22 2016

Pass per-frame task runners to Workers (when possible) [2nd try]

- For in-process workers (i.e. dedicated/compositor workers): always use
  the associated document's task runners
- For out-of-process workers (i.e. service/shared workers): keep using the
  default task runner of the main thread, but via TaskRunnerHelper so
  we could still start using different task runners for different tasks
  (e.g. loading vs timer) if we want.

[New] Diff from previous patch (PS1, https://codereview.chromium.org/2163983004):

- No longer clone and hold per-frame task runners, instead reset them
  when the parent context goes away using ContextLifecycleObserver
  (because Workers could post tasks that trigger manual deletion for non-
  oilpan objects and if such tasks get dropped we could leak [*])
- Support TaskType in a very limited way

[*] The 'manual deletion' part in the worker code should be probably fixed /
removed, but I'd like it to be fixed separately.

BUG= 627034 

Review-Url: https://codereview.chromium.org/2236833002
Cr-Commit-Position: refs/heads/master@{#412347}

[modify] https://crrev.com/a1e7e84803441a117280bbf0d20e8d7479453691/third_party/WebKit/Source/core/core.gypi
[modify] https://crrev.com/a1e7e84803441a117280bbf0d20e8d7479453691/third_party/WebKit/Source/core/dom/TaskRunnerHelper.h
[modify] https://crrev.com/a1e7e84803441a117280bbf0d20e8d7479453691/third_party/WebKit/Source/core/workers/InProcessWorkerMessagingProxy.cpp
[modify] https://crrev.com/a1e7e84803441a117280bbf0d20e8d7479453691/third_party/WebKit/Source/core/workers/InProcessWorkerMessagingProxy.h
[modify] https://crrev.com/a1e7e84803441a117280bbf0d20e8d7479453691/third_party/WebKit/Source/core/workers/InProcessWorkerObjectProxy.cpp
[modify] https://crrev.com/a1e7e84803441a117280bbf0d20e8d7479453691/third_party/WebKit/Source/core/workers/InProcessWorkerObjectProxy.h
[add] https://crrev.com/a1e7e84803441a117280bbf0d20e8d7479453691/third_party/WebKit/Source/core/workers/ParentFrameTaskRunners.cpp
[add] https://crrev.com/a1e7e84803441a117280bbf0d20e8d7479453691/third_party/WebKit/Source/core/workers/ParentFrameTaskRunners.h
[modify] https://crrev.com/a1e7e84803441a117280bbf0d20e8d7479453691/third_party/WebKit/Source/web/WebEmbeddedWorkerImpl.cpp
[modify] https://crrev.com/a1e7e84803441a117280bbf0d20e8d7479453691/third_party/WebKit/Source/web/WebEmbeddedWorkerImpl.h
[modify] https://crrev.com/a1e7e84803441a117280bbf0d20e8d7479453691/third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp
[modify] https://crrev.com/a1e7e84803441a117280bbf0d20e8d7479453691/third_party/WebKit/Source/web/WebSharedWorkerImpl.h

Comment 7 by nhiroki@chromium.org, Oct 6 2016

Labels: M-54
Status: Fixed (was: Assigned)

Comment 8 by nhiroki@chromium.org, Oct 6 2016

Components: Blink>Scheduling
►

Sign in to add a comment