Move Frame reference to be held by DOMWindow base class.

BUG=none

Review-Url: https://codereview.chromium.org/2626403002
Cr-Commit-Position: refs/heads/master@{#443501}

third_party/WebKit/Source/core/frame/DOMWindow.cpp

@@ -34,9 +34,12 @@
 
 namespace blink {
 
-DOMWindow::DOMWindow() : m_windowIsClosing(false) {}
+DOMWindow::DOMWindow(Frame& frame) : m_frame(frame), m_windowIsClosing(false) {}
 
-DOMWindow::~DOMWindow() {}
+DOMWindow::~DOMWindow() {
+  // The frame must be disconnected before finalization.
+  DCHECK(!m_frame);
+}
 
 v8::Local<v8::Object> DOMWindow::wrap(v8::Isolate*,
                                       v8::Local<v8::Object> creationContext) {
@@ -432,6 +435,7 @@ void DOMWindow::focus(ExecutionContext* context) {
 }
 
 DEFINE_TRACE(DOMWindow) {
+  visitor->trace(m_frame);
   visitor->trace(m_location);
   EventTargetWithInlineData::trace(visitor);
 }

third_party/WebKit/Source/core/frame/DOMWindow.h

@@ -9,9 +9,10 @@
 #include "core/CoreExport.h"
 #include "core/events/EventTarget.h"
 #include "core/frame/DOMWindowBase64.h"
+#include "core/frame/Frame.h"
 #include "platform/heap/Handle.h"
 #include "platform/scroll/ScrollableArea.h"
-
+#include "wtf/Assertions.h"
 #include "wtf/Forward.h"
 
 namespace blink {
@@ -26,7 +27,6 @@ class DOMVisualViewport;
 class Document;
 class Element;
 class External;
-class Frame;
 class FrameRequestCallback;
 class History;
 class IdleRequestCallback;
@@ -49,14 +49,20 @@ class CORE_EXPORT DOMWindow : public EventTargetWithInlineData,
  public:
   ~DOMWindow() override;
 
+  Frame* frame() const {
+    // If the DOMWindow still has a frame reference, that frame must point
+    // back to this DOMWindow: otherwise, it's easy to get into a situation
+    // where script execution leaks between different DOMWindows.
+    SECURITY_DCHECK(!m_frame || m_frame->domWindow() == this);
+    return m_frame;
+  }
+
   // GarbageCollectedFinalized overrides:
   DECLARE_VIRTUAL_TRACE();
 
   virtual bool isLocalDOMWindow() const = 0;
   virtual bool isRemoteDOMWindow() const = 0;
 
-  virtual Frame* frame() const = 0;
-
   // ScriptWrappable overrides:
   v8::Local<v8::Object> wrap(v8::Isolate*,
                              v8::Local<v8::Object> creationContext) final;
@@ -255,12 +261,18 @@ class CORE_EXPORT DOMWindow : public EventTargetWithInlineData,
   DEFINE_ATTRIBUTE_EVENT_LISTENER(orientationchange);
 
  protected:
-  DOMWindow();
+  explicit DOMWindow(Frame&);
 
   virtual void schedulePostMessage(MessageEvent*,
                                    PassRefPtr<SecurityOrigin> target,
                                    Document* source) = 0;
 
+  void disconnectFromFrame() { m_frame = nullptr; }
+
+ private:
+  Member<Frame> m_frame;
+  mutable Member<Location> m_location;
+
   // Set to true when close() has been called. Needed for
   // |window.closed| determinism; having it return 'true'
   // only after the layout widget's deferred window close
@@ -268,8 +280,6 @@ class CORE_EXPORT DOMWindow : public EventTargetWithInlineData,
   // implementation details to scripts.
   bool m_windowIsClosing;
 
- private:
-  mutable Member<Location> m_location;
 };
 
 }  // namespace blink

third_party/WebKit/Source/core/frame/LocalDOMWindow.cpp

@@ -273,7 +273,7 @@ bool LocalDOMWindow::allowPopUp() {
 }
 
 LocalDOMWindow::LocalDOMWindow(LocalFrame& frame)
-    : m_frame(&frame),
+    : DOMWindow(frame),
       m_visualViewport(DOMVisualViewport::create(this)),
       m_unusedPreloadsTimer(
           TaskRunnerHelper::get(TaskType::UnspecedTimer, &frame),
@@ -485,7 +485,7 @@ void LocalDOMWindow::frameDestroyed() {
   resetLocation();
   m_properties.clear();
   removeAllEventListeners();
-  m_frame = nullptr;
+  disconnectFromFrame();
 }
 
 void LocalDOMWindow::registerProperty(DOMWindowProperty* property) {
@@ -1608,7 +1608,6 @@ DOMWindow* LocalDOMWindow::open(const String& urlString,
 }
 
 DEFINE_TRACE(LocalDOMWindow) {
-  visitor->trace(m_frame);
   visitor->trace(m_document);
   visitor->trace(m_properties);
   visitor->trace(m_screen);
@@ -1636,12 +1635,4 @@ DEFINE_TRACE_WRAPPERS(LocalDOMWindow) {
   DOMWindow::traceWrappers(visitor);
 }
 
-LocalFrame* LocalDOMWindow::frame() const {
-  // If the LocalDOMWindow still has a frame reference, that frame must point
-  // back to this LocalDOMWindow: otherwise, it's easy to get into a situation
-  // where script execution leaks between different LocalDOMWindows.
-  SECURITY_DCHECK(!m_frame || m_frame->domWindow() == this);
-  return m_frame;
-}
-
 }  // namespace blink

third_party/WebKit/Source/core/frame/LocalDOMWindow.h

@@ -82,6 +82,8 @@ class CORE_EXPORT LocalDOMWindow final : public DOMWindow,
 
   ~LocalDOMWindow() override;
 
+  LocalFrame* frame() const { return toLocalFrame(DOMWindow::frame()); }
+
   DECLARE_VIRTUAL_TRACE();
   DECLARE_VIRTUAL_TRACE_WRAPPERS();
 
@@ -95,7 +97,6 @@ class CORE_EXPORT LocalDOMWindow final : public DOMWindow,
   LocalDOMWindow* toLocalDOMWindow() override;
 
   // DOMWindow overrides:
-  LocalFrame* frame() const override;
   Screen* screen() const override;
   History* history() const override;
   BarProp* locationbar() const override;
@@ -260,7 +261,6 @@ class CORE_EXPORT LocalDOMWindow final : public DOMWindow,
 
   void willDetachFrameHost();
 
-  Member<LocalFrame> m_frame;
   Member<Document> m_document;
   Member<DOMVisualViewport> m_visualViewport;
   TaskRunnerTimer<LocalDOMWindow> m_unusedPreloadsTimer;

third_party/WebKit/Source/core/frame/RemoteDOMWindow.cpp

@@ -18,14 +18,9 @@ ExecutionContext* RemoteDOMWindow::getExecutionContext() const {
 }
 
 DEFINE_TRACE(RemoteDOMWindow) {
-  visitor->trace(m_frame);
   DOMWindow::trace(visitor);
 }
 
-RemoteFrame* RemoteDOMWindow::frame() const {
-  return m_frame.get();
-}
-
 Screen* RemoteDOMWindow::screen() const {
   ASSERT_NOT_REACHED();
   return nullptr;
@@ -290,16 +285,16 @@ CustomElementRegistry* RemoteDOMWindow::customElements(ScriptState*) const {
   return nullptr;
 }
 
-RemoteDOMWindow::RemoteDOMWindow(RemoteFrame& frame) : m_frame(&frame) {}
+RemoteDOMWindow::RemoteDOMWindow(RemoteFrame& frame) : DOMWindow(frame) {}
 
 void RemoteDOMWindow::frameDetached() {
-  m_frame = nullptr;
+  disconnectFromFrame();
 }
 
 void RemoteDOMWindow::schedulePostMessage(MessageEvent* event,
                                           PassRefPtr<SecurityOrigin> target,
                                           Document* source) {
-  m_frame->client()->forwardPostMessage(event, std::move(target),
+  frame()->client()->forwardPostMessage(event, std::move(target),
                                         source->frame());
 }
 

third_party/WebKit/Source/core/frame/RemoteDOMWindow.h

@@ -17,12 +17,13 @@ class RemoteDOMWindow final : public DOMWindow {
     return new RemoteDOMWindow(frame);
   }
 
+  RemoteFrame* frame() const { return toRemoteFrame(DOMWindow::frame()); }
+
   // EventTarget overrides:
   ExecutionContext* getExecutionContext() const override;
 
   // DOMWindow overrides:
   DECLARE_VIRTUAL_TRACE();
-  RemoteFrame* frame() const override;
   Screen* screen() const override;
   History* history() const override;
   BarProp* locationbar() const override;
@@ -106,8 +107,6 @@ class RemoteDOMWindow final : public DOMWindow {
   // already RemoteDOMWindow.
   bool isLocalDOMWindow() const override { return false; }
   bool isRemoteDOMWindow() const override { return true; }
-
-  Member<RemoteFrame> m_frame;
 };
 
 DEFINE_TYPE_CASTS(RemoteDOMWindow,

third_party/WebKit/Source/core/frame/RemoteFrame.cpp

@@ -103,6 +103,12 @@ void RemoteFrame::detach(FrameDetachType type) {
   client()->willBeDetached();
   m_windowProxyManager->clearForClose();
   setView(nullptr);
+  // ... the RemoteDOMWindow will need to be informed of detachment,
+  // as otherwise it will keep a strong reference back to this RemoteFrame.
+  // That combined with wrappers (owned and kept alive by RemoteFrame) keeping
+  // persistent strong references to RemoteDOMWindow will prevent the GCing
+  // of all these objects. Break the cycle by notifying of detachment.
+  toRemoteDOMWindow(m_domWindow)->frameDetached();
   if (m_webLayer)
     setWebLayer(nullptr);
   Frame::detach(type);
@@ -144,14 +150,6 @@ void RemoteFrame::setView(RemoteFrameView* view) {
   // Oilpan: as RemoteFrameView performs no finalization actions,
   // no explicit dispose() of it needed here. (cf. FrameView::dispose().)
   m_view = view;
-
-  // ... the RemoteDOMWindow will need to be informed of detachment,
-  // as otherwise it will keep a strong reference back to this RemoteFrame.
-  // That combined with wrappers (owned and kept alive by RemoteFrame) keeping
-  // persistent strong references to RemoteDOMWindow will prevent the GCing
-  // of all these objects. Break the cycle by notifying of detachment.
-  if (!m_view)
-    toRemoteDOMWindow(m_domWindow)->frameDetached();
 }
 
 void RemoteFrame::createView() {