Support the Event mechanism in WorkletGlobalScope |
|||||||
Issue descriptionDuring the implementation of MessagePort in AudioWorkletNode and AudioWorkletProcessor, we found that V8AbstractEventListener cannot be used in WorkletGlobalScope. https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.cpp?sq=package:chromium&l=63 It is necessary to rearrange the class design so V8AbstractEventListener can support WorkerOrWorkletGlobalScope. Related WIP CL: https://chromium-review.googlesource.com/c/chromium/src/+/747621
,
Nov 8 2017
I'll take a look.
,
Nov 9 2017
,
Nov 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3a8139a4c20831d215c5bfcdfae38915d3dc418e commit 3a8139a4c20831d215c5bfcdfae38915d3dc418e Author: Hiroki Nakagawa <nhiroki@chromium.org> Date: Thu Nov 09 22:18:48 2017 Worklet: Move EventListener stuff from WorkerGlobalScope to WorkerOrWorkletGlobalScope This is a preparation CL for introducing MessagePort support in AudioWorklet. AudioWorklet is the first worklet that will have the Event mechanism. To implement it, this CL moves functions etc related to events from WorkerGlobalScope to WorkerOrWorkletGlobalScope so that they are usable in both workers and worklets. Bug: 782696 Change-Id: I96dbee4f58d90f71f813db42938b1b7f5277f3dd Reviewed-on: https://chromium-review.googlesource.com/759438 Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Kinuko Yasuda <kinuko@chromium.org> Cr-Commit-Position: refs/heads/master@{#515317} [modify] https://crrev.com/3a8139a4c20831d215c5bfcdfae38915d3dc418e/third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.cpp [modify] https://crrev.com/3a8139a4c20831d215c5bfcdfae38915d3dc418e/third_party/WebKit/Source/bindings/core/v8/V8AbstractEventListener.h [modify] https://crrev.com/3a8139a4c20831d215c5bfcdfae38915d3dc418e/third_party/WebKit/Source/bindings/core/v8/WorkerOrWorkletScriptController.cpp [modify] https://crrev.com/3a8139a4c20831d215c5bfcdfae38915d3dc418e/third_party/WebKit/Source/core/workers/WorkerEventQueue.cpp [modify] https://crrev.com/3a8139a4c20831d215c5bfcdfae38915d3dc418e/third_party/WebKit/Source/core/workers/WorkerEventQueue.h [modify] https://crrev.com/3a8139a4c20831d215c5bfcdfae38915d3dc418e/third_party/WebKit/Source/core/workers/WorkerGlobalScope.cpp [modify] https://crrev.com/3a8139a4c20831d215c5bfcdfae38915d3dc418e/third_party/WebKit/Source/core/workers/WorkerGlobalScope.h [modify] https://crrev.com/3a8139a4c20831d215c5bfcdfae38915d3dc418e/third_party/WebKit/Source/core/workers/WorkerOrWorkletGlobalScope.cpp [modify] https://crrev.com/3a8139a4c20831d215c5bfcdfae38915d3dc418e/third_party/WebKit/Source/core/workers/WorkerOrWorkletGlobalScope.h [modify] https://crrev.com/3a8139a4c20831d215c5bfcdfae38915d3dc418e/third_party/WebKit/Source/core/workers/WorkletGlobalScope.cpp [modify] https://crrev.com/3a8139a4c20831d215c5bfcdfae38915d3dc418e/third_party/WebKit/Source/core/workers/WorkletGlobalScope.h
,
Nov 9 2017
hongchan@: I think you can now resume your work. Please let me know if you need additional changes :)
,
Nov 10 2017
Thanks for the fix! :) But unfortunately I am having another issue: 1. WorkerOrWorkletGlobalScope::IsClosing() is supposed to be impl. by subclasses. https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/workers/WorkerOrWorkletGlobalScope.h?q=WorkerOrWorklet&sq=package:chromium&l=63 2. WorkletGlobalScope::IsClosing() always returns false. https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/workers/WorkletGlobalScope.h?sq=package:chromium&l=49 3. WorkerOrWorkletGlobalScope::DeregisterEventListener() fails at CHECKs: https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/workers/WorkerOrWorkletGlobalScope.cpp?type=cs&sq=package:chromium&l=172 I'll add more issue as I investigate MessagePort integration on AudioWorkletGlobalScope.
,
Nov 10 2017
Exposing MessagePort to AudioWorkletGlobalScope may implicate a bit more plumbing. It exposes MessagePort, Event, EventTarget to AudioWorkletGlobalScope. (I changed HTML/DOM spec to reflect this.) Now it requires proper implementations of methods like Dispose(), IsClosing(). Not really familiar with the management of lifecycle or internal storage in WorkletGlobalScope, but I'll try to update this issue as much as possible.
,
Nov 15 2017
I found another issue: 4. ToV8Context() (in V8BindingForCore.cpp) called from V8AbstractEventListener::handleEvent() returns an empty v8::Context when the current execution context is WorkletGlobalScope. Also, we need to replace ToWorkerGlobalScope() in V8WorkerGlobalScopeEventListener::HandleEvent() with ToWorkerOrWorkletGlobalScope(). Copied from https://chromium-review.googlesource.com/c/chromium/src/+/747621#message-c892c28b0e9511bebf678c375876a8f528f0f214
,
Nov 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2cfad34cce2c9a4c54752bec654998a5f382f61c commit 2cfad34cce2c9a4c54752bec654998a5f382f61c Author: Hiroki Nakagawa <nhiroki@chromium.org> Date: Wed Nov 15 08:30:41 2017 Worklet: Replace To/IsWorkerGlobalScope() with To/IsWorkerOrWorkletGlobalScope in Event code This CL enables worklets to reuse the Event handler code for workers. Tests will be added by a following CL: https://chromium-review.googlesource.com/c/chromium/src/+/747621 Bug: 782696 Change-Id: Idf5bf9553dccb9a6de4e064d4317f4d7ebdbb4e0 Reviewed-on: https://chromium-review.googlesource.com/770643 Reviewed-by: Kentaro Hara <haraken@chromium.org> Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org> Cr-Commit-Position: refs/heads/master@{#516637} [modify] https://crrev.com/2cfad34cce2c9a4c54752bec654998a5f382f61c/third_party/WebKit/Source/bindings/core/v8/V8BindingForCore.cpp [modify] https://crrev.com/2cfad34cce2c9a4c54752bec654998a5f382f61c/third_party/WebKit/Source/bindings/core/v8/V8WorkerGlobalScopeEventListener.cpp [modify] https://crrev.com/2cfad34cce2c9a4c54752bec654998a5f382f61c/third_party/WebKit/Source/bindings/core/v8/V8WorkerGlobalScopeEventListener.h
,
Nov 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cd5e88dc00881e051bc6cd650a493095d2a5ebb0 commit cd5e88dc00881e051bc6cd650a493095d2a5ebb0 Author: Hiroki Nakagawa <nhiroki@chromium.org> Date: Fri Nov 17 05:10:12 2017 Worklet: Rename V8WorkerGlobalScopeEventListener to V8WorkerOrWorkletEventListener This is a follow-up CL for https://chromium-review.googlesource.com/c/759438/ After the CL, V8WorkerGlobalScopeEventListener is used not only for Workers but also for Worklets. Bug: 782696 Change-Id: Iece08a78ff0d640aca21e49bf23ecf9623204daa Reviewed-on: https://chromium-review.googlesource.com/776207 Reviewed-by: Kentaro Hara <haraken@chromium.org> Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org> Cr-Commit-Position: refs/heads/master@{#517312} [modify] https://crrev.com/cd5e88dc00881e051bc6cd650a493095d2a5ebb0/third_party/WebKit/Source/bindings/bindings.gni [modify] https://crrev.com/cd5e88dc00881e051bc6cd650a493095d2a5ebb0/third_party/WebKit/Source/bindings/core/v8/V8EventListenerHelper.cpp [rename] https://crrev.com/cd5e88dc00881e051bc6cd650a493095d2a5ebb0/third_party/WebKit/Source/bindings/core/v8/V8WorkerOrWorkletEventListener.cpp [rename] https://crrev.com/cd5e88dc00881e051bc6cd650a493095d2a5ebb0/third_party/WebKit/Source/bindings/core/v8/V8WorkerOrWorkletEventListener.h
,
Nov 22 2017
Looks like all blockers are gone. Feel free to reopen this issue if you find another blocker. |
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by hongchan@chromium.org
, Nov 8 2017