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

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

Issue 2769803003: v8binding: Initializes WindowProxy iff it's uninitialized. (Closed)
Patch Set: . 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..01932edaae6b8c404533446e95805047112499dc 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,55 @@ 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 to the context of |ContextUninitialized|
haraken 2017/03/28 11:12:42 access the context
Yuki 2017/03/28 12:22:16 Done.
+ // because |ContextUninitialized| means that author script has never accessed
+ // to the context, hence there must exist no reference to the context.
haraken 2017/03/28 11:12:42 accessed the context
Yuki 2017/03/28 12:22:16 Done.
+ //
+ // * ContextInitialized
+ // The context is initialized and yet attached to a frame.
haraken 2017/03/28 11:12:42 yet => still
Yuki 2017/03/28 12:22:16 Done.
+ // - Possible next states: ContextDetachedFromFrame, GlobalObjectDetached
+ //
+ // * ContextDetachedFromFrame
+ // The context is initialized, once attached to a frame and now detached. Note
+ // 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 does not support more things than the spec,
haraken 2017/03/28 11:12:42 // Blink supports less things than the spec.
Yuki 2017/03/28 12:22:16 Done.
+ // though. V8PerContextData is cut off from the context.
+ // - Possible next states: n/a
haraken 2017/03/28 11:12:42 Isn't it possible to transit from ContextDetachedF
Yuki 2017/03/28 12:22:16 IIUC, no way. GlobalObjectDetached means that it'
+ //
+ // * 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 reference to the context.
haraken 2017/03/28 11:12:42 references
Yuki 2017/03/28 12:22:16 Done.
+ // The spec does not support some of web features as same as
+ // |ContextDetachedFromFrame|. Blink does not support this state well samely.
haraken 2017/03/28 11:12:42 // The spec does not support full web features in
Yuki 2017/03/28 12:22:16 Done.
+ // - Possible next states: ContextUninitialized
haraken 2017/03/28 11:12:42 ContextUninitialized or destructor ?
Yuki 2017/03/28 12:22:16 Ditto.
+ // This state is in the middle of navigation, so the next state is
+ // |ContextUninitialized| for a new window/context/document of a new page.
haraken 2017/03/28 11:12:42 This comment looks a bit confusing. It's possible
Yuki 2017/03/28 12:22:16 Oops, I was wrong on this point. GlobalObjectDeta
enum class Lifecycle {
+ // v8::Context is not yet initialized.
ContextUninitialized,
+ // v8::Context is initialized.
ContextInitialized,
- ContextDetached,
+ // A context is detached from a frame.
+ ContextDetachedFromFrame,
+ // 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