Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(171)

Unified Diff: third_party/WebKit/Source/bindings/core/v8/WindowProxy.h

Issue 2769803003: v8binding: Initializes WindowProxy iff it's uninitialized. (Closed)
Patch Set: Synced. Created 3 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/bindings/core/v8/WindowProxy.h
diff --git a/third_party/WebKit/Source/bindings/core/v8/WindowProxy.h b/third_party/WebKit/Source/bindings/core/v8/WindowProxy.h
index 5b012aaf1a0dc628afc0646d9bd629844ccc74de..4b6ce0665ace7aec32840325bd463979019e1871 100644
--- a/third_party/WebKit/Source/bindings/core/v8/WindowProxy.h
+++ b/third_party/WebKit/Source/bindings/core/v8/WindowProxy.h
@@ -42,7 +42,6 @@ namespace blink {
class DOMWindow;
class Frame;
-class ScriptController;
// WindowProxy implements the split window model of a window for a frame. In the
// HTML standard, the split window model is composed of the Window interface
@@ -163,16 +162,56 @@ class WindowProxy : public GarbageCollectedFinalized<WindowProxy> {
virtual bool isLocal() const { return false; }
protected:
- // TODO(dcheng): Remove this friend declaration once LocalWindowProxyManager
- // and ScriptController are merged.
- friend class ScriptController;
-
- // A valid transition is from ContextUninitialized to ContextInitialized,
- // and then ContextDetached. Other transitions are forbidden.
+ // Lifecycle represents the following four states.
+ //
+ // * ContextUninitialized
+ // We lazily initialize WindowProxies for performance reasons, and this state
+ // is "to be initialized on demand". WindowProxy basically behaves the same as
+ // |ContextInitialized| from a point of view of call sites.
+ // - Possible next states: ContextInitialized
+ // It's possible to detach the context from a frame or navigate to a new page
+ // without initializing the WindowProxy, however, there is no transition to
+ // |ContextDetachedFromFrame| or |GlobalObjectDetached| because
+ // |disposeContext| does not change the state if the state is
+ // |ContextUninitialized|. In either case of a) the browsing context is
+ // detached from a frame or b) the page is navigated away, there must be no
+ // way for author script to access the context of |ContextUninitialized|
+ // because |ContextUninitialized| means that author script has never accessed
+ // the context, hence there must exist no reference to the context.
+ //
+ // * ContextInitialized
+ // The context is initialized and still attached to a frame.
+ // - Possible next states: ContextDetachedFromFrame, GlobalObjectDetached
+ //
+ // * ContextDetachedFromFrame
+ // The context is initialized, once attached to a frame and now detached. Note
dcheng 2017/03/28 21:25:42 Hmm... maybe be more explicit that it's the frame
Yuki 2017/03/30 14:48:37 Done.
+ // that the context is still alive and author script may have references to
+ // the context and hence author script may run in the context.
+ // The spec does not support some of web features such as setTimeout, etc. on
+ // a detached window. Blink supports less things than the spec.
+ // V8PerContextData is cut off from the context.
+ // - Possible next states: n/a
+ //
+ // * GlobalObjectDetached
+ // The context is initialized, attached to a frame, and now navigated away.
+ // The global object (inner global) is detached from the global proxy (outer
+ // global), but the (detached) global object and context are still alive, and
+ // author script may have references to the context.
dcheng 2017/03/28 21:25:42 May be worth noting that this state is also used w
Yuki 2017/03/30 14:48:37 Good to know. When swapping frames, currently we
dcheng 2017/03/30 21:03:01 We actually do =) See https://cs.chromium.org/chro
Yuki 2017/03/31 07:32:09 Oops, thank you for pointing it out. I've added a
+ // The spec does not support full web features in this state. Blink supports
+ // less things than the spec.
+ // - Possible next states: ContextInitialized
+ // This state is in the middle of navigation. Once it's completed and the
+ // context gets accessed, the WindowProxy is lazily re-initialized for a new
dcheng 2017/03/28 21:25:42 It might be more accurate to say that the context
Yuki 2017/03/30 14:48:38 Done.
+ // window and document, and then the state becomes |ContextInitialized|.
enum class Lifecycle {
+ // v8::Context is not yet initialized.
ContextUninitialized,
+ // v8::Context is initialized.
ContextInitialized,
- ContextDetached,
+ // A context is detached from a frame.
+ ContextDetachedFromFrame,
dcheng 2017/03/28 21:25:42 Nit: I think it would be more accurate to call thi
haraken 2017/03/29 08:54:53 ContextIsNotInitialized, ContextIsInitialized, Glo
Yuki 2017/03/30 14:48:38 Good points. Updated to: ContextIsNotInitialized
+ // The global object (inner global) is detached from the global proxy.
+ GlobalObjectDetached,
};
WindowProxy(v8::Isolate*, Frame&, RefPtr<DOMWrapperWorld>);

Powered by Google App Engine
This is Rietveld 408576698