Switch RemoteWindowProxy to use v8::Context::NewRemoteContext.

third_party/WebKit/Source/bindings/core/v8/WindowProxy.h

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 9089f013b83894a81d2d280d5737db54716a7745..511f070ae0470017bfa073c2e34c0254ceb96c0d 100644
--- a/third_party/WebKit/Source/bindings/core/v8/WindowProxy.h
+++ b/third_party/WebKit/Source/bindings/core/v8/WindowProxy.h
@@ -31,35 +31,125 @@
 #ifndef WindowProxy_h
 #define WindowProxy_h
 
+#include <v8.h>
 #include "bindings/core/v8/DOMWrapperWorld.h"
 #include "bindings/core/v8/ScopedPersistent.h"
-#include "bindings/core/v8/ScriptState.h"
+#include "core/CoreExport.h"
 #include "platform/heap/Handle.h"
 #include "wtf/RefPtr.h"
-#include <v8.h>
 
 namespace blink {
 
 class Frame;
-class ScriptController;
 
-// WindowProxy represents all the per-global object state for a Frame that
-// persist between navigations.
+// WindowProxy implements the split window model for a Frame. The split window
+// model consists of the inner global object and the outer global proxy.

[Yuki, 2017/02/16 12:47:39]

I'd like to have a note about which term in Blink corresponds to which term in
HTML spec, i.e. "inner global object" corresponds to Window and "outer global
proxy" corresponds to WindowProxy in HTML spec.

I also think we'd like to note that WindowProxy class in Blink doesn't
(directly) correspond to WindowProxy in HTML spec.

[dcheng, 2017/02/16 19:50:23]

Done. I reworded a lot of this comment; hopefully, it's a bit more
understandable now.

+//
+// ====== Inner Global Object ======
+// The inner global object contains the global for the script environment a
+// Frame. In Blink, the inner global object has a 1:1 relationship with the
+// DOMWindow implementation class. All methods and attributes defined on the
+// Window interface are exposed via the inner global object. Variables defined

[Yuki, 2017/02/16 12:47:39]

Variables defined by ???

[dcheng, 2017/02/16 19:50:23]

Done =)

+// by
+//
+// The inner global object is not reused across navigations: each inner global
+// object has an associated Document which does not change. On navigation, the
+// new Document receives a new inner global object.
+//
+// There is one exception to the previous rule. If:
+// - the previous Document is the initial empty document
+// - the new Document is same-origin to the previous Document
+// then the inner global object will be reused for the new Document.
+//
+// ====== Outer Global Proxy ====
+// The outer global proxy is reused across navigations. It implements the
+// security checks for same-origin/cross-origin access to the Window interface.

[Yuki, 2017/02/16 12:47:39]

Is this correct?

AccessCheckCallbackAndHandler (i.e. V8Window::securityCheck) is set on the inner
global object in case of local.

My understanding is that everything including security checks is done on the
inner global object if it's local.  The outer global proxy's role is to detach
the global object across navigations.

[dcheng, 2017/02/16 19:50:23]

This is actually something I'm not sure about. From a conceptual point of view,
I think we should think of the security checks as belonging on the outer global
proxy itself. However, from an implementation perspective, I think we currently
have access checks on both the proxy template and the global template. See
https://cs.chromium.org/chromium/src/v8/src/api.cc?rcl=547f4de8ab3155d315bba8...

+jochen I wonder if we can remove this, now that we're using cross-origin
interceptors.

+// When the check passes (i.e. the access is same-origin), the access is
+// forwarded to the inner global object of the active Document in this
+// WindowProxy's Frame).
+//
+// When the security check fails, the access is delegated to the outer global
+// proxy's cross-origin interceptors. The cross-origin interceptors may choose
+// to return a value (if the property is exposed cross-origin) or throw an
+// exception otherwise.
+//
+// Note that the cross-origin interceptors are only used for cross-origin
+// accesses: a same-origin access to a method that is available cross-origin,
+// such as Window.postMessage, will be delegated to the inner global object.
+//
+// ====== LocalWindowProxy vs RemoteWindowProxy ======
+// In Blink, there is also the concept of a LocalWindowProxy and a
+// RemoteWindowProxy. A LocalWindowProxy implements the split window model for a
+// frame in the same process, while a RemoteWindowProxy implements the split
+// window model for a frame in a different process. Note that while having a
+// RemoteWindowProxy implies a cross-origin frame, the reverse is not true: a
+// LocalWindowProxy could be same-origin or cross-origin.

[Yuki, 2017/02/16 12:47:39]

I think that it's worth mentioning that we break the definition of the same
origin in the spec.  Even if two pages are the same origin in the spec, if it's
served in a remote frame, then we treat as they're cross origin.

I think it's worth writing comments whenever we intentionally do different
things than the spec.

[dcheng, 2017/02/16 19:50:23]

Done.

+//
+// ====== LocalWindowProxy ======
+// Since a LocalWindowProxy can represent a same-origin or cross-origin frame,
+// the entire prototype chain must be available:
+//
+//   outer global proxy
+//     -- has prototype --> inner global object
+//     -- has prototype --> Window.prototype
+//     -- has prototype --> WindowProperties [1]
+//     -- has prototype --> EventTarget.prototype
+//     -- has prototype --> Object.prototype
+//     -- has prototype --> null
+//
+// [1] WindowProperties is the named properties object of the Window interface.
+//
+// ====== RemoteWindowProxy ======
+// Since a RemoteWindowProxy only represents a cross-origin frame, it has a much
+// simpler prototype chain.
+//
+//   outer global proxy
+//     -- null

