Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2513413003:
Worker: Provide a way to access parent frame task runners from a worker thread (Closed)
|
Created:
4 years ago by nhiroki Modified:
4 years ago CC:
chromium-reviews, shans, rjwright, blink-reviews-animation_chromium.org, shimazu+worker_chromium.org, kinuko+worker_chromium.org, Raymond Toy, darktears, blink-reviews, horo+watch_chromium.org, falken+watch_chromium.org, hongchan, kinuko+watch, blink-worker-reviews_chromium.org, Eric Willigers Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionWorker: Provide a way to access parent frame task runners from a worker thread
To use per-frame task runners on a worker thread, this CL provides a way to
access parent frame task runners from a worker thread. To be specific,
WorkerReportingProxy will have a new interface getParentFrameTaskRunners() and
classes on a worker thread will post a task using the interface.
This is a part of the project to migrate to per-frame task scheduler. A general
design doc about the project is available here:
https://docs.google.com/document/d/10It1DFRP7H3gev9hQA-7dtTcvcMFhrxuCt3-k21yjgQ/edit?usp=sharing
BUG=667310
Committed: https://crrev.com/409972dd9d6a640a5854d74b142bef19e25a36b5
Cr-Commit-Position: refs/heads/master@{#434604}
Patch Set 1 #
Total comments: 4
Patch Set 2 : associate dummy documents with per-frame scheduler #Patch Set 3 : revert PS2 #Patch Set 4 : update comments #
Total comments: 2
Patch Set 5 : update comments #
Total comments: 4
Dependent Patchsets: Messages
Total messages: 46 (28 generated)
The CQ bit was checked by nhiroki@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by nhiroki@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Worker: Provide a way to access parent frame task runners from a worker thread BUG=667310 ========== to ========== Worker: Provide a way to access parent frame task runners from a worker thread To use per-frame task runners on a worker thread, this CL provides a way to access parent frame task runners from a worker thread. To be specific, WorkerReportingProxy will have a new interface getParentFrameTaskRunners() and classes on a worker thread will post a task using the interface. BUG=667310 ==========
Patchset #1 (id:1) has been deleted
nhiroki@chromium.org changed reviewers: + falken@chromium.org, haraken@chromium.org
PTAL, thanks!
On 2016/11/21 16:54:10, nhiroki wrote: > PTAL, thanks! This is a usage example: https://codereview.chromium.org/2518073002/
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2513413003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp (right): https://codereview.chromium.org/2513413003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp:431: m_parentFrameTaskRunners = ParentFrameTaskRunners::create(nullptr); When is the correct document set to m_parentFrameTaskRunners? https://codereview.chromium.org/2513413003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp (right): https://codereview.chromium.org/2513413003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp:391: m_parentFrameTaskRunners = ParentFrameTaskRunners::create(nullptr); When is the correct document set to m_parentFrameTaskRunners?
Thank you for your comments. https://codereview.chromium.org/2513413003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp (right): https://codereview.chromium.org/2513413003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp:431: m_parentFrameTaskRunners = ParentFrameTaskRunners::create(nullptr); On 2016/11/22 01:26:17, haraken wrote: > > When is the correct document set to m_parentFrameTaskRunners? In my understanding, there is no correct document for a service worker because it may associate with multiple documents or no document, for example, when a service worker is initiated by push messaging. Choosing an arbitrary document when a service worker gets associated with some documents could be an option, but it could be harmful to switch per-frame task runners in the middle of life of the worker. Another option would be to adopt a dummy document, I'm not sure if this is more reasonable than using a default task runners though. WDYT? https://codereview.chromium.org/2513413003/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp (right): https://codereview.chromium.org/2513413003/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp:391: m_parentFrameTaskRunners = ParentFrameTaskRunners::create(nullptr); On 2016/11/22 01:26:17, haraken wrote: > > When is the correct document set to m_parentFrameTaskRunners? Ditto. A shared worker can be associated with multiple documents and it's the problem how to choose one of them.
On 2016/11/22 03:27:21, nhiroki wrote: > Thank you for your comments. > > https://codereview.chromium.org/2513413003/diff/20001/third_party/WebKit/Sour... > File third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp (right): > > https://codereview.chromium.org/2513413003/diff/20001/third_party/WebKit/Sour... > third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp:431: > m_parentFrameTaskRunners = ParentFrameTaskRunners::create(nullptr); > On 2016/11/22 01:26:17, haraken wrote: > > > > When is the correct document set to m_parentFrameTaskRunners? > > In my understanding, there is no correct document for a service worker because > it may associate with multiple documents or no document, for example, when a > service worker is initiated by push messaging. > > Choosing an arbitrary document when a service worker gets associated with some > documents could be an option, but it could be harmful to switch per-frame task > runners in the middle of life of the worker. > > Another option would be to adopt a dummy document, I'm not sure if this is more > reasonable than using a default task runners though. WDYT? > > https://codereview.chromium.org/2513413003/diff/20001/third_party/WebKit/Sour... > File third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp (right): > > https://codereview.chromium.org/2513413003/diff/20001/third_party/WebKit/Sour... > third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp:391: > m_parentFrameTaskRunners = ParentFrameTaskRunners::create(nullptr); > On 2016/11/22 01:26:17, haraken wrote: > > > > When is the correct document set to m_parentFrameTaskRunners? > > Ditto. A shared worker can be associated with multiple documents and it's the > problem how to choose one of them. I understand that you cannot associate a service worker with a frame, but would it be possible to associate a task with a frame when the task is actually posted? If yes, we might want to improve ParentFrameTaskRunner so that it can associate a task with a frame when postTask is called. If no, this CL makes sense to me.
Patchset #2 (id:40001) has been deleted
The CQ bit was checked by nhiroki@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Worker: Provide a way to access parent frame task runners from a worker thread To use per-frame task runners on a worker thread, this CL provides a way to access parent frame task runners from a worker thread. To be specific, WorkerReportingProxy will have a new interface getParentFrameTaskRunners() and classes on a worker thread will post a task using the interface. BUG=667310 ========== to ========== Worker: Provide a way to access parent frame task runners from a worker thread To use per-frame task runners on a worker thread, this CL provides a way to access parent frame task runners from a worker thread. To be specific, WorkerReportingProxy will have a new interface getParentFrameTaskRunners() and classes on a worker thread will post a task using the interface. DesingDoc: https://docs.google.com/document/d/10It1DFRP7H3gev9hQA-7dtTcvcMFhrxuCt3-k21yj... BUG=667310 ==========
Description was changed from ========== Worker: Provide a way to access parent frame task runners from a worker thread To use per-frame task runners on a worker thread, this CL provides a way to access parent frame task runners from a worker thread. To be specific, WorkerReportingProxy will have a new interface getParentFrameTaskRunners() and classes on a worker thread will post a task using the interface. DesingDoc: https://docs.google.com/document/d/10It1DFRP7H3gev9hQA-7dtTcvcMFhrxuCt3-k21yj... BUG=667310 ========== to ========== Worker: Provide a way to access parent frame task runners from a worker thread To use per-frame task runners on a worker thread, this CL provides a way to access parent frame task runners from a worker thread. To be specific, WorkerReportingProxy will have a new interface getParentFrameTaskRunners() and classes on a worker thread will post a task using the interface. General design doc: https://docs.google.com/document/d/10It1DFRP7H3gev9hQA-7dtTcvcMFhrxuCt3-k21yj... BUG=667310 ==========
Description was changed from ========== Worker: Provide a way to access parent frame task runners from a worker thread To use per-frame task runners on a worker thread, this CL provides a way to access parent frame task runners from a worker thread. To be specific, WorkerReportingProxy will have a new interface getParentFrameTaskRunners() and classes on a worker thread will post a task using the interface. General design doc: https://docs.google.com/document/d/10It1DFRP7H3gev9hQA-7dtTcvcMFhrxuCt3-k21yj... BUG=667310 ========== to ========== Worker: Provide a way to access parent frame task runners from a worker thread To use per-frame task runners on a worker thread, this CL provides a way to access parent frame task runners from a worker thread. To be specific, WorkerReportingProxy will have a new interface getParentFrameTaskRunners() and classes on a worker thread will post a task using the interface. General design doc about per-frame task scheduler: https://docs.google.com/document/d/10It1DFRP7H3gev9hQA-7dtTcvcMFhrxuCt3-k21yj... BUG=667310 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
On 2016/11/22 03:50:40, haraken wrote: > On 2016/11/22 03:27:21, nhiroki wrote: > > Thank you for your comments. > > > > > https://codereview.chromium.org/2513413003/diff/20001/third_party/WebKit/Sour... > > File third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp (right): > > > > > https://codereview.chromium.org/2513413003/diff/20001/third_party/WebKit/Sour... > > third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp:431: > > m_parentFrameTaskRunners = ParentFrameTaskRunners::create(nullptr); > > On 2016/11/22 01:26:17, haraken wrote: > > > > > > When is the correct document set to m_parentFrameTaskRunners? > > > > In my understanding, there is no correct document for a service worker because > > it may associate with multiple documents or no document, for example, when a > > service worker is initiated by push messaging. > > > > Choosing an arbitrary document when a service worker gets associated with some > > documents could be an option, but it could be harmful to switch per-frame task > > runners in the middle of life of the worker. > > > > Another option would be to adopt a dummy document, I'm not sure if this is > more > > reasonable than using a default task runners though. WDYT? > > > > > https://codereview.chromium.org/2513413003/diff/20001/third_party/WebKit/Sour... > > File third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp (right): > > > > > https://codereview.chromium.org/2513413003/diff/20001/third_party/WebKit/Sour... > > third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp:391: > > m_parentFrameTaskRunners = ParentFrameTaskRunners::create(nullptr); > > On 2016/11/22 01:26:17, haraken wrote: > > > > > > When is the correct document set to m_parentFrameTaskRunners? > > > > Ditto. A shared worker can be associated with multiple documents and it's the > > problem how to choose one of them. > > I understand that you cannot associate a service worker with a frame, but would > it be possible to associate a task with a frame when the task is actually > posted? If yes, we might want to improve ParentFrameTaskRunner so that it can > associate a task with a frame when postTask is called. If no, this CL makes > sense to me. Do you mean associating a task with a dummy document's frame? If so, yes, it's feasible. The patchset-2 does it.
On 2016/11/25 04:32:50, nhiroki wrote: > On 2016/11/22 03:50:40, haraken wrote: > > On 2016/11/22 03:27:21, nhiroki wrote: > > > Thank you for your comments. > > > > > > > > > https://codereview.chromium.org/2513413003/diff/20001/third_party/WebKit/Sour... > > > File third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp > (right): > > > > > > > > > https://codereview.chromium.org/2513413003/diff/20001/third_party/WebKit/Sour... > > > third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp:431: > > > m_parentFrameTaskRunners = ParentFrameTaskRunners::create(nullptr); > > > On 2016/11/22 01:26:17, haraken wrote: > > > > > > > > When is the correct document set to m_parentFrameTaskRunners? > > > > > > In my understanding, there is no correct document for a service worker > because > > > it may associate with multiple documents or no document, for example, when a > > > service worker is initiated by push messaging. > > > > > > Choosing an arbitrary document when a service worker gets associated with > some > > > documents could be an option, but it could be harmful to switch per-frame > task > > > runners in the middle of life of the worker. > > > > > > Another option would be to adopt a dummy document, I'm not sure if this is > > more > > > reasonable than using a default task runners though. WDYT? > > > > > > > > > https://codereview.chromium.org/2513413003/diff/20001/third_party/WebKit/Sour... > > > File third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp (right): > > > > > > > > > https://codereview.chromium.org/2513413003/diff/20001/third_party/WebKit/Sour... > > > third_party/WebKit/Source/web/WebSharedWorkerImpl.cpp:391: > > > m_parentFrameTaskRunners = ParentFrameTaskRunners::create(nullptr); > > > On 2016/11/22 01:26:17, haraken wrote: > > > > > > > > When is the correct document set to m_parentFrameTaskRunners? > > > > > > Ditto. A shared worker can be associated with multiple documents and it's > the > > > problem how to choose one of them. > > > > I understand that you cannot associate a service worker with a frame, but > would > > it be possible to associate a task with a frame when the task is actually > > posted? If yes, we might want to improve ParentFrameTaskRunner so that it can > > associate a task with a frame when postTask is called. If no, this CL makes > > sense to me. > > Do you mean associating a task with a dummy document's frame? If so, yes, it's > feasible. The patchset-2 does it. No. My question is whether it's possible to associate a task with a valid frame (not a frame of the dummy frame) when the task is actually posted. If that's possible, we should improve the infrastructure of ParentFrameTaskRunner so that a task can be associated with a frame when the task gets posted. For example we can replace ParentFrameTaskRunner with something like: class WorkerTaskRunner { WorkerTaskRunner() { ... } // WorkerTaskRunner is not associated with any frame. postTask(LocalFrame* frame, ...) { ... } // The frame association is done when the task is posted to the frame. (Though we cannot expose LocalFrame* itself because LocalFrame* should not be accessible from non-main threads.) };
Discussed offline. PS1 (not PS2) LGTM. Let's pass in nullptr to ParentFrameTaskRunner but explicitly mention that we're intentionally using a nullptr to get a task runner that is not associated with any frame (because that's what service worker and shared worker want).
On 2016/11/25 11:08:40, haraken wrote: > Discussed offline. PS1 (not PS2) LGTM. > > Let's pass in nullptr to ParentFrameTaskRunner but explicitly mention that we're > intentionally using a nullptr to get a task runner that is not associated with > any frame (because that's what service worker and shared worker want). Thank you. Reverted the PS2 and updated comments :)
The CQ bit was checked by nhiroki@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
falken, ping? :)
Description was changed from ========== Worker: Provide a way to access parent frame task runners from a worker thread To use per-frame task runners on a worker thread, this CL provides a way to access parent frame task runners from a worker thread. To be specific, WorkerReportingProxy will have a new interface getParentFrameTaskRunners() and classes on a worker thread will post a task using the interface. General design doc about per-frame task scheduler: https://docs.google.com/document/d/10It1DFRP7H3gev9hQA-7dtTcvcMFhrxuCt3-k21yj... BUG=667310 ========== to ========== Worker: Provide a way to access parent frame task runners from a worker thread To use per-frame task runners on a worker thread, this CL provides a way to access parent frame task runners from a worker thread. To be specific, WorkerReportingProxy will have a new interface getParentFrameTaskRunners() and classes on a worker thread will post a task using the interface. This is a part of the project to migrate to per-frame task scheduler. A general design doc about the project is available here: https://docs.google.com/document/d/10It1DFRP7H3gev9hQA-7dtTcvcMFhrxuCt3-k21yj... BUG=667310 ==========
Description was changed from ========== Worker: Provide a way to access parent frame task runners from a worker thread To use per-frame task runners on a worker thread, this CL provides a way to access parent frame task runners from a worker thread. To be specific, WorkerReportingProxy will have a new interface getParentFrameTaskRunners() and classes on a worker thread will post a task using the interface. This is a part of the project to migrate to per-frame task scheduler. A general design doc about the project is available here: https://docs.google.com/document/d/10It1DFRP7H3gev9hQA-7dtTcvcMFhrxuCt3-k21yj... BUG=667310 ========== to ========== Worker: Provide a way to access parent frame task runners from a worker thread To use per-frame task runners on a worker thread, this CL provides a way to access parent frame task runners from a worker thread. To be specific, WorkerReportingProxy will have a new interface getParentFrameTaskRunners() and classes on a worker thread will post a task using the interface. This is a part of the project to migrate to per-frame task scheduler. A general design doc about the project is available here: https://docs.google.com/document/d/10It1DFRP7H3gev9hQA-7dtTcvcMFhrxuCt3-k21yj... BUG=667310 ==========
I have a comment tweak suggestion but lgtm. https://codereview.chromium.org/2513413003/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp (right): https://codereview.chromium.org/2513413003/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp:427: // ServiceWorker is never associated with any document's frame. We have a "never associated" may be confusing since typically we'd say service worker may be "associated" with many documents or none at all (indeed the next sentence implies this). Maybe this comment can be expanded with something like: "Service worker can sometimes run tasks that are initiated by/associated with a document's frame but these documents can be from a different process. So we intentionally populate the task runners with null document in order to use the thread's default task runner. Note that |m_document| should not be used as it's a dummy document for loading that doesn't represent the frame of any associated document." (same kind of thing for the shared worker comment).
Sorry, I should have recorded our offline discussion: As haraken@ said, we decided to associate a null Document here because... 1) ServiceWorker and SharedWorker are never associated with any frame other than a dummy document's frame. We don't know whether it's beneficial to associate the dummy document's frame. (I guess this could be rather harmful if the frame is considered as backgrounded and its priority is lowered, a service worker is very active though) 2) ServiceWorker and SharedWorker use ParentFrameTaskRunners just for posting a control message from a worker thread to the main thread (not for posting a task to a specific frame), that is, a task cannot be associated with any frame even when it's about to be posted.
Thank you :) https://codereview.chromium.org/2513413003/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp (right): https://codereview.chromium.org/2513413003/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/web/ServiceWorkerGlobalScopeProxy.cpp:427: // ServiceWorker is never associated with any document's frame. We have a On 2016/11/28 01:37:44, falken wrote: > "never associated" may be confusing since typically we'd say service worker may > be "associated" with many documents or none at all (indeed the next sentence > implies this). Maybe this comment can be expanded with something like: "Service > worker can sometimes run tasks that are initiated by/associated with a > document's frame but these documents can be from a different process. So we > intentionally populate the task runners with null document in order to use the > thread's default task runner. Note that |m_document| should not be used as it's > a dummy document for loading that doesn't represent the frame of any associated > document." > > (same kind of thing for the shared worker comment). Updated the comments.
The CQ bit was checked by nhiroki@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org, falken@chromium.org Link to the patchset: https://codereview.chromium.org/2513413003/#ps120001 (title: "update comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
drive-by two nits https://codereview.chromium.org/2513413003/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/workers/WorkerReportingProxy.h (right): https://codereview.chromium.org/2513413003/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/workers/WorkerReportingProxy.h:62: virtual ParentFrameTaskRunners* getParentFrameTaskRunners() = 0; This method as is might feel slightly misplaced to me. If it's to be used only for some reporting (is it?) it'd be nice to comment like that, or we might rather want to update the class-level comment (&| naming) of this interface if it could be used other than for reporting? https://codereview.chromium.org/2513413003/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp (right): https://codereview.chromium.org/2513413003/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp:41: // (Empty) WorkerReportingProxy implementation: Empty -> well, 'almost' empty?
CQ is committing da patch.
Bot data: {"patchset_id": 120001, "attempt_start_ts": 1480300033789510,
"parent_rev": "e6b536518bfaf30f939bead38eb5005b02555277", "commit_rev":
"3544f7f52822dd4c0300c2bbf199dde38d9c8de0"}
Message was sent while issue was closed.
Description was changed from ========== Worker: Provide a way to access parent frame task runners from a worker thread To use per-frame task runners on a worker thread, this CL provides a way to access parent frame task runners from a worker thread. To be specific, WorkerReportingProxy will have a new interface getParentFrameTaskRunners() and classes on a worker thread will post a task using the interface. This is a part of the project to migrate to per-frame task scheduler. A general design doc about the project is available here: https://docs.google.com/document/d/10It1DFRP7H3gev9hQA-7dtTcvcMFhrxuCt3-k21yj... BUG=667310 ========== to ========== Worker: Provide a way to access parent frame task runners from a worker thread To use per-frame task runners on a worker thread, this CL provides a way to access parent frame task runners from a worker thread. To be specific, WorkerReportingProxy will have a new interface getParentFrameTaskRunners() and classes on a worker thread will post a task using the interface. This is a part of the project to migrate to per-frame task scheduler. A general design doc about the project is available here: https://docs.google.com/document/d/10It1DFRP7H3gev9hQA-7dtTcvcMFhrxuCt3-k21yj... BUG=667310 ==========
Message was sent while issue was closed.
Committed patchset #5 (id:120001)
Message was sent while issue was closed.
Description was changed from ========== Worker: Provide a way to access parent frame task runners from a worker thread To use per-frame task runners on a worker thread, this CL provides a way to access parent frame task runners from a worker thread. To be specific, WorkerReportingProxy will have a new interface getParentFrameTaskRunners() and classes on a worker thread will post a task using the interface. This is a part of the project to migrate to per-frame task scheduler. A general design doc about the project is available here: https://docs.google.com/document/d/10It1DFRP7H3gev9hQA-7dtTcvcMFhrxuCt3-k21yj... BUG=667310 ========== to ========== Worker: Provide a way to access parent frame task runners from a worker thread To use per-frame task runners on a worker thread, this CL provides a way to access parent frame task runners from a worker thread. To be specific, WorkerReportingProxy will have a new interface getParentFrameTaskRunners() and classes on a worker thread will post a task using the interface. This is a part of the project to migrate to per-frame task scheduler. A general design doc about the project is available here: https://docs.google.com/document/d/10It1DFRP7H3gev9hQA-7dtTcvcMFhrxuCt3-k21yj... BUG=667310 Committed: https://crrev.com/409972dd9d6a640a5854d74b142bef19e25a36b5 Cr-Commit-Position: refs/heads/master@{#434604} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/409972dd9d6a640a5854d74b142bef19e25a36b5 Cr-Commit-Position: refs/heads/master@{#434604}
Message was sent while issue was closed.
Thank you :) https://codereview.chromium.org/2513413003/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/workers/WorkerReportingProxy.h (right): https://codereview.chromium.org/2513413003/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/workers/WorkerReportingProxy.h:62: virtual ParentFrameTaskRunners* getParentFrameTaskRunners() = 0; On 2016/11/28 04:24:09, kinuko wrote: > This method as is might feel slightly misplaced to me. If it's to be used only > for some reporting (is it?) it'd be nice to comment like that, or we might > rather want to update the class-level comment (&| naming) of this interface if > it could be used other than for reporting? Good point. Now this looks a bit strange to me, too... This ParentFrameTaskRunners is used for reporting worker activities by WorkerReportingProxy or sending control messages (e.g., termination request) by WorkerThread. This virtual interface is for the latter use. Maybe we could remove this interface and then WorkerThread has its own ParentFrameTaskRunners for control messages. I'll work on it. https://codereview.chromium.org/2513413003/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp (right): https://codereview.chromium.org/2513413003/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/modules/webaudio/AudioWorkletThreadTest.cpp:41: // (Empty) WorkerReportingProxy implementation: On 2016/11/28 04:24:09, kinuko wrote: > Empty -> well, 'almost' empty? Acknowledged. I'll fix this in a separate CL. |
