Move Frame to the Oilpan heap.

Source/core/frame/LocalDOMWindow.cpp

 /*
  * Copyright (C) 2006, 2007, 2008, 2010 Apple Inc. All rights reserved.
  * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies)
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 466 matching lines @@
 {
     if (!ContextFeatures::pushStateEnabled(document()))
         return;
 
     // FIXME: https://bugs.webkit.org/show_bug.cgi?id=36202 Popstate event needs to fire asynchronously
     dispatchEvent(PopStateEvent::create(stateObject, &history()));
 }
 
 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
     // defer firing of popstate until we're in the complete state.
     if (document()->isLoadCompleted())
         enqueuePopstateEvent(stateObject);
     else
         m_pendingStateObject = stateObject;
 }
 
 LocalDOMWindow::~LocalDOMWindow()
 {
-    ASSERT(m_hasBeenReset);
-
 #if ENABLE(OILPAN)
     // Oilpan: the frame host and document objects are
     // also garbage collected; cannot notify these
     // when removing event listeners.
     removeAllEventListenersInternal(DoNotBroadcastListenerRemoval);
 
     // 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 assert that the
LocalDOMWindow has been reset and then we reset it again. Isn't the reset call
redundant here?

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

See https://codereview.chromium.org/517043003/#msg36

     reset();
 
     removeAllEventListenersInternal(DoBroadcastListenerRemoval);
 
     ASSERT(m_document->isStopped());
     clearDocument();
 #endif
 }
 
 const AtomicString& LocalDOMWindow::interfaceName() const
@@ skipping 14 matching lines @@
 PassRefPtrWillBeRawPtr<MediaQueryList> LocalDOMWindow::matchMedia(const String& media)
 {
     return document() ? document()->mediaQueryMatcher().matchMedia(media) : nullptr;
 }
 
 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 override here in the non-oilpan
build as well. But maybe the assert is worth it here? The assert is in the
destructor which is maybe enough?

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

Hmm, it doesn't add much value -- if frame destruction should change the order
in which it issues frameDestroyed() vs clearing its LocalDOMWindow, that isn't a
problem. If it fails to do the latter, then that will cause badness regardless.

Let's remove the override; nice catch (I failed to see the potential
simplification while removing the reset() call.) I kept frameDestroyed() as a
virtual.

+    // notifications.
+    ASSERT(m_hasBeenReset);
 }
+#endif
 
 void LocalDOMWindow::willDetachFrameHost()
 {
     m_frame->host()->eventHandlerRegistry().didRemoveAllEventHandlers(*this);
     m_frame->console().messageStorage()->frameWindowDiscarded(this);
     InspectorInstrumentation::frameWindowDiscarded(m_frame, this);
 }
 
 void LocalDOMWindow::willDestroyDocumentInFrame()
 {
@@ skipping 336 matching lines @@
             frameConsole()->addMessage(consoleMessage.release());
             return;
         }
     }
 
     dispatchEvent(event);
 }
 
 DOMSelection* LocalDOMWindow::getSelection()
 {
-    if (!isCurrentlyDisplayedInFrame() || !m_frame)
+    if (!isCurrentlyDisplayedInFrame())
         return 0;
 
     return m_frame->document()->getSelection();
 }
 
 Element* LocalDOMWindow::frameElement() const
 {
     if (!m_frame)
         return 0;
 
@@ skipping 137 matching lines @@
 
     return String();
 }
 
 bool LocalDOMWindow::find(const String& string, bool caseSensitive, bool backwards, bool wrap, bool /*wholeWord*/, bool /*searchInFrames*/, bool /*showDialog*/) const
 {
     if (!isCurrentlyDisplayedInFrame())
         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);
 }
 
 bool LocalDOMWindow::offscreenBuffering() const
 {
     return true;
 }
 
@@ skipping 112 matching lines @@
     if (!view)
         return 0;
 
     m_frame->document()->updateLayoutIgnorePendingStylesheets();
 
     return adjustForAbsoluteZoom(view->scrollY(), m_frame->pageZoomFactor());
 }
 
 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
 {
     if (!isCurrentlyDisplayedInFrame())
         return 0;
 
     return m_frame->tree().scopedChildCount();
 }
 
@@ skipping 648 matching lines @@
     LocalFrame* dialogFrame = createWindow(urlString, emptyAtom, windowFeatures,
         *callingWindow, *firstFrame, *m_frame, function, functionContext);
     if (!dialogFrame)
         return;
     UserGestureIndicatorDisabler disabler;
     dialogFrame->host()->chrome().runModal();
 }
 
 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();
 
     return 0;
 }
 
 DOMWindowLifecycleNotifier& LocalDOMWindow::lifecycleNotifier()
 {
     return static_cast<DOMWindowLifecycleNotifier&>(LifecycleContext<LocalDOMWindow>::lifecycleNotifier());
 }
@@ skipping 23 matching lines @@
     visitor->trace(m_sessionStorage);
     visitor->trace(m_localStorage);
     visitor->trace(m_applicationCache);
     visitor->trace(m_performance);
     visitor->trace(m_css);
     visitor->trace(m_eventQueue);
 #endif
     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)
 {
     ASSERT_NOT_REACHED(); // LocalDOMWindow has [Custom=ToV8].
     return v8::Handle<v8::Object>();
 }
 
 } // namespace blink