[Yuki, 2017/02/16 12:47:39]

Is this really null?  I thought it's an empty handle.  v8::Local<v8::Object>()
is different from v8::Null(isolate).

[dcheng, 2017/02/16 19:50:23]

From my printf debugging, I believe this is correct. Also see
https://cs.chromium.org/chromium/src/v8/src/bootstrapper.cc?rcl=547f4de8ab315...

[Yuki, 2017/02/17 06:49:31]

Thanks for checking.

nit: you might want to make it
  -- has prototype --> null
to follow the above style (it's a hidden prototype, though).

[dcheng, 2017/02/17 07:16:52]

Done.

+//
+// Property access to get/set attributes and methods on the outer global proxy
+// are redirected through the cross-origin interceptors, since any access will
+// fail the security check, by definition.
+//
+// ====== Functions that use Window as the receiver object ======
+// Functions that use Window as the receiver object have to deal with an
+// additional complication. Typically, v8 performs a check that the receiver
+// object is compatible by walking the prototype chain and looking for a
+// matching v8::ObjectTemplate. In this case, v8 will use Window's
+// v8::ObjectTemplate to perform the receiver check.
+//
+// However, the outer global proxy's v8::ObjectTemplate is *not* the Window's
+// v8::ObjectTemplate. Internally, v8 always creates a new v8::ObjectTemplate
+// for each global proxy object (the proxy template) and sets the proxy
+// template's constructor's prototype template to Window's v8::ObjectTemplate.
+//
+// For LocalWindowProxy, this is fine: the inner global object is still created
+// using Window's v8::ObjectTemplate, so walking up the prototype chain will
+// find a compatible receiver.
+//
+// However, RemoteWindowProxy doesn't have an inner global object, so the
+// compatible receiver check will fail. To get around this, all method callbacks
+// for the Window interface perform the receiver check in the Blink bindings
+// using V8Window::hasInstance().
+//
+// Note that this hack is only needed for method callbacks, since
+// method/attribute getters/setters are handled by the cross-origin interceptor,
+// which doesn't need to perform receiver checks.

[Yuki, 2017/02/16 12:47:39]

Currently cross origin accessible attributes are implemented as data properties
(not accessor properties), so this works.  However, once we implement them as
accessor properties, we'll need the same things as methods because author script
can extract get and set function objects.

[dcheng, 2017/02/16 19:50:23]

This comment is attempting to document things as they are (well, mostly. I
cheated a little on the access checks comments when talking about the outer
global proxy). However, for this comment, I think we should only update this to
mention data properties when we actually change how it's implemented.

+//
+// ====== References ======
+// https://wiki.mozilla.org/Gecko:SplitWindow
+// https://whatwg.org/C/browsers.html#the-windowproxy-exotic-object
 class WindowProxy : public GarbageCollectedFinalized<WindowProxy> {
  public:
   virtual ~WindowProxy();
 
   DECLARE_TRACE();
 
-  v8::Local<v8::Context> contextIfInitialized() const {
-    return m_scriptState ? m_scriptState->context() : v8::Local<v8::Context>();
-  }
   void initializeIfNeeded();
 
   void clearForClose();
   void clearForNavigation();
 
-  v8::Local<v8::Object> globalIfNotDetached();
+  CORE_EXPORT v8::Local<v8::Object> globalIfNotDetached();
   v8::Local<v8::Object> releaseGlobal();
   void setGlobal(v8::Local<v8::Object>);
 
@@ -68,10 +158,6 @@ class WindowProxy : public GarbageCollectedFinalized<WindowProxy> {
   DOMWrapperWorld& world() { return *m_world; }
 
  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.
   enum class Lifecycle {
@@ -85,24 +171,24 @@ class WindowProxy : public GarbageCollectedFinalized<WindowProxy> {
   virtual void initialize() = 0;
 
   enum GlobalDetachmentBehavior { DoNotDetachGlobal, DetachGlobal };
-  virtual void disposeContext(GlobalDetachmentBehavior);
-
-  // Associates the window wrapper and its prototype chain with the native
-  // DOMWindow object.  Also does some more Window-specific initialization.
-  void setupWindowPrototypeChain();
+  virtual void disposeContext(GlobalDetachmentBehavior) = 0;
 
   v8::Isolate* isolate() const { return m_isolate; }
   Frame* frame() const { return m_frame.get(); }
-  ScriptState* getScriptState() const { return m_scriptState.get(); }
+
+#if DCHECK_IS_ON()
+  void didAttachGlobalProxy() { m_isGlobalProxyAttached = true; }
+  void didDetachGlobalProxy() { m_isGlobalProxyAttached = false; }
+#endif
 
  private:
   v8::Isolate* const m_isolate;
   const Member<Frame> m_frame;
+#if DCHECK_IS_ON()
+  bool m_isGlobalProxyAttached = false;
+#endif
 
  protected:
-  // TODO(dcheng): Move this to LocalWindowProxy once RemoteWindowProxy uses
-  // remote contexts.
-  RefPtr<ScriptState> m_scriptState;
   // TODO(dcheng): Consider making these private and using getters.
   const RefPtr<DOMWrapperWorld> m_world;
   ScopedPersistent<v8::Object> m_globalProxy;