Refactor WindowProxy into Local and Remote subclasses.

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

Index: third_party/WebKit/Source/bindings/core/v8/WindowProxyManager.h
diff --git a/third_party/WebKit/Source/bindings/core/v8/WindowProxyManager.h b/third_party/WebKit/Source/bindings/core/v8/WindowProxyManager.h
index e785aff91e4b21695a72637a0407f71273d9c028..0509a3249d4f3afad0d7e2fd459be1e8cb15131b 100644
--- a/third_party/WebKit/Source/bindings/core/v8/WindowProxyManager.h
+++ b/third_party/WebKit/Source/bindings/core/v8/WindowProxyManager.h
@@ -5,52 +5,100 @@
 #ifndef WindowProxyManager_h
 #define WindowProxyManager_h
 
+#include "bindings/core/v8/LocalWindowProxy.h"
+#include "bindings/core/v8/RemoteWindowProxy.h"
 #include "core/CoreExport.h"
+#include "core/frame/LocalFrame.h"
+#include "core/frame/RemoteFrame.h"
 #include "platform/heap/Handle.h"
-#include "wtf/Vector.h"
 #include <utility>
 #include <v8.h>
 
 namespace blink {
 
 class DOMWrapperWorld;
-class Frame;
 class SecurityOrigin;
-class WindowProxy;
+class ScriptController;
 
-class CORE_EXPORT WindowProxyManager final
-    : public GarbageCollected<WindowProxyManager> {
+class WindowProxyManagerBase : public GarbageCollected<WindowProxyManagerBase> {
  public:
-  static WindowProxyManager* create(Frame&);
-
   DECLARE_TRACE();
 
-  Frame* frame() const { return m_frame.get(); }
   v8::Isolate* isolate() const { return m_isolate; }
-  WindowProxy* mainWorldProxy() const { return m_windowProxy.get(); }
 
+  void clearForClose();
+  void CORE_EXPORT clearForNavigation();
+
+  void CORE_EXPORT
+  releaseGlobals(HashMap<DOMWrapperWorld*, v8::Local<v8::Object>>&);
+  void CORE_EXPORT
+  setGlobals(const HashMap<DOMWrapperWorld*, v8::Local<v8::Object>>&);
+
+ protected:
+  explicit WindowProxyManagerBase(Frame&);
+
+  Frame* frame() const { return m_frame; }
+  WindowProxy* mainWorldProxy() const { return m_windowProxy.get(); }
   WindowProxy* windowProxy(DOMWrapperWorld&);
 
-  void clearForClose();
-  void clearForNavigation();
+ private:
+  using IsolatedWorldMap = HeapHashMap<int, Member<WindowProxy>>;
+
+  const Member<Frame> m_frame;
+  v8::Isolate* const m_isolate;
+
+ protected:

[Yuki, 2017/01/11 10:09:24]

nit: Could you unify two protected: sections into one?
IIRC, our coding style recommends one public:, protected:, and private: section
per class.

[dcheng, 2017/01/11 10:35:29]

Made this private and added protected getters for LocalWindowProxyManager.

+  const Member<WindowProxy> m_windowProxy;
+  IsolatedWorldMap m_isolatedWorlds;
+};
+
+template <typename FrameType, typename ProxyType>
+class WindowProxyManager : public WindowProxyManagerBase {

[Yuki, 2017/01/11 10:09:24]

nit: I guess "WindowProxyManager" is not a good name for this template class. 
This template class looks a kind of hacks to make the following implementations
easy, and doesn't have meaningful role as other classes have.  I personally
don't want to give this very nice and general name to this class.

My understanding is:
  WindowProxyManagerBase = the unified root class
  LocalWindowProxyManager = local version
  RemoteWindowProxyManager = remote version
and we don't need to expose this class to anyone.

What about WindowProxyManager{Internal,ImplHelper} or something like that?

[dcheng, 2017/01/11 10:35:29]

Renamed it ImplHelper.

+ private:
+  using Base = WindowProxyManagerBase;
+
+ public:
+  FrameType* frame() const { return static_cast<FrameType*>(Base::frame()); }
+  ProxyType* mainWorldProxy() const {
+    return static_cast<ProxyType*>(Base::mainWorldProxy());
+  }
+  ProxyType* windowProxy(DOMWrapperWorld& world) {
+    return static_cast<ProxyType*>(Base::windowProxy(world));
+  }
+
+ protected:
+  explicit WindowProxyManager(Frame& frame) : WindowProxyManagerBase(frame) {}
+};
+
+class LocalWindowProxyManager
+    : public WindowProxyManager<LocalFrame, LocalWindowProxy> {
+ public:
+  static LocalWindowProxyManager* create(LocalFrame& frame) {
+    return new LocalWindowProxyManager(frame);
+  }
 
   // Sets the given security origin to the main world's context.  Also updates
   // the security origin of the context for each isolated world.
   void updateSecurityOrigin(SecurityOrigin*);
 
-  void releaseGlobals(HashMap<DOMWrapperWorld*, v8::Local<v8::Object>>&);
-  void setGlobals(const HashMap<DOMWrapperWorld*, v8::Local<v8::Object>>&);
-
  private:
-  typedef HeapHashMap<int, Member<WindowProxy>> IsolatedWorldMap;
+  // TODO(dcheng): Merge LocalWindowProxyManager and ScriptController?
+  friend class ScriptController;
 
-  explicit WindowProxyManager(Frame&);
+  LocalWindowProxyManager(LocalFrame& frame)
+      : WindowProxyManager<LocalFrame, LocalWindowProxy>(frame) {}
+};
 
-  Member<Frame> m_frame;
-  v8::Isolate* const m_isolate;
+class RemoteWindowProxyManager
+    : public WindowProxyManager<RemoteFrame, RemoteWindowProxy> {
+ public:
+  static RemoteWindowProxyManager* create(RemoteFrame& frame) {
+    return new RemoteWindowProxyManager(frame);
+  }
 
-  const Member<WindowProxy> m_windowProxy;
-  IsolatedWorldMap m_isolatedWorlds;
+ private:
+  RemoteWindowProxyManager(RemoteFrame& frame)
+      : WindowProxyManager<RemoteFrame, RemoteWindowProxy>(frame) {}
 };
 
 }  // namespace blink