New issue
Advanced search Search tips

Issue 893478 link

Starred by 1 user
Status: Fixed
Owner:
nhiroki@chromium.org
Closed: Oct 12
Cc:
smcgruer@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Task



Sign in to add a comment

Merge (MainThread/Threaded)WorkletGlobalScope into WorkletGlobalScope

Project Member Reported by nhiroki@chromium.org, Oct 9 
In the current implementation, WorkletGlobalScope has the following class hierarchy:

- WorkerOrWorkletGlobalScope
  - WorkletGlobalScope
    - MainThreadWorkletGlobalScope
      - PaintWorkletGlobalScope
      - LayoutWorkletGlobalScope
    - ThreadedWorkletGlobalScope
      - AudioWorkletGlobalScope
      - AnimationWorkletGlobalScope

Separation between MainThreadWorkletGlobalScope and ThreadedWorkletGlobalScope makes it difficult to implement worklets that can run on both the main thread and a worker thread (e.g., threaded paint worklets. See issue 829967). To makes it easier, we could merge them into WorkletGlobalScope like this:

- WorkerOrWorkletGlobalScope
  - WorkletGlobalScope
    - PaintWorkletGlobalScope
    - LayoutWorkletGlobalScope
    - AudioWorkletGlobalScope
    - AnimationWorkletGlobalScope

Probably thread-specific parts in MainThread/ThreadedWorkletGlobalScope will be implemented in WorkletGlobalScope::Delegate or something, and injected by each WorkletGlobalScope subclass to WorkletGlobalScope.
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 10 

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

commit e11ab3d22db5cfbe54850e73c6de9c023f89d0a4
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Wed Oct 10 02:01:00 2018

Worklet: Remove unnecessary MainThreadWorkletGlobalScope::Terminate()

MainThreadWorkletGlobalScope::Terminate() is not necessary because it just
calls public WorkerOrWorkletGlobalScope::Dispose(). For cleanup, this CL makes
callers of Terminate() directlly calls Dispose(), and removes Terminate().

Bug:  893478 
Change-Id: I4790baacc38d28a29fb78abafa47785304eba002
Reviewed-on: https://chromium-review.googlesource.com/c/1272735
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598175}
[modify] https://crrev.com/e11ab3d22db5cfbe54850e73c6de9c023f89d0a4/third_party/blink/renderer/core/layout/custom/layout_worklet_global_scope_proxy.cc
[modify] https://crrev.com/e11ab3d22db5cfbe54850e73c6de9c023f89d0a4/third_party/blink/renderer/core/workers/main_thread_worklet_global_scope.cc
[modify] https://crrev.com/e11ab3d22db5cfbe54850e73c6de9c023f89d0a4/third_party/blink/renderer/core/workers/main_thread_worklet_global_scope.h
[modify] https://crrev.com/e11ab3d22db5cfbe54850e73c6de9c023f89d0a4/third_party/blink/renderer/core/workers/main_thread_worklet_test.cc
[modify] https://crrev.com/e11ab3d22db5cfbe54850e73c6de9c023f89d0a4/third_party/blink/renderer/modules/csspaint/paint_worklet_global_scope_proxy.cc
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 11 

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

commit 651f257ebdfe91db3e87d93ddb60d9831dc54892
Author: Hiroki Nakagawa <nhiroki@chromium.org>
Date: Thu Oct 11 15:50:18 2018

Worklet: Merge (MainThread/Threaded)WorkletGlobalScope into WorkletGlobalScope

This is a cleanup CL to merge MainThreadWorkletGlobalScope and
ThreadedWorkletGlobalScope into WorkletGlobalScope for shallowing the class
hierarchy of WorkletGlobalScope and making it more extensible.

========== Details ==========

Before this CL, WorkletGlobalScope has the following class hierarchy.
MainThreadWorkletGlobalScope and ThreadedWorkletGlobalScope implement thread
specific things, for example, GetTaskRunner().

