WindowProxy should correctly maintain the lifecycle of contexts |
||
Issue descriptionThe current lifecycle management of WindowProxy's contexts is really messy. People are abusing WindowProxy::isContextInitialized/isGlobalInitialized without understanding when they return true/false. We should introduce a clearer lifecycle model to WindowProxy.
,
Dec 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/45830933ad06e7f8a213e97269522469b1602863 commit 45830933ad06e7f8a213e97269522469b1602863 Author: haraken <haraken@chromium.org> Date: Wed Dec 28 10:08:14 2016 Remove existingWindowProxy() The only call site of existingWindowProxy() is V8Initializer::messageHandlerInMainThread(). However, as commented in the code, that code should not be needed because if the context is during initialization it should already have bailed out at the if(!scriptState->contextIsValid()) check at the very beginning of V8Initializer::messageHandlerInMainThread(). If the reasoning is correct, we can simply remove all code around existingWindowProxy(). BUG= 677253 Review-Url: https://codereview.chromium.org/2607743002 Cr-Commit-Position: refs/heads/master@{#440840} [modify] https://crrev.com/45830933ad06e7f8a213e97269522469b1602863/third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp [modify] https://crrev.com/45830933ad06e7f8a213e97269522469b1602863/third_party/WebKit/Source/bindings/core/v8/ScriptController.h [modify] https://crrev.com/45830933ad06e7f8a213e97269522469b1602863/third_party/WebKit/Source/bindings/core/v8/V8Initializer.cpp [modify] https://crrev.com/45830933ad06e7f8a213e97269522469b1602863/third_party/WebKit/Source/bindings/core/v8/WindowProxyManager.cpp [modify] https://crrev.com/45830933ad06e7f8a213e97269522469b1602863/third_party/WebKit/Source/bindings/core/v8/WindowProxyManager.h
,
Dec 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/3d044ba6f81e1bd0cd255d4351a811976b80bd50 commit 3d044ba6f81e1bd0cd255d4351a811976b80bd50 Author: haraken <haraken@chromium.org> Date: Wed Dec 28 10:44:27 2016 Remove an isContextInitialized() check from ScriptController::executeScriptInIsolatedWorld By checking scriptState->contextIsValid(), we don't need to check isContextInitialized(). BUG= 677253 Review-Url: https://codereview.chromium.org/2605853002 Cr-Commit-Position: refs/heads/master@{#440841} [modify] https://crrev.com/3d044ba6f81e1bd0cd255d4351a811976b80bd50/third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp
,
Dec 28 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/403eecaa2fc7ea026528eecbdeeda3483b45ca6f commit 403eecaa2fc7ea026528eecbdeeda3483b45ca6f Author: haraken <haraken@chromium.org> Date: Wed Dec 28 10:50:28 2016 Remove an isGlobalInitialized() check from WindowProxy::updateDocument() Context states transit from Uninitialized to Initialized, and then to Detached. - isGlobalInitialized() returns true when the context state is Initialized or Detached. - isContextInitialized() returns true when the context state is Initialized. Thus !isContextInitialized() is stronger than !isGlobalInitialized(). Hence we can remove if(!isGlobalInitialized()) from WindowProxy::updateDocument(). BUG= 677253 Review-Url: https://codereview.chromium.org/2603953002 Cr-Commit-Position: refs/heads/master@{#440842} [modify] https://crrev.com/403eecaa2fc7ea026528eecbdeeda3483b45ca6f/third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp
,
Jan 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b336c47b04fb8a5d5508397496d7c1e58e5681ee commit b336c47b04fb8a5d5508397496d7c1e58e5681ee Author: haraken <haraken@chromium.org> Date: Thu Jan 05 12:52:27 2017 Remove an isContextInitialized() check from WindowProxyManager::updateSecurityOrigin() The check is not needed since it is already checked in WindowProxy::updateSecurityOrigin(). BUG= 677253 Review-Url: https://codereview.chromium.org/2606863003 Cr-Commit-Position: refs/heads/master@{#441640} [modify] https://crrev.com/b336c47b04fb8a5d5508397496d7c1e58e5681ee/third_party/WebKit/Source/bindings/core/v8/WindowProxyManager.cpp
,
Jan 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cbb1ee21e72f53c373d4a3468bfc8a6dfc10f514 commit cbb1ee21e72f53c373d4a3468bfc8a6dfc10f514 Author: haraken <haraken@chromium.org> Date: Thu Jan 05 14:03:52 2017 Remove the declaration of WindowProxy::updateDocumentWrapper The method is not defined. BUG= 677253 Review-Url: https://codereview.chromium.org/2612953003 Cr-Commit-Position: refs/heads/master@{#441646} [modify] https://crrev.com/cbb1ee21e72f53c373d4a3468bfc8a6dfc10f514/third_party/WebKit/Source/bindings/core/v8/WindowProxy.h
,
Jan 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5807dc6e5f6be9c395dc81b43335055f2d6a2a97 commit 5807dc6e5f6be9c395dc81b43335055f2d6a2a97 Author: haraken <haraken@chromium.org> Date: Fri Jan 06 01:46:07 2017 Remove ScriptController::namedItemAdded/Removed They are unnecessary indirection to WindowProxy::namedItemAdded/Removed. Also this CL updates a dcheck in WindowProxy::namedItemAdded/Removed to verify that those methods are called after the window proxy is initialized. (The check will be replaced with a better lifecycle model in a follow-up CL.) BUG= 677253 Review-Url: https://codereview.chromium.org/2616903002 Cr-Commit-Position: refs/heads/master@{#441803} [modify] https://crrev.com/5807dc6e5f6be9c395dc81b43335055f2d6a2a97/third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp [modify] https://crrev.com/5807dc6e5f6be9c395dc81b43335055f2d6a2a97/third_party/WebKit/Source/bindings/core/v8/ScriptController.h [modify] https://crrev.com/5807dc6e5f6be9c395dc81b43335055f2d6a2a97/third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp [modify] https://crrev.com/5807dc6e5f6be9c395dc81b43335055f2d6a2a97/third_party/WebKit/Source/core/html/HTMLDocument.cpp
,
Jan 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/40458d4dd913d5fd6f4f1bb2da083a8a7136a9af commit 40458d4dd913d5fd6f4f1bb2da083a8a7136a9af Author: haraken <haraken@chromium.org> Date: Fri Jan 06 07:09:44 2017 Remove ScriptController::initializeMainWorld ScriptController::updateDocument is unnecessarily complicated. Assuming that ScriptController::updateDocument is never called after the context is detached (I added a dcheck to verify the fact in WindowProxy::updateDocument()), we can simplify ScriptController::updateDocument. It also enables us to remove ScriptController::initializeMainWorld. BUG= 677253 Review-Url: https://codereview.chromium.org/2608163009 Cr-Commit-Position: refs/heads/master@{#441896} [modify] https://crrev.com/40458d4dd913d5fd6f4f1bb2da083a8a7136a9af/third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp [modify] https://crrev.com/40458d4dd913d5fd6f4f1bb2da083a8a7136a9af/third_party/WebKit/Source/bindings/core/v8/ScriptController.h [modify] https://crrev.com/40458d4dd913d5fd6f4f1bb2da083a8a7136a9af/third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp [modify] https://crrev.com/40458d4dd913d5fd6f4f1bb2da083a8a7136a9af/third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp [modify] https://crrev.com/40458d4dd913d5fd6f4f1bb2da083a8a7136a9af/third_party/WebKit/Source/core/loader/FrameLoader.cpp [modify] https://crrev.com/40458d4dd913d5fd6f4f1bb2da083a8a7136a9af/third_party/WebKit/Source/core/page/Page.cpp
,
Jan 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/efc489008d2094b83736b130cb8f5349ff0c4309 commit efc489008d2094b83736b130cb8f5349ff0c4309 Author: haraken <haraken@chromium.org> Date: Fri Jan 06 09:09:11 2017 Remove isGlobalInitialized Now it's unused. We can remove it. BUG= 677253 Review-Url: https://codereview.chromium.org/2619523003 Cr-Commit-Position: refs/heads/master@{#441906} [modify] https://crrev.com/efc489008d2094b83736b130cb8f5349ff0c4309/third_party/WebKit/Source/bindings/core/v8/WindowProxy.h
,
Jan 6 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/559b67fbc41fba740506cd5d6f3d20d20f65039b commit 559b67fbc41fba740506cd5d6f3d20d20f65039b Author: haraken <haraken@chromium.org> Date: Fri Jan 06 09:20:42 2017 Remove an isContextInitialized() check from ScriptController::windowProxy() WindowProxy::initializeIfNeeded() is no-op when isContextInitialized() returns true. So we can remove the check from ScriptController::windowProxy() and unconditionally call WindowProxy::initializeIfNeeded(). This CL will cause a subtle change: Before this CL, dispatchDidClearWindowObjectInMainWorld was not called if WindowProxy gets initialized via WindowProxy::setGlobal. After this CL, dispatchDidClearWindowObjectInMainWorld will be called always when WindowProxy gets initialized. I don't think the difference matters (and I guess the new behavior will be more correct). BUG= 677253 Review-Url: https://codereview.chromium.org/2613893002 Cr-Commit-Position: refs/heads/master@{#441908} [modify] https://crrev.com/559b67fbc41fba740506cd5d6f3d20d20f65039b/third_party/WebKit/Source/bindings/core/v8/ScriptController.cpp [modify] https://crrev.com/559b67fbc41fba740506cd5d6f3d20d20f65039b/third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp
,
Jan 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/381723f8f6664f04b9524e6212e218e1b1ec7510 commit 381723f8f6664f04b9524e6212e218e1b1ec7510 Author: haraken <haraken@chromium.org> Date: Tue Jan 10 06:21:42 2017 Introduce a lifecycle model to WindowProxy This CL intorduces the following lifecycle model to WindowProxy: ContextUninitialized => ContextInitialized => ContextDetached This clarifies the lifecycle and enables us to insert dchecks to detect invalid lifecycle transitions. BUG= 677253 Review-Url: https://codereview.chromium.org/2615213002 Cr-Commit-Position: refs/heads/master@{#442519} [modify] https://crrev.com/381723f8f6664f04b9524e6212e218e1b1ec7510/third_party/WebKit/Source/bindings/core/v8/V8BindingForTesting.cpp [modify] https://crrev.com/381723f8f6664f04b9524e6212e218e1b1ec7510/third_party/WebKit/Source/bindings/core/v8/WindowProxy.cpp [modify] https://crrev.com/381723f8f6664f04b9524e6212e218e1b1ec7510/third_party/WebKit/Source/bindings/core/v8/WindowProxy.h
,
Jan 10 2017
|
||
►
Sign in to add a comment |
||
Comment 1 by bugdroid1@chromium.org
, Dec 28 2016