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

Unified Diff: Source/core/frame/LocalFrame.cpp

Issue 517043003: Move Frame to the Oilpan heap. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: Experiment: also clear FrameView and FrameLoader during owner disconnect for non-Oilpan Created 6 years, 3 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
« no previous file with comments | « Source/core/frame/LocalFrame.h ('k') | Source/core/frame/RemoteFrame.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Source/core/frame/LocalFrame.cpp
diff --git a/Source/core/frame/LocalFrame.cpp b/Source/core/frame/LocalFrame.cpp
index 94a5d12e52729f9b52217b2c695faf5249daa7ce..2903a179fdbcba7e32f60f96acb1119edeebe31f 100644
--- a/Source/core/frame/LocalFrame.cpp
+++ b/Source/core/frame/LocalFrame.cpp
@@ -106,30 +106,58 @@ inline LocalFrame::LocalFrame(FrameLoaderClient* client, FrameHost* host, FrameO
{
}
-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()
{
- setView(nullptr);
- loader().clear();
+ // Verify that the FrameView and FrameLoader have instead been
+ // cleared as part of detaching the frame.
+ ASSERT(!m_owner);
+ ASSERT(!m_view);
+#if !ENABLE(OILPAN)
+ // 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, explicit
+ // frameDestroyed() notifications aren't needed.
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();
@@ -153,13 +181,20 @@ bool LocalFrame::inScope(TreeScope* scope) const
return owner->treeScope() == scope;
}
-void LocalFrame::setView(PassRefPtr<FrameView> view)
+void LocalFrame::detachView()
{
- // 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.
+ // 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)
+{
+ 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
@@ -199,7 +234,7 @@ void LocalFrame::setPrinting(bool printing, const FloatSize& pageSize, const Flo
}
// 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);
}
@@ -235,6 +270,11 @@ FloatSize LocalFrame::resizePageRectsKeepingRatio(const FloatSize& originalSize,
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());
}
@@ -248,7 +288,7 @@ 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));
@@ -277,8 +317,8 @@ void LocalFrame::willDetachFrameHost()
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.
@@ -294,9 +334,9 @@ void LocalFrame::willDetachFrameHost()
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
@@ -513,7 +553,7 @@ void LocalFrame::setPageAndTextZoomFactors(float pageZoomFactor, float textZoomF
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);
}
@@ -525,7 +565,7 @@ void LocalFrame::setPageAndTextZoomFactors(float pageZoomFactor, float textZoomF
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();
}
@@ -665,8 +705,14 @@ double LocalFrame::devicePixelRatio() const
void LocalFrame::disconnectOwnerElement()
{
if (owner()) {
- if (Document* doc = document())
- doc->topDocument().clearAXObjectCache();
+ if (Document* document = this->document())
+ document->topDocument().clearAXObjectCache();
+ // Clear the FrameView and FrameLoader right here rather than
+ // (as previously) during finalization. It simplifies and is
+ // Oilpan friendly, as we cannot access various heap objects
+ // touched here then.
+ setView(nullptr);
+ loader().clear();
}
Frame::disconnectOwnerElement();
}
« no previous file with comments | « Source/core/frame/LocalFrame.h ('k') | Source/core/frame/RemoteFrame.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698