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

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

Issue 517043003: Move Frame to the Oilpan heap. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: Add LocalFrame::detachView() + more Oilpan frame finalization comments 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
Index: Source/core/frame/LocalDOMWindow.cpp
diff --git a/Source/core/frame/LocalDOMWindow.cpp b/Source/core/frame/LocalDOMWindow.cpp
index d6604f3b4211042f4662bf3159ad5b11d304fa56..0e3cbde8ba75781f02120283e85058afb02b1e91 100644
--- a/Source/core/frame/LocalDOMWindow.cpp
+++ b/Source/core/frame/LocalDOMWindow.cpp
@@ -484,7 +484,7 @@ void LocalDOMWindow::enqueuePopstateEvent(PassRefPtr<SerializedScriptValue> stat
void LocalDOMWindow::statePopped(PassRefPtr<SerializedScriptValue> stateObject)
{
- if (!frame())
+ if (!m_frame)
return;
// Per step 11 of section 6.5.9 (history traversal) of the HTML5 spec, we
@@ -497,8 +497,6 @@ void LocalDOMWindow::statePopped(PassRefPtr<SerializedScriptValue> stateObject)
LocalDOMWindow::~LocalDOMWindow()
{
- ASSERT(m_hasBeenReset);
-
#if ENABLE(OILPAN)
// Oilpan: the frame host and document objects are
// also garbage collected; cannot notify these
@@ -508,6 +506,7 @@ LocalDOMWindow::~LocalDOMWindow()
// Cleared when detaching document.
ASSERT(!m_eventQueue);
#else
+ ASSERT(m_hasBeenReset);
Mads Ager (chromium) 2014/09/16 12:17:45 This looks a little odd (it is not new though). We
sof 2014/09/17 09:42:58 See https://codereview.chromium.org/517043003/#msg
reset();
removeAllEventListenersInternal(DoBroadcastListenerRemoval);
@@ -542,11 +541,15 @@ Page* LocalDOMWindow::page()
return frame() ? frame()->page() : 0;
}
+#if !ENABLE(OILPAN)
void LocalDOMWindow::frameDestroyed()
{
FrameDestructionObserver::frameDestroyed();
- reset();
+ // The frame resets its LocalDOMWindow prior to frame destruction
Mads Ager (chromium) 2014/09/16 12:17:45 So, really, we could remove the frameDestroyed ove
sof 2014/09/17 09:42:58 Hmm, it doesn't add much value -- if frame destruc
+ // notifications.
+ ASSERT(m_hasBeenReset);
}
+#endif
void LocalDOMWindow::willDetachFrameHost()
{
@@ -903,7 +906,7 @@ void LocalDOMWindow::dispatchMessageEventWithOriginCheck(SecurityOrigin* intende
DOMSelection* LocalDOMWindow::getSelection()
{
- if (!isCurrentlyDisplayedInFrame() || !m_frame)
+ if (!isCurrentlyDisplayedInFrame())
return 0;
return m_frame->document()->getSelection();
@@ -1061,8 +1064,8 @@ bool LocalDOMWindow::find(const String& string, bool caseSensitive, bool backwar
return false;
// |m_frame| can be destructed during |Editor::findString()| via
- // |Document::updateLayou()|, e.g. event handler removes a frame.
- RefPtr<LocalFrame> protectFrame(m_frame);
+ // |Document::updateLayout()|, e.g. event handler removes a frame.
+ RefPtrWillBeRawPtr<LocalFrame> protectFrame(m_frame.get());
// FIXME (13016): Support wholeWord, searchInFrames and showDialog
return m_frame->editor().findString(string, !backwards, caseSensitive, wrap, false);
@@ -1195,7 +1198,13 @@ int LocalDOMWindow::scrollY() const
bool LocalDOMWindow::closed() const
{
- return !m_frame;
+ // FIXME: Oilpan: the timing of when m_frame is GCed is clearly
+ // different to what happens non-Oilpan. m_frame->hasBeenClosed()
+ // is considered in both settings to make the behavior the same --
+ // the flag is set when the WebView (and frame) is closed. Finalization
+ // of the m_frame will happen at some point later.
+ //
+ return !m_frame || m_frame->hasBeenClosed();
}
unsigned LocalDOMWindow::length() const
@@ -1864,11 +1873,10 @@ void LocalDOMWindow::showModalDialog(const String& urlString, const String& dial
LocalDOMWindow* LocalDOMWindow::anonymousIndexedGetter(uint32_t index)
{
- LocalFrame* frame = this->frame();
- if (!frame)
+ if (!m_frame)
return 0;
- Frame* child = frame->tree().scopedChild(index);
+ Frame* child = m_frame->tree().scopedChild(index);
if (child)
return child->domWindow();
@@ -1912,6 +1920,7 @@ void LocalDOMWindow::trace(Visitor* visitor)
WillBeHeapSupplementable<LocalDOMWindow>::trace(visitor);
EventTargetWithInlineData::trace(visitor);
LifecycleContext<LocalDOMWindow>::trace(visitor);
+ FrameDestructionObserver::trace(visitor);
}
v8::Handle<v8::Object> LocalDOMWindow::wrap(v8::Handle<v8::Object> creationContext, v8::Isolate* isolate)

Powered by Google App Engine
This is Rietveld 408576698