- WorkerOrWorkletGlobalScope
  - WorkletGlobalScope
    - MainThreadWorkletGlobalScope
      - PaintWorkletGlobalScope
      - LayoutWorkletGlobalScope
    - ThreadedWorkletGlobalScope
      - AudioWorkletGlobalScope
      - AnimationWorkletGlobalScope

This separation has been worked well until now because each subclass of
WorkletGlobalScope runs only on a specific thread (i.e., the main thread or a
worker thread). However, the situation has been changed. We started implementing
off-the-main-thread PaintWorklet (See https://crbug.com/829967) and noticed
this separation makes it so difficult to enable PaintWorklet to run on both the
main thread and a worker thread.

To mitigate the difficulty, this CL merges them into WorkletGlobalScope. After
this CL, the class hierarchy looks like this:

- WorkerOrWorkletGlobalScope
  - WorkletGlobalScope
    - PaintWorkletGlobalScope
    - LayoutWorkletGlobalScope
    - AudioWorkletGlobalScope
    - AnimationWorkletGlobalScope

In this new hierarchy, WorkletGlobalScope implements thread specific things, and
provides multiple constructors for its subclasses to specify thread affinity.

Bug:  893478 
Change-Id: Iad817c7dd10ce5bd51a4c381fbd34aa1669454f7
Reviewed-on: https://chromium-review.googlesource.com/c/1272416
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Makoto Shimazu <shimazu@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598779}
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/bindings/core/v8/binding_security.cc
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/bindings/core/v8/worker_or_worklet_script_controller.h
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/core/inspector/main_thread_debugger.cc
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/core/inspector/worker_thread_debugger.cc
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/core/layout/custom/layout_worklet_global_scope.cc
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/core/layout/custom/layout_worklet_global_scope.h
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/core/loader/modulescript/module_script_loader.cc
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/core/loader/modulescript/module_script_loader_test.cc
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/core/workers/BUILD.gn
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/core/workers/README.md
[delete] https://crrev.com/6b57f8cdf347d74620fc047ad7ff37bc7088d229/third_party/blink/renderer/core/workers/main_thread_worklet_global_scope.cc
[delete] https://crrev.com/6b57f8cdf347d74620fc047ad7ff37bc7088d229/third_party/blink/renderer/core/workers/main_thread_worklet_global_scope.h
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/core/workers/main_thread_worklet_test.cc
[delete] https://crrev.com/6b57f8cdf347d74620fc047ad7ff37bc7088d229/third_party/blink/renderer/core/workers/threaded_worklet_global_scope.cc
[delete] https://crrev.com/6b57f8cdf347d74620fc047ad7ff37bc7088d229/third_party/blink/renderer/core/workers/threaded_worklet_global_scope.h
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/core/workers/threaded_worklet_object_proxy.cc
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/core/workers/threaded_worklet_test.cc
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/core/workers/worker_or_worklet_global_scope.cc
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/core/workers/worker_or_worklet_global_scope.h
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/core/workers/worker_thread.cc
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/core/workers/worklet_global_scope.cc
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/core/workers/worklet_global_scope.h
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/modules/animationworklet/animation_worklet_global_scope.cc
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/modules/animationworklet/animation_worklet_global_scope.h
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/modules/animationworklet/animation_worklet_thread.cc
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/modules/csspaint/paint_worklet_global_scope.cc
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/modules/csspaint/paint_worklet_global_scope.h
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/modules/webaudio/audio_worklet_global_scope.cc
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/modules/webaudio/audio_worklet_global_scope.h
[modify] https://crrev.com/651f257ebdfe91db3e87d93ddb60d9831dc54892/third_party/blink/renderer/modules/webaudio/audio_worklet_thread.cc

Comment 4 by nhiroki@google.com, Oct 12

Status: Fixed (was: Started)
►

Sign in to add a comment