Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2910953002:
Worklet: Enqueue tasks into outsideSetting's event loop (Closed)
|
Created:
3 years, 6 months ago by nhiroki Modified:
3 years, 6 months ago CC:
chromium-reviews, shimazu+worker_chromium.org, kinuko+worker_chromium.org, blink-reviews, horo+watch_chromium.org, falken+watch_chromium.org, blink-worker-reviews_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionWorklet: Enqueue tasks into outsideSetting's event loop
The Worklet spec requires some of tasks during addModule() algorithm are
enqueued into the outsideSetting's event loop. From implementation point of
view, this corresponds to parent frame's task runners. This CL introduces the
parent frame task runners for such places.
Note that in the current implementation all code related to addModule() run on
the main thread because only the main thread worklets support module loading, so
technically this change is not necessary for now. This change will be really
necessary when the threaded worklets also support module loading to jump over
the thread boundary between the main thread and worklet's context thread.
There are no unit tests, but layout tests confirm this change doesn't break any
thing.
Spec: https://drafts.css-houdini.org/worklets/#dom-worklet-addmodule
DesignDoc: https://docs.google.com/document/d/1cgLcrua7H_7x_o5GlzYrAi2qt-TqTzgtOeixFAugR6g/edit?usp=sharing
BUG=727194
Review-Url: https://codereview.chromium.org/2910953002
Cr-Commit-Position: refs/heads/master@{#475456}
Committed: https://chromium.googlesource.com/chromium/src/+/315dddc6bbfe2ca11f6d3b127f02e5fc66d71aae
Patch Set 1 #
Total comments: 2
Patch Set 2 : clean up #Patch Set 3 : fix #
Total comments: 8
Patch Set 4 : add comments #
Total comments: 4
Patch Set 5 : ParentFrameTaskRunners -> WebTaskRunner #Patch Set 6 : fix header inclusion #
Total comments: 4
Patch Set 7 : outside_settings_task_runner #
Total comments: 2
Messages
Total messages: 47 (29 generated)
Description was changed from ========== Worklet: Enqueue tasks into outsideSetting's event loop as the spec BUG=727194 ========== to ========== Worklet: Enqueue tasks into outsideSetting's event loop as the spec The Worklet spec requires some of tasks during addModule() algorithm are enqueued into the outsideSetting's event loop. From implementation point of view, this corresponds to parent frame's task runners. This CL introduces the parent frame task runners for such places. Note that in the current implementation all code related to addModule() run on the main thread because only the main thread worklets support module loading. However, this change will be necessary when the threaded worklets also support module loading to jump over the thread boundary between the main thread and worklet's context thread. Spec: https://drafts.css-houdini.org/worklets/#dom-worklet-addmodule DesignDoc: https://docs.google.com/document/d/1cgLcrua7H_7x_o5GlzYrAi2qt-TqTzgtOeixFAugR... BUG=727194 ==========
Description was changed from ========== Worklet: Enqueue tasks into outsideSetting's event loop as the spec The Worklet spec requires some of tasks during addModule() algorithm are enqueued into the outsideSetting's event loop. From implementation point of view, this corresponds to parent frame's task runners. This CL introduces the parent frame task runners for such places. Note that in the current implementation all code related to addModule() run on the main thread because only the main thread worklets support module loading. However, this change will be necessary when the threaded worklets also support module loading to jump over the thread boundary between the main thread and worklet's context thread. Spec: https://drafts.css-houdini.org/worklets/#dom-worklet-addmodule DesignDoc: https://docs.google.com/document/d/1cgLcrua7H_7x_o5GlzYrAi2qt-TqTzgtOeixFAugR... BUG=727194 ========== to ========== Worklet: Enqueue tasks into outsideSetting's event loop as the spec The Worklet spec requires some of tasks during addModule() algorithm are enqueued into the outsideSetting's event loop. From implementation point of view, this corresponds to parent frame's task runners. This CL introduces the parent frame task runners for such places. Note that in the current implementation all code related to addModule() run on the main thread because only the main thread worklets support module loading, so technically this change is not necessary for now. This change will be really necessary when the threaded worklets also support module loading to jump over the thread boundary between the main thread and worklet's context thread. Spec: https://drafts.css-houdini.org/worklets/#dom-worklet-addmodule DesignDoc: https://docs.google.com/document/d/1cgLcrua7H_7x_o5GlzYrAi2qt-TqTzgtOeixFAugR... BUG=727194 ==========
Description was changed from ========== Worklet: Enqueue tasks into outsideSetting's event loop as the spec The Worklet spec requires some of tasks during addModule() algorithm are enqueued into the outsideSetting's event loop. From implementation point of view, this corresponds to parent frame's task runners. This CL introduces the parent frame task runners for such places. Note that in the current implementation all code related to addModule() run on the main thread because only the main thread worklets support module loading, so technically this change is not necessary for now. This change will be really necessary when the threaded worklets also support module loading to jump over the thread boundary between the main thread and worklet's context thread. Spec: https://drafts.css-houdini.org/worklets/#dom-worklet-addmodule DesignDoc: https://docs.google.com/document/d/1cgLcrua7H_7x_o5GlzYrAi2qt-TqTzgtOeixFAugR... BUG=727194 ========== to ========== Worklet: Enqueue tasks into outsideSetting's event loop The Worklet spec requires some of tasks during addModule() algorithm are enqueued into the outsideSetting's event loop. From implementation point of view, this corresponds to parent frame's task runners. This CL introduces the parent frame task runners for such places. Note that in the current implementation all code related to addModule() run on the main thread because only the main thread worklets support module loading, so technically this change is not necessary for now. This change will be really necessary when the threaded worklets also support module loading to jump over the thread boundary between the main thread and worklet's context thread. Spec: https://drafts.css-houdini.org/worklets/#dom-worklet-addmodule DesignDoc: https://docs.google.com/document/d/1cgLcrua7H_7x_o5GlzYrAi2qt-TqTzgtOeixFAugR... BUG=727194 ==========
Patchset #1 (id:1) 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...
Patchset #1 (id:20001) 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...
nhiroki@chromium.org changed reviewers: + falken@chromium.org, kinuko@chromium.org
PTAL, thanks!
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
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...
https://codereview.chromium.org/2910953002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/workers/WorkletModuleTreeClient.h (right): https://codereview.chromium.org/2910953002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/workers/WorkletModuleTreeClient.h:32: CrossThreadPersistent<ParentFrameTaskRunners> task_runners_; Does this need to be cross-thread persistent?
Thank you. https://codereview.chromium.org/2910953002/diff/40001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/workers/WorkletModuleTreeClient.h (right): https://codereview.chromium.org/2910953002/diff/40001/third_party/WebKit/Sour... third_party/WebKit/Source/core/workers/WorkletModuleTreeClient.h:32: CrossThreadPersistent<ParentFrameTaskRunners> task_runners_; On 2017/05/29 14:08:55, kinuko wrote: > Does this need to be cross-thread persistent? I think this is necessary for threaded worklets. In the case, this is created on the main thread, passed to worklet's thread and accessed from that.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Some nitty comments. Is there no unit test appropriate for this CL? https://codereview.chromium.org/2910953002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/workers/MainThreadWorklet.cpp (right): https://codereview.chromium.org/2910953002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/workers/MainThreadWorklet.cpp:63: // purpose. Would like to make more clear what's in the spec and what's in the impl: "In the specification, outsideSettings is used for posting a task to the document's responsible event loop. In our implementation, ParentFrameTaskRunners does this." https://codereview.chromium.org/2910953002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/workers/MainThreadWorkletGlobalScope.cpp (right): https://codereview.chromium.org/2910953002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/workers/MainThreadWorkletGlobalScope.cpp:55: // https://drafts.css-houdini.org/worklets/#fetch-and-invoke-a-worklet-script Not sure if this documentation is better on the impl or interface. I like the link to the spec since it's clear where the "Step" comments below are from. Maybe it makes sense to put it on both? Although from the caller's point of view, this function does the entire "fetch and invoke a worklet script" algorithm so maybe the interface comment doesn't need to mention "first half" as that's an impl detail. https://codereview.chromium.org/2910953002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/workers/MainThreadWorkletGlobalScope.h (right): https://codereview.chromium.org/2910953002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/workers/MainThreadWorkletGlobalScope.h:42: ParentFrameTaskRunners*, I'm not sure it's clear what ParentFrameTaskRunners is. Could we document this method and give the parameter a name/documentation? https://codereview.chromium.org/2910953002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/workers/WorkletModuleTreeClient.h (right): https://codereview.chromium.org/2910953002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/workers/WorkletModuleTreeClient.h:14: // A client of the module tree fetching living on the worklet context's thread. +1 to add a comment. It's a bit obscure though if you were to open this class on its own. Could it make sense to just say "A ModuleTreeClient that lives on the worklet context's thread."
Thanks for your comments. On 2017/05/30 01:33:23, falken wrote: > Some nitty comments. > > Is there no unit test appropriate for this CL? There is no unit test, there are layout tests that cover this change though. I think adding a unit test for this CL doesn't really make sense because FetchAndInvokeScript() is an async operation by nature and actual behavior doesn't really change at least on the main thread worklets.
https://codereview.chromium.org/2910953002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/workers/MainThreadWorklet.cpp (right): https://codereview.chromium.org/2910953002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/workers/MainThreadWorklet.cpp:63: // purpose. On 2017/05/30 01:33:23, falken wrote: > Would like to make more clear what's in the spec and what's in the impl: > > "In the specification, outsideSettings is used for posting a task to the > document's responsible event loop. In our implementation, ParentFrameTaskRunners > does this." Done. https://codereview.chromium.org/2910953002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/workers/MainThreadWorkletGlobalScope.cpp (right): https://codereview.chromium.org/2910953002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/workers/MainThreadWorkletGlobalScope.cpp:55: // https://drafts.css-houdini.org/worklets/#fetch-and-invoke-a-worklet-script On 2017/05/30 01:33:23, falken wrote: > Not sure if this documentation is better on the impl or interface. I like the > link to the spec since it's clear where the "Step" comments below are from. > Maybe it makes sense to put it on both? Yeah, it makes sense. Added a comment on the header. > Although from the caller's point of > view, this function does the entire "fetch and invoke a worklet script" > algorithm so maybe the interface comment doesn't need to mention "first half" as > that's an impl detail. Good point. I didn't add "first half" part to the header comment. https://codereview.chromium.org/2910953002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/workers/MainThreadWorkletGlobalScope.h (right): https://codereview.chromium.org/2910953002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/workers/MainThreadWorkletGlobalScope.h:42: ParentFrameTaskRunners*, On 2017/05/30 01:33:23, falken wrote: > I'm not sure it's clear what ParentFrameTaskRunners is. Could we document this > method and give the parameter a name/documentation? Added function comments. I feel "ParentFrameTaskRunners" itself would be self-explanatory: it's a set of task runners associated with the parent frames. https://codereview.chromium.org/2910953002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/workers/WorkletModuleTreeClient.h (right): https://codereview.chromium.org/2910953002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/workers/WorkletModuleTreeClient.h:14: // A client of the module tree fetching living on the worklet context's thread. On 2017/05/30 01:33:23, falken wrote: > +1 to add a comment. It's a bit obscure though if you were to open this class on > its own. Could it make sense to just say > > "A ModuleTreeClient that lives on the worklet context's thread." Done.
On 2017/05/30 02:14:08, nhiroki (slow) wrote: > Thanks for your comments. > > On 2017/05/30 01:33:23, falken wrote: > > Some nitty comments. > > > > Is there no unit test appropriate for this CL? > > There is no unit test, there are layout tests that cover this change though. I > think adding a unit test for this CL doesn't really make sense because > FetchAndInvokeScript() is an async operation by nature and actual behavior > doesn't really change at least on the main thread worklets. OK let's add that to the CL description.
ah sent that too early. lgtm.
Looking good, a few more questions... https://codereview.chromium.org/2910953002/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/workers/MainThreadWorklet.cpp (right): https://codereview.chromium.org/2910953002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/workers/MainThreadWorklet.cpp:63: // ParentFrameTaskRunners does this. Do we want to pass the entire ParentFrameTaskRunners here, or is a task runner for 'responsible event loop' is the only one we'll need? https://codereview.chromium.org/2910953002/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/workers/WorkletModuleTreeClient.cpp (right): https://codereview.chromium.org/2910953002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/workers/WorkletModuleTreeClient.cpp:29: // The steps are implemented in WorkletPendingTasks::Abort(). The spec seems to say 'responsible event loop' is a newly created workletEventLoop (am I following right?), is kUnspecedLoading the right one?
Description was changed from ========== Worklet: Enqueue tasks into outsideSetting's event loop The Worklet spec requires some of tasks during addModule() algorithm are enqueued into the outsideSetting's event loop. From implementation point of view, this corresponds to parent frame's task runners. This CL introduces the parent frame task runners for such places. Note that in the current implementation all code related to addModule() run on the main thread because only the main thread worklets support module loading, so technically this change is not necessary for now. This change will be really necessary when the threaded worklets also support module loading to jump over the thread boundary between the main thread and worklet's context thread. Spec: https://drafts.css-houdini.org/worklets/#dom-worklet-addmodule DesignDoc: https://docs.google.com/document/d/1cgLcrua7H_7x_o5GlzYrAi2qt-TqTzgtOeixFAugR... BUG=727194 ========== to ========== Worklet: Enqueue tasks into outsideSetting's event loop The Worklet spec requires some of tasks during addModule() algorithm are enqueued into the outsideSetting's event loop. From implementation point of view, this corresponds to parent frame's task runners. This CL introduces the parent frame task runners for such places. Note that in the current implementation all code related to addModule() run on the main thread because only the main thread worklets support module loading, so technically this change is not necessary for now. This change will be really necessary when the threaded worklets also support module loading to jump over the thread boundary between the main thread and worklet's context thread. There are no unit tests, but layout tests confirm this change doesn't break any thing. Spec: https://drafts.css-houdini.org/worklets/#dom-worklet-addmodule DesignDoc: https://docs.google.com/document/d/1cgLcrua7H_7x_o5GlzYrAi2qt-TqTzgtOeixFAugR... BUG=727194 ==========
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...
Thank you! Updated. https://codereview.chromium.org/2910953002/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/workers/MainThreadWorklet.cpp (right): https://codereview.chromium.org/2910953002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/workers/MainThreadWorklet.cpp:63: // ParentFrameTaskRunners does this. On 2017/05/30 02:32:58, kinuko wrote: > Do we want to pass the entire ParentFrameTaskRunners here, or is a task runner > for 'responsible event loop' is the only one we'll need? We don't need the entire task runners. Changed. https://codereview.chromium.org/2910953002/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/workers/WorkletModuleTreeClient.cpp (right): https://codereview.chromium.org/2910953002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/workers/WorkletModuleTreeClient.cpp:29: // The steps are implemented in WorkletPendingTasks::Abort(). On 2017/05/30 02:32:58, kinuko wrote: > The spec seems to say 'responsible event loop' is a newly created > workletEventLoop (am I following right?), is kUnspecedLoading the right one? According to [1], "workletEventLoop" is the message loop for "insideSettings" (i.e., WorkeltGlobalScope). We're posting a task to the message loop for "outsideSettings" (i.e., Document). The spec doesn't clearly say about the task type like this[2]: "A responsible event loop: An event loop that is used when it would not be immediately clear what event loop to use." kUnspecedLoading is used in MainThreadWorklet::addModule() because this is a part of module loading. That's the reason why I chose it here. [1] https://drafts.css-houdini.org/worklets/#creating-a-workletglobalscope [2] https://html.spec.whatwg.org/multipage/webappapis.html#responsible-event-loop
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...) mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
lgtm https://codereview.chromium.org/2910953002/diff/140001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/workers/MainThreadWorklet.cpp (right): https://codereview.chromium.org/2910953002/diff/140001/third_party/WebKit/Sou... third_party/WebKit/Source/core/workers/MainThreadWorklet.cpp:62: // does this. Now the last part of this comment reads a bit weird. Maybe say something like: '... a task to the document's responsible event loop. In our implementation we use the document's UnspecedLoading task runner as that is what we commonly use for module loading' ? https://codereview.chromium.org/2910953002/diff/140001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/workers/MainThreadWorkletGlobalScope.h (right): https://codereview.chromium.org/2910953002/diff/140001/third_party/WebKit/Sou... third_party/WebKit/Source/core/workers/MainThreadWorkletGlobalScope.h:43: // the given WorkletPendingTasks via |parent_frame_task_runner|. nit: parent_frame_task_runner -> outside_settings_task_runner? (Maybe with a note like 'i.e. the parent frame's relevant task runner')
The CQ bit was checked by nhiroki@chromium.org to run a CQ dry run
Thank you. https://codereview.chromium.org/2910953002/diff/140001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/workers/MainThreadWorklet.cpp (right): https://codereview.chromium.org/2910953002/diff/140001/third_party/WebKit/Sou... third_party/WebKit/Source/core/workers/MainThreadWorklet.cpp:62: // does this. On 2017/05/30 04:38:52, kinuko wrote: > Now the last part of this comment reads a bit weird. Maybe say something like: > '... a task to the document's responsible event loop. In our implementation we > use the document's UnspecedLoading task runner as that is what we commonly use > for module loading' ? Done. https://codereview.chromium.org/2910953002/diff/140001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/workers/MainThreadWorkletGlobalScope.h (right): https://codereview.chromium.org/2910953002/diff/140001/third_party/WebKit/Sou... third_party/WebKit/Source/core/workers/MainThreadWorkletGlobalScope.h:43: // the given WorkletPendingTasks via |parent_frame_task_runner|. On 2017/05/30 04:38:52, kinuko wrote: > nit: > parent_frame_task_runner -> outside_settings_task_runner? (Maybe with a note > like 'i.e. the parent frame's relevant task runner') Done.
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
nhiroki@chromium.org changed reviewers: + haraken@chromium.org
+haraken@, can you review modules/csspaint? Thanks
LGTM https://codereview.chromium.org/2910953002/diff/160001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/workers/WorkletModuleTreeClient.h (right): https://codereview.chromium.org/2910953002/diff/160001/third_party/WebKit/Sou... third_party/WebKit/Source/core/workers/WorkletModuleTreeClient.h:33: CrossThreadPersistent<WorkletPendingTasks> pending_tasks_; Why do we need to change the Member to CrossThreadPersistent?
Thank you. https://codereview.chromium.org/2910953002/diff/160001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/workers/WorkletModuleTreeClient.h (right): https://codereview.chromium.org/2910953002/diff/160001/third_party/WebKit/Sou... third_party/WebKit/Source/core/workers/WorkletModuleTreeClient.h:33: CrossThreadPersistent<WorkletPendingTasks> pending_tasks_; On 2017/05/30 05:33:43, haraken wrote: > > Why do we need to change the Member to CrossThreadPersistent? For threaded worklets, |pending_tasks_| is created on the main thread and passed to the worker thread, while WorkletModuleTreeClient is naturally created on the worker thread.
The CQ bit was unchecked by nhiroki@chromium.org
The CQ bit was checked by nhiroki@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from falken@chromium.org, kinuko@chromium.org Link to the patchset: https://codereview.chromium.org/2910953002/#ps160001 (title: "outside_settings_task_runner")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2017/05/30 05:59:00, nhiroki (slow) wrote: > Thank you. > > https://codereview.chromium.org/2910953002/diff/160001/third_party/WebKit/Sou... > File third_party/WebKit/Source/core/workers/WorkletModuleTreeClient.h (right): > > https://codereview.chromium.org/2910953002/diff/160001/third_party/WebKit/Sou... > third_party/WebKit/Source/core/workers/WorkletModuleTreeClient.h:33: > CrossThreadPersistent<WorkletPendingTasks> pending_tasks_; > On 2017/05/30 05:33:43, haraken wrote: > > > > Why do we need to change the Member to CrossThreadPersistent? > > For threaded worklets, |pending_tasks_| is created on the main thread and passed > to the worker thread, while WorkletModuleTreeClient is naturally created on the > worker thread. Makes sense.
CQ is committing da patch.
Bot data: {"patchset_id": 160001, "attempt_start_ts": 1496124045895170,
"parent_rev": "0b74898cc7b8d9fdfe31d7ee00a206380b498ec9", "commit_rev":
"315dddc6bbfe2ca11f6d3b127f02e5fc66d71aae"}
Message was sent while issue was closed.
Description was changed from ========== Worklet: Enqueue tasks into outsideSetting's event loop The Worklet spec requires some of tasks during addModule() algorithm are enqueued into the outsideSetting's event loop. From implementation point of view, this corresponds to parent frame's task runners. This CL introduces the parent frame task runners for such places. Note that in the current implementation all code related to addModule() run on the main thread because only the main thread worklets support module loading, so technically this change is not necessary for now. This change will be really necessary when the threaded worklets also support module loading to jump over the thread boundary between the main thread and worklet's context thread. There are no unit tests, but layout tests confirm this change doesn't break any thing. Spec: https://drafts.css-houdini.org/worklets/#dom-worklet-addmodule DesignDoc: https://docs.google.com/document/d/1cgLcrua7H_7x_o5GlzYrAi2qt-TqTzgtOeixFAugR... BUG=727194 ========== to ========== Worklet: Enqueue tasks into outsideSetting's event loop The Worklet spec requires some of tasks during addModule() algorithm are enqueued into the outsideSetting's event loop. From implementation point of view, this corresponds to parent frame's task runners. This CL introduces the parent frame task runners for such places. Note that in the current implementation all code related to addModule() run on the main thread because only the main thread worklets support module loading, so technically this change is not necessary for now. This change will be really necessary when the threaded worklets also support module loading to jump over the thread boundary between the main thread and worklet's context thread. There are no unit tests, but layout tests confirm this change doesn't break any thing. Spec: https://drafts.css-houdini.org/worklets/#dom-worklet-addmodule DesignDoc: https://docs.google.com/document/d/1cgLcrua7H_7x_o5GlzYrAi2qt-TqTzgtOeixFAugR... BUG=727194 Review-Url: https://codereview.chromium.org/2910953002 Cr-Commit-Position: refs/heads/master@{#475456} Committed: https://chromium.googlesource.com/chromium/src/+/315dddc6bbfe2ca11f6d3b127f02... ==========
Message was sent while issue was closed.
Committed patchset #7 (id:160001) as https://chromium.googlesource.com/chromium/src/+/315dddc6bbfe2ca11f6d3b127f02... |
