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

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

Issue 603193005: Move the Widget hierarchy to the Oilpan heap. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: Rebased upto r183571 Created 6 years, 2 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
Index: Source/core/frame/LocalFrame.cpp
diff --git a/Source/core/frame/LocalFrame.cpp b/Source/core/frame/LocalFrame.cpp
index 365bb0381a686066e5ba2fe36b0bb26730278847..53d994eb092f0dcdf59400663637b997a5edd976 100644
--- a/Source/core/frame/LocalFrame.cpp
+++ b/Source/core/frame/LocalFrame.cpp
@@ -48,6 +48,7 @@
#include "core/frame/LocalDOMWindow.h"
#include "core/frame/Settings.h"
#include "core/html/HTMLFrameElementBase.h"
+#include "core/html/HTMLPlugInElement.h"
#include "core/inspector/ConsoleMessageStorage.h"
#include "core/inspector/InspectorInstrumentation.h"
#include "core/loader/FrameLoaderClient.h"
@@ -145,6 +146,7 @@ void LocalFrame::trace(Visitor* visitor)
visitor->trace(m_destructionObservers);
visitor->trace(m_loader);
visitor->trace(m_navigationScheduler);
+ visitor->trace(m_view);
visitor->trace(m_pagePopupOwner);
visitor->trace(m_script);
visitor->trace(m_editor);
@@ -153,11 +155,23 @@ void LocalFrame::trace(Visitor* visitor)
visitor->trace(m_eventHandler);
visitor->trace(m_console);
visitor->trace(m_inputMethodController);
+ visitor->trace(m_pluginElements);
haraken 2014/10/11 17:33:02 Is it safe to trace m_pluginElements? There is no
sof 2014/10/12 08:16:22 It is safe with the weak callback stack that the i
haraken 2014/10/12 12:56:04 I'm suggesting the first one :) If I'm understand
sof 2014/10/12 13:09:12 It does not always return true; if no one has mark
haraken 2014/10/12 13:38:23 Do you know how it's guaranteed that weak processi
sof 2014/10/12 16:22:13 There are no such guarantees, but the weak callbac
sof 2014/10/12 18:35:36 I've switched m_pluginElements to HashSet<HTMLPlug
+ visitor->registerWeakMembers<LocalFrame, &LocalFrame::clearWeakMembers>(this);
HeapSupplementable<LocalFrame>::trace(visitor);
#endif
Frame::trace(visitor);
}
+#if ENABLE(OILPAN)
+void LocalFrame::clearWeakMembers(Visitor* visitor)
+{
+ for (HeapHashSet<WeakMember<HTMLPlugInElement> >::const_iterator it = m_pluginElements.begin(); it != m_pluginElements.end(); ++it) {
+ if (!visitor->isAlive(*it))
+ (*it)->shouldDisposePlugin();
haraken 2014/10/11 17:33:02 shouldDisposePlugin() only does simple things and
sof 2014/10/12 08:16:22 How would you know if the LocalFrame is still aliv
haraken 2014/10/12 12:56:04 I'm a bit confused. Pre-finalizers are invoked bef
sof 2014/10/12 13:09:12 It's still alive, yes. But the plugin is allowed t
haraken 2014/10/12 13:38:23 I agree that dispose() does a bunch of things, but
sof 2014/10/12 16:22:13 Yes, it does not, but it should only be called/sig
+ }
+}
+#endif
+
void LocalFrame::navigate(Document& originDocument, const KURL& url, bool lockBackForwardList)
{
m_navigationScheduler.scheduleLocationChange(&originDocument, url.string(), lockBackForwardList);
@@ -202,8 +216,9 @@ void LocalFrame::detachView()
m_view->prepareForDetach();
}
-void LocalFrame::setView(PassRefPtr<FrameView> view)
+void LocalFrame::setView(PassRefPtrWillBeRawPtr<FrameView> view)
{
+ ASSERT(!m_view || m_view != view);
detachView();
// Prepare for destruction now, so any unload event handlers get run and the LocalDOMWindow is
@@ -448,7 +463,7 @@ void LocalFrame::createView(const IntSize& viewportSize, const Color& background
setView(nullptr);
- RefPtr<FrameView> frameView;
+ RefPtrWillBeRawPtr<FrameView> frameView = nullptr;
if (isLocalRoot) {
frameView = FrameView::create(this, viewportSize);
@@ -723,6 +738,12 @@ void LocalFrame::disconnectOwnerElement()
if (Document* document = this->document())
document->topDocument().clearAXObjectCache();
#if ENABLE(OILPAN)
+ // First give the plugin elements holding persisted,
+ // renderer-less plugins the opportunity to dispose of them.
+ for (HeapHashSet<WeakMember<HTMLPlugInElement> >::const_iterator it = m_pluginElements.begin(); it != m_pluginElements.end(); ++it)
+ (*it)->disconnectContentFrame();
+ m_pluginElements.clear();
+
// Clear the FrameView and FrameLoader right here rather than
// during finalization. Too late to access various heap objects
// at that stage.
@@ -747,4 +768,18 @@ void LocalFrame::setPagePopupOwner(Element& owner)
m_pagePopupOwner = &owner;
}
+#if ENABLE(OILPAN)
+void LocalFrame::registerPluginElement(HTMLPlugInElement* plugin)
+{
+ m_pluginElements.add(plugin);
+}
+
+void LocalFrame::unregisterPluginElement(HTMLPlugInElement* plugin)
+{
+ ASSERT(!ThreadState::current()->isSweepInProgress());
haraken 2014/10/11 17:33:02 I wanted to add this check since it would cause an
sof 2014/10/12 08:16:22 Gone :)
+ ASSERT(m_pluginElements.contains(plugin));
+ m_pluginElements.remove(plugin);
+}
+#endif
+
} // namespace blink

Powered by Google App Engine
This is Rietveld 408576698