DOMWrapperWorld::allWorldsInMainThread doesn't account for worker worlds created on the main thread |
||||
Issue descriptionWorklets can create worker worlds on the main thread. The current implementation doesn't account for these worlds. Either the name of the method should be changed, or its implementation. https://cs.chromium.org/chromium/src/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.cpp?type=cs&l=128
,
Mar 2 2017
,
Mar 7 2017
CL: https://codereview.chromium.org/2735823006/
,
Mar 9 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/611dfce737a7df5d5a1e88e8528d405ebed37b8d commit 611dfce737a7df5d5a1e88e8528d405ebed37b8d Author: nhiroki <nhiroki@chromium.org> Date: Thu Mar 09 08:44:55 2017 Bindings: Separate WorldIdConstants to WorldTypes and WorldId This is a clean-up CL and doesn't change behavior. For improving extensibility, this CL separates WorldIdConstants to 2 parts: WorldTypes and WorldId. This encapsulates identifier allocation logic in DOMWrapperWorld[1] and makes it easier to expand the identifier space for Worklets[2]. [1] WorldIds for IsolatedWorlds still need to be given from out of DOMWrapperWorld because of its unique convention to allocate the identifier. [2] https://codereview.chromium.org/2735823006/ BUG= 697622 , 697629 Review-Url: https://codereview.chromium.org/2735973006 Cr-Commit-Position: refs/heads/master@{#455700} [modify] https://crrev.com/611dfce737a7df5d5a1e88e8528d405ebed37b8d/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.cpp [modify] https://crrev.com/611dfce737a7df5d5a1e88e8528d405ebed37b8d/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.h [modify] https://crrev.com/611dfce737a7df5d5a1e88e8528d405ebed37b8d/third_party/WebKit/Source/bindings/core/v8/V8GCController.cpp [modify] https://crrev.com/611dfce737a7df5d5a1e88e8528d405ebed37b8d/third_party/WebKit/Source/bindings/core/v8/V8PerIsolateData.cpp [modify] https://crrev.com/611dfce737a7df5d5a1e88e8528d405ebed37b8d/third_party/WebKit/Source/bindings/core/v8/WorkerOrWorkletScriptController.cpp [modify] https://crrev.com/611dfce737a7df5d5a1e88e8528d405ebed37b8d/third_party/WebKit/Source/core/xml/DocumentXMLTreeViewer.cpp [modify] https://crrev.com/611dfce737a7df5d5a1e88e8528d405ebed37b8d/third_party/WebKit/Source/modules/fetch/DataConsumerHandleTestUtil.cpp [modify] https://crrev.com/611dfce737a7df5d5a1e88e8528d405ebed37b8d/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
,
Mar 14 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0fe6bc1c1b65086da86d225ac7dfe9b00a317a1e commit 0fe6bc1c1b65086da86d225ac7dfe9b00a317a1e Author: nhiroki <nhiroki@chromium.org> Date: Tue Mar 14 13:22:54 2017 Bindings: Manage multiple DOMWrapperWorlds for worklets Before this CL, there are 2 issues: 1) DOMWrapperWorld for a worker-or-worklet is managed as per-thread singleton. This doesn't work when multiple worklets are created on the same thread. 2) DOMWrapperWorld assumes that a worker-or-worklet world is never created on the main thread, but actually it's possible for the main thread worklet (i.e. PaintWorklet). This CL introduces a per-thread |worldMap| to manage multiple worker-or-worklet worlds on a single thread for the issue 1), and teaches the main thread that there can be worker-or-worklet worlds for the issue 2 ). BUG= 697622 , 697629 Review-Url: https://codereview.chromium.org/2735823006 Cr-Commit-Position: refs/heads/master@{#456683} [modify] https://crrev.com/0fe6bc1c1b65086da86d225ac7dfe9b00a317a1e/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.cpp [modify] https://crrev.com/0fe6bc1c1b65086da86d225ac7dfe9b00a317a1e/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.h
,
Mar 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/178fc4a5d430afe4ae1cd9fabbf1a631dd09e965 commit 178fc4a5d430afe4ae1cd9fabbf1a631dd09e965 Author: nhiroki <nhiroki@chromium.org> Date: Thu Mar 16 05:22:23 2017 Bindings: Manage multiple DOMWrapperWorlds for worklets (2) This is a follow-up CL for https://crrev.com/2735823006/ This CL... - merges isolatedWorldMap() into worldMap() for simplification, - fixes ArrayBuffer accumulation to correctly handle all worlds on non-main thread, and - adds unit tests for DOMWrapperWorld. BUG= 697622 , 697629 Review-Url: https://codereview.chromium.org/2747163002 Cr-Commit-Position: refs/heads/master@{#457357} [modify] https://crrev.com/178fc4a5d430afe4ae1cd9fabbf1a631dd09e965/third_party/WebKit/Source/bindings/bindings.gni [modify] https://crrev.com/178fc4a5d430afe4ae1cd9fabbf1a631dd09e965/third_party/WebKit/Source/bindings/core/v8/DOMDataStore.h [modify] https://crrev.com/178fc4a5d430afe4ae1cd9fabbf1a631dd09e965/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.cpp [modify] https://crrev.com/178fc4a5d430afe4ae1cd9fabbf1a631dd09e965/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.h [add] https://crrev.com/178fc4a5d430afe4ae1cd9fabbf1a631dd09e965/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorldTest.cpp [modify] https://crrev.com/178fc4a5d430afe4ae1cd9fabbf1a631dd09e965/third_party/WebKit/Source/bindings/core/v8/SerializedScriptValue.cpp
,
Mar 16 2017
,
Mar 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1d85fa7d0be347292a5805ec87ca3d2d10a8c57f commit 1d85fa7d0be347292a5805ec87ca3d2d10a8c57f Author: nhiroki <nhiroki@chromium.org> Date: Fri Mar 17 12:24:56 2017 Bindings: Remove DOMWrapperWorld::fromWorldId() for cleanup This CL removes DOMWrapperWorld::fromWorldId() that looks like a simple getter but actually has a side effect like this: 1) When a given id is for the main world, ensures the main world and returns it. 2) When a given id is for an isolated world, ensures the isolated world and returns it. 3) Otherwise, looks up a given id in the world map and then returns the finding. If the world doesn't exist, returns nullptr. After this CL, mainWorld() is used for 1) and ensureIsolatedWorld() is used for 2). fromWorldId() hasn't been used for 3), so this CL does not introduce an alternative for the case. Also, this CL pushes the main world into the world map for cleanup. Before this CL, the main world is managed separately from other worlds. Managing all worlds in the single map would be simpler. BUG= 697622 , 697629 Review-Url: https://codereview.chromium.org/2753013003 Cr-Commit-Position: refs/heads/master@{#457739} [modify] https://crrev.com/1d85fa7d0be347292a5805ec87ca3d2d10a8c57f/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.cpp [modify] https://crrev.com/1d85fa7d0be347292a5805ec87ca3d2d10a8c57f/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.h [modify] https://crrev.com/1d85fa7d0be347292a5805ec87ca3d2d10a8c57f/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorldTest.cpp [modify] https://crrev.com/1d85fa7d0be347292a5805ec87ca3d2d10a8c57f/third_party/WebKit/Source/web/SuspendableScriptExecutor.cpp [modify] https://crrev.com/1d85fa7d0be347292a5805ec87ca3d2d10a8c57f/third_party/WebKit/Source/web/SuspendableScriptExecutor.h [modify] https://crrev.com/1d85fa7d0be347292a5805ec87ca3d2d10a8c57f/third_party/WebKit/Source/web/WebLocalFrameImpl.cpp
,
Mar 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3e5d3fc45d06833189606b0ad750b9dee95ef50a commit 3e5d3fc45d06833189606b0ad750b9dee95ef50a Author: nhiroki <nhiroki@chromium.org> Date: Wed Mar 29 05:53:18 2017 Bindings: Manage the main world separately from other worlds This CL speculatively reverts a part of the CL[1]. The CL[1] pushed the main world into the world map for cleanup, but that CL seems to cause a performance regression on a perf test (see issue 704778 ) probably because of changing the hashmap traits and/or the world map management. [1] https://codereview.chromium.org/2753013003/ BUG= 697622 , 697629 , 704778 Review-Url: https://codereview.chromium.org/2777763003 Cr-Commit-Position: refs/heads/master@{#460294} [modify] https://crrev.com/3e5d3fc45d06833189606b0ad750b9dee95ef50a/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorld.cpp [modify] https://crrev.com/3e5d3fc45d06833189606b0ad750b9dee95ef50a/third_party/WebKit/Source/bindings/core/v8/DOMWrapperWorldTest.cpp |
||||
►
Sign in to add a comment |
||||
Comment 1 by jbroman@chromium.org
, Mar 1 2017