Move Frame to the Oilpan heap.

Source/core/frame/LocalFrame.cpp

 /*
  * Copyright (C) 1998, 1999 Torben Weis <weis@kde.org>
  *                     1999 Lars Knoll <knoll@kde.org>
  *                     1999 Antti Koivisto <koivisto@kde.org>
  *                     2000 Simon Hausmann <hausmann@kde.org>
  *                     2000 Stefan Schimanski <1Stein@gmx.de>
  *                     2001 George Staikos <staikos@kde.org>
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011 Apple Inc. All rights reserved.
  * Copyright (C) 2005 Alexey Proskuryakov <ap@nypop.com>
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
@@ skipping 88 matching lines @@
     , m_selection(FrameSelection::create(this))
     , m_eventHandler(adoptPtrWillBeNoop(new EventHandler(this)))
     , m_console(FrameConsole::create(*this))
     , m_inputMethodController(InputMethodController::create(*this))
     , m_pageZoomFactor(parentPageZoomFactor(this))
     , m_textZoomFactor(parentTextZoomFactor(this))
     , m_inViewSourceMode(false)
 {
 }
 
-PassRefPtr<LocalFrame> LocalFrame::create(FrameLoaderClient* client, FrameHost* host, FrameOwner* owner)
+PassRefPtrWillBeRawPtr<LocalFrame> LocalFrame::create(FrameLoaderClient* client, FrameHost* host, FrameOwner* owner)
 {
-    RefPtr<LocalFrame> frame = adoptRef(new LocalFrame(client, host, owner));
+    RefPtrWillBeRawPtr<LocalFrame> frame = adoptRefWillBeNoop(new LocalFrame(client, host, owner));
     InspectorInstrumentation::frameAttachedToParent(frame.get());
     return frame.release();
 }
 
 LocalFrame::~LocalFrame()
 {
+#if ENABLE(OILPAN)
+    // With Oilpan, the FrameView is detached. A more complete
+    // setView(nullptr) will touch other on-heap objects, which
+    // is not allowed.
+    detachView();
+    m_view = nullptr;
+    loader().dispose(FrameLoader::DoNotDisposeFrameContents);
+    // Oilpan: see setDOMWindow() comment why it is acceptable
+    // not to mirror the non-Oilpan call below.
+    //
+    // Also, FrameDestructionObservers that live longer than this
+    // frame object keep weak references to the frame; those will
+    // be automatically cleared by the garbage collector. Hence,
+    // no need to trigger frameDestroyed() notifications.
+#else
     setView(nullptr);
-    loader().clear();
+    loader().dispose(FrameLoader::DoDisposeFrameContents);
     setDOMWindow(nullptr);
 
-    // FIXME: What to do here... some of this is redundant with ~Frame.
-    HashSet<FrameDestructionObserver*>::iterator stop = m_destructionObservers.end();
-    for (HashSet<FrameDestructionObserver*>::iterator it = m_destructionObservers.begin(); it != stop; ++it)
+    HashSet<RawPtr<FrameDestructionObserver> >::iterator stop = m_destructionObservers.end();
+    for (HashSet<RawPtr<FrameDestructionObserver> >::iterator it = m_destructionObservers.begin(); it != stop; ++it)
         (*it)->frameDestroyed();
+#endif
+}
+
+void LocalFrame::trace(Visitor* visitor)
+{
+#if ENABLE(OILPAN)
+    visitor->trace(m_destructionObservers);
+#endif
+    visitor->trace(m_loader);
+    visitor->trace(m_navigationScheduler);
+    visitor->trace(m_pagePopupOwner);
+    visitor->trace(m_editor);
+    visitor->trace(m_spellChecker);
+    visitor->trace(m_selection);
+    visitor->trace(m_eventHandler);
+    visitor->trace(m_console);
+    visitor->trace(m_inputMethodController);
+    Frame::trace(visitor);
+    WillBeHeapSupplementable<LocalFrame>::trace(visitor);
 }
 
 void LocalFrame::detach()
 {
     // A lot of the following steps can result in the current frame being
     // detached, so protect a reference to it.
-    RefPtr<LocalFrame> protect(this);
+    RefPtrWillBeRawPtr<LocalFrame> protect(this);
     m_loader.stopAllLoaders();
     m_loader.closeURL();
     detachChildren();
     // stopAllLoaders() needs to be called after detachChildren(), because detachChildren()
     // will trigger the unload event handlers of any child frames, and those event
     // handlers might start a new subresource load in this frame.
     m_loader.stopAllLoaders();
     m_loader.detachFromParent();
 }
 
 bool LocalFrame::inScope(TreeScope* scope) const
 {
     ASSERT(scope);
     Document* doc = document();
     if (!doc)
         return false;
     // FIXME: This check is broken in for OOPI.
     HTMLFrameOwnerElement* owner = doc->ownerElement();
     if (!owner)
         return false;
     return owner->treeScope() == scope;
 }
 
+void LocalFrame::detachView()
+{
+    // We detach the FrameView's custom scroll bars as early as
+    // possible to prevent m_doc->detach() from messing with the view
+    // such that its scroll bars won't be torn down.
+    //
+    // FIXME: We should revisit this.
+    if (m_view)
+        m_view->prepareForDetach();
+}
+
 void LocalFrame::setView(PassRefPtr<FrameView> view)
 {
-    // We the custom scroll bars as early as possible to prevent m_doc->detach()
-    // from messing with the view such that its scroll bars won't be torn down.
-    // FIXME: We should revisit this.
-    if (m_view)
-        m_view->prepareForDetach();
+    if (!view)

[Mads Ager (chromium), 2014/09/16 12:17:45]

Why this extra check compared to before?

[sof, 2014/09/17 09:42:58]

It is not needed really, just pointlessly optimizing call patterns. Retired.

+        detachView();
 
     // Prepare for destruction now, so any unload event handlers get run and the LocalDOMWindow is
     // notified. If we wait until the view is destroyed, then things won't be hooked up enough for
     // these calls to work.
     if (!view && document() && document()->isActive()) {
         // FIXME: We don't call willRemove here. Why is that OK?
         document()->prepareForDestruction();

[Mads Ager (chromium), 2014/09/16 12:17:45]

Are we losing this with Oilpan on destruction because we are not doing a
setView(0). If so, is that OK? Looks like that will disable child frame
disconnection. I haven't digged too deeply in to what that means, but it looks
like a lot of stuff is going on there that we need to double check?

[sof, 2014/09/17 09:42:58]

The issue of signalling document destruction is definitely an issue. But let's
deal with this one via FrameLoader::dispose() -- this call is redundant during
frame finalization, as it is repeated in that dispose().

     }
 
     eventHandler().clear();
 
     m_view = view;
 
     if (m_view && isMainFrame()) {
         if (settings()->pinchVirtualViewportEnabled())
             m_host->pinchViewport().mainFrameDidChangeSize();
         else
@@ skipping 12 matching lines @@
 
     document()->styleResolverChanged();
     if (shouldUsePrintingLayout()) {
         view()->forceLayoutForPagination(pageSize, originalPageSize, maximumShrinkRatio);
     } else {
         view()->forceLayout();
         view()->adjustViewSize();
     }
 
     // Subframes of the one we're printing don't lay out to the page size.
-    for (RefPtr<Frame> child = tree().firstChild(); child; child = child->tree().nextSibling()) {
+    for (RefPtrWillBeRawPtr<Frame> child = tree().firstChild(); child; child = child->tree().nextSibling()) {
         if (child->isLocalFrame())
             toLocalFrame(child.get())->setPrinting(printing, FloatSize(), FloatSize(), 0);
     }
 }
 
 bool LocalFrame::shouldUsePrintingLayout() const
 {
     // Only top frame being printed should be fit to page size.
     // Subframes should be constrained by parents only.
     return document()->printing() && (!tree().parent() || !tree().parent()->isLocalFrame() || !toLocalFrame(tree().parent())->document()->printing());
@@ skipping 15 matching lines @@
         float ratio = originalSize.width() / originalSize.height();
         resultSize.setHeight(floorf(expectedSize.height()));
         resultSize.setWidth(floorf(resultSize.height() * ratio));
     }
     return resultSize;
 }
 
 void LocalFrame::setDOMWindow(PassRefPtrWillBeRawPtr<LocalDOMWindow> domWindow)
 {
     if (m_domWindow) {
+        // Oilpan: the assumption is that FrameDestructionObserver::willDetachFrameHost()
+        // on LocalWindow will have signalled these frameWindowDiscarded() notifications.
+        //
+        // It is not invoked when finalizing the LocalFrame, as setDOMWindow() isn't
+        // performed (accessing the m_domWindow heap object is unsafe then.)
         console().messageStorage()->frameWindowDiscarded(m_domWindow.get());
         InspectorInstrumentation::frameWindowDiscarded(this, m_domWindow.get());
     }
     if (domWindow)
         script().clearWindowProxy();
     Frame::setDOMWindow(domWindow);
 }
 
 void LocalFrame::didChangeVisibilityState()
 {
     if (document())
         document()->didChangeVisibilityState();
 
-    Vector<RefPtr<LocalFrame> > childFrames;
+    WillBeHeapVector<RefPtrWillBeMember<LocalFrame> > childFrames;
     for (Frame* child = tree().firstChild(); child; child = child->tree().nextSibling()) {
         if (child->isLocalFrame())
             childFrames.append(toLocalFrame(child));
     }
 
     for (size_t i = 0; i < childFrames.size(); ++i)
         childFrames[i]->didChangeVisibilityState();
 }
 
 void LocalFrame::addDestructionObserver(FrameDestructionObserver* observer)
 {
     m_destructionObservers.add(observer);
 }
 
 void LocalFrame::removeDestructionObserver(FrameDestructionObserver* observer)
 {
     m_destructionObservers.remove(observer);
 }
 
 void LocalFrame::willDetachFrameHost()
 {
     // We should never be detatching the page during a Layout.
     RELEASE_ASSERT(!m_view || !m_view->isInPerformLayout());
 
     Frame* parent = tree().parent();
     if (parent && parent->isLocalFrame())
         toLocalFrame(parent)->loader().checkLoadComplete();
 
-    HashSet<FrameDestructionObserver*>::iterator stop = m_destructionObservers.end();
-    for (HashSet<FrameDestructionObserver*>::iterator it = m_destructionObservers.begin(); it != stop; ++it)
+    WillBeHeapHashSet<RawPtrWillBeWeakMember<FrameDestructionObserver> >::iterator stop = m_destructionObservers.end();
+    for (WillBeHeapHashSet<RawPtrWillBeWeakMember<FrameDestructionObserver> >::iterator it = m_destructionObservers.begin(); it != stop; ++it)
         (*it)->willDetachFrameHost();
 
     // FIXME: Page should take care of updating focus/scrolling instead of Frame.
     // FIXME: It's unclear as to why this is called more than once, but it is,
     // so page() could be null.
     if (page() && page()->focusController().focusedFrame() == this)
         page()->focusController().setFocusedFrame(nullptr);
     script().clearScriptObjects();
 
     if (page() && page()->scrollingCoordinator() && m_view)
         page()->scrollingCoordinator()->willDestroyScrollableArea(m_view.get());
 }
 
 void LocalFrame::detachFromFrameHost()
 {
-    // We should never be detatching the page during a Layout.
+    // We should never be detaching the page during a Layout.
     RELEASE_ASSERT(!m_view || !m_view->isInPerformLayout());
-    m_host = 0;
+    m_host = nullptr;
 }
 
 String LocalFrame::documentTypeString() const
 {
     if (DocumentType* doctype = document()->doctype())
         return createMarkup(doctype);
 
     return String();
 }
 
@@ skipping 196 matching lines @@
             // Update the scroll position when doing a full page zoom, so the content stays in relatively the same position.
             LayoutPoint scrollPosition = view->scrollPosition();
             float percentDifference = (pageZoomFactor / m_pageZoomFactor);
             view->setScrollPosition(IntPoint(scrollPosition.x() * percentDifference, scrollPosition.y() * percentDifference));
         }
     }
 
     m_pageZoomFactor = pageZoomFactor;
     m_textZoomFactor = textZoomFactor;
 
-    for (RefPtr<Frame> child = tree().firstChild(); child; child = child->tree().nextSibling()) {
+    for (RefPtrWillBeRawPtr<Frame> child = tree().firstChild(); child; child = child->tree().nextSibling()) {
         if (child->isLocalFrame())
             toLocalFrame(child.get())->setPageAndTextZoomFactors(m_pageZoomFactor, m_textZoomFactor);
     }
 
     document->setNeedsStyleRecalc(SubtreeStyleChange);
     document->updateLayoutIgnorePendingStylesheets();
 }
 
 void LocalFrame::deviceOrPageScaleFactorChanged()
 {
     document()->mediaQueryAffectingValueChanged();
-    for (RefPtr<Frame> child = tree().firstChild(); child; child = child->tree().nextSibling()) {
+    for (RefPtrWillBeRawPtr<Frame> child = tree().firstChild(); child; child = child->tree().nextSibling()) {
         if (child->isLocalFrame())
             toLocalFrame(child.get())->deviceOrPageScaleFactorChanged();
     }
 }
 
 bool LocalFrame::isURLAllowed(const KURL& url) const
 {
     // We allow one level of self-reference because some sites depend on that,
     // but we don't allow more than one.
     if (page()->subframeCount() >= Page::maxNumberOfFrames)
@@ skipping 140 matching lines @@
 
     return curFrame;
 }
 
 void LocalFrame::setPagePopupOwner(Element& owner)
 {
     m_pagePopupOwner = &owner;
 }
 
 } // namespace blink