Remove all event listeners during window's frame destruction step.

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 185 matching lines @@
     RefPtr<SerializedScriptValue> m_message;
     String m_origin;
     RefPtrWillBeMember<LocalDOMWindow> m_source;
     OwnPtr<MessagePortChannelArray> m_channels;
     RefPtr<SecurityOrigin> m_targetOrigin;
     RefPtrWillBeMember<ScriptCallStack> m_stackTrace;
     RefPtr<UserGestureToken> m_userGestureToken;
     int m_asyncOperationId;
 };
 
-static void disableSuddenTermination()
+static void updateSuddenTerminationStatus(LocalDOMWindow* domWindow, bool addedListener, FrameLoaderClient::SuddenTerminationDisablerType disablerType)
 {
-    blink::Platform::current()->suddenTerminationChanged(false);
-}
-
-static void enableSuddenTermination()
-{
-    blink::Platform::current()->suddenTerminationChanged(true);
+    blink::Platform::current()->suddenTerminationChanged(!addedListener);
+    if (domWindow->frame() && domWindow->frame()->loader().client())
+        domWindow->frame()->loader().client()->suddenTerminationDisablerChanged(addedListener, disablerType);
 }
 
 typedef HashCountedSet<LocalDOMWindow*> DOMWindowSet;
 
 static DOMWindowSet& windowsWithUnloadEventListeners()
 {
     DEFINE_STATIC_LOCAL(DOMWindowSet, windowsWithUnloadEventListeners, ());
     return windowsWithUnloadEventListeners;
 }
 
 static DOMWindowSet& windowsWithBeforeUnloadEventListeners()
 {
     DEFINE_STATIC_LOCAL(DOMWindowSet, windowsWithBeforeUnloadEventListeners, ());
     return windowsWithBeforeUnloadEventListeners;
 }
 
 static void addUnloadEventListener(LocalDOMWindow* domWindow)
 {
     DOMWindowSet& set = windowsWithUnloadEventListeners();
-    if (set.isEmpty()) {
-        disableSuddenTermination();
-        if (domWindow->frame()) {
-            domWindow->frame()->loader().client()->suddenTerminationDisablerChanged(
-                true, FrameLoaderClient::UnloadHandler);
-        }
-    }
+    if (set.isEmpty())
+        updateSuddenTerminationStatus(domWindow, true, FrameLoaderClient::UnloadHandler);
+
     set.add(domWindow);
 }
 
 static void removeUnloadEventListener(LocalDOMWindow* domWindow)
 {
     DOMWindowSet& set = windowsWithUnloadEventListeners();
     DOMWindowSet::iterator it = set.find(domWindow);
     if (it == set.end())
         return;
     set.remove(it);
-    if (set.isEmpty()) {
-        enableSuddenTermination();
-        if (domWindow->frame()) {
-            domWindow->frame()->loader().client()->suddenTerminationDisablerChanged(
-                false, FrameLoaderClient::UnloadHandler);
-        }
-    }
+    if (set.isEmpty())
+        updateSuddenTerminationStatus(domWindow, false, FrameLoaderClient::UnloadHandler);
 }
 
 static void removeAllUnloadEventListeners(LocalDOMWindow* domWindow)
 {
     DOMWindowSet& set = windowsWithUnloadEventListeners();
     DOMWindowSet::iterator it = set.find(domWindow);
     if (it == set.end())
         return;
     set.removeAll(it);
-    if (set.isEmpty()) {
-        enableSuddenTermination();
-        if (domWindow->frame()) {
-            domWindow->frame()->loader().client()->suddenTerminationDisablerChanged(
-                false, FrameLoaderClient::UnloadHandler);
-        }
-    }
+    if (set.isEmpty())
+        updateSuddenTerminationStatus(domWindow, false, FrameLoaderClient::UnloadHandler);
 }
 
 static void addBeforeUnloadEventListener(LocalDOMWindow* domWindow)
 {
     DOMWindowSet& set = windowsWithBeforeUnloadEventListeners();
-    if (set.isEmpty()) {
-        disableSuddenTermination();
-        if (domWindow->frame()) {
-            domWindow->frame()->loader().client()->suddenTerminationDisablerChanged(
-                true, FrameLoaderClient::BeforeUnloadHandler);
-        }
-    }
+    if (set.isEmpty())
+        updateSuddenTerminationStatus(domWindow, true, FrameLoaderClient::BeforeUnloadHandler);
+
     set.add(domWindow);
 }
 
 static void removeBeforeUnloadEventListener(LocalDOMWindow* domWindow)
 {
     DOMWindowSet& set = windowsWithBeforeUnloadEventListeners();
     DOMWindowSet::iterator it = set.find(domWindow);
     if (it == set.end())
         return;
     set.remove(it);
-    if (set.isEmpty()) {
-        enableSuddenTermination();
-        if (domWindow->frame()) {
-            domWindow->frame()->loader().client()->suddenTerminationDisablerChanged(
-                false, FrameLoaderClient::BeforeUnloadHandler);
-        }
-    }
+    if (set.isEmpty())
+        updateSuddenTerminationStatus(domWindow, false, FrameLoaderClient::BeforeUnloadHandler);
 }
 
 static void removeAllBeforeUnloadEventListeners(LocalDOMWindow* domWindow)
 {
     DOMWindowSet& set = windowsWithBeforeUnloadEventListeners();
     DOMWindowSet::iterator it = set.find(domWindow);
     if (it == set.end())
         return;
     set.removeAll(it);
-    if (set.isEmpty()) {
-        enableSuddenTermination();
-        if (domWindow->frame()) {
-            domWindow->frame()->loader().client()->suddenTerminationDisablerChanged(
-                false, FrameLoaderClient::BeforeUnloadHandler);
-        }
-    }
+    if (set.isEmpty())
+        updateSuddenTerminationStatus(domWindow, false, FrameLoaderClient::BeforeUnloadHandler);
 }
 
 static bool allowsBeforeUnloadListeners(LocalDOMWindow* window)
 {
     ASSERT_ARG(window, window);
     LocalFrame* frame = window->frame();
     if (!frame)
         return false;
     return frame->isMainFrame();
 }
@@ skipping 44 matching lines @@
     return frame() && allowPopUp(*frame());
 }
 
 LocalDOMWindow::LocalDOMWindow(LocalFrame& frame)
     : m_frameObserver(WindowFrameObserver::create(this, frame))
     , m_shouldPrintWhenFinishedLoading(false)
 #if ENABLE(ASSERT)
     , m_hasBeenReset(false)
 #endif
 {
+#if ENABLE(OILPAN)
+    ThreadState::current()->registerPreFinalizer(*this);
+#endif
 }
 
 void LocalDOMWindow::clearDocument()
 {
     if (!m_document)
         return;
 
     if (m_document->isActive()) {
         // FIXME: We don't call willRemove here. Why is that OK?
         // This detach() call is also mostly redundant. Most of the calls to
@@ skipping 132 matching lines @@
     // 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()
 {
 #if ENABLE(OILPAN)

[haraken, 2015/03/18 23:38:45]

Not related to your CL, I'm wondering if we could remove this ENABLE(OILPAN)
branch. Now that we have a pre-finalizer in LocalDOMWindow, I guess it would be
possible to refactor the code so that oilpan and non-oilpan use the same code.

Ignore this comment if I'm not making sense. I don't quite remember why we have
different ASSERTs between oilpan and non-oilpan here. Either way, a separate CL
is fine.

[sof, 2015/03/19 06:27:36]

It would be nice to empty this dtor entirely of anything but asserts, but I
haven't investigated what parts of clearDocument() can be moved up/out.

-    // 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);
-    reset();
-
-    removeAllEventListenersInternal(DoBroadcastListenerRemoval);
-
     ASSERT(m_document->isStopped());
     clearDocument();
 #endif
 }
 
+void LocalDOMWindow::dispose()
+{
+    // Oilpan: should the LocalDOMWindow be GCed along with its LocalFrame, the frame
+    // may not have explicitly detached itself first. In that case, handle the clearing
+    // of event listeners in a prefinalizing action.
+    //
+    // (Non-Oilpan, LocalDOMWindow::reset() will always be invoked which takes care of this step.)

[haraken, 2015/03/18 23:38:45]

Help me understand: The LocalDOMWindow::reset() is called by
LocalFrame::setDOMWindow(), and it seems there is no difference in timing when
the setDOMWindow() is called in oilpan and non-oilpan. If that's the case, why
do you argue that reset() is always called in non-oilpan but not in oilpan?

[sof, 2015/03/19 06:27:36]

Yes, if the frame is detached, reset() will be called at the same time. Should
that not happen & when end up finalizing LocalFrame, ~LocalFrame() will only
call setDOMWindow() with Oilpan disabled. That's where the difference comes
from.

(A comment with a quiet word of caution about not adding too much code to this
dispose() method might be worth it here. We don't want to be doing too much work
in these actions.)

[haraken, 2015/03/19 06:54:45]

Thanks for the clarification; makes sense!

+    if (!frame())
+        return;
+
+    removeAllEventListeners();
+}
+
 const AtomicString& LocalDOMWindow::interfaceName() const
 {
     return EventTargetNames::LocalDOMWindow;
 }
 
 ExecutionContext* LocalDOMWindow::executionContext() const
 {
     return m_document.get();
 }
 
@@ skipping 17 matching lines @@
     frame()->host()->eventHandlerRegistry().didRemoveAllEventHandlers(*this);
     frame()->host()->consoleMessageStorage().frameWindowDiscarded(this);
     LocalDOMWindow::notifyContextDestroyed();
 }
 
 void LocalDOMWindow::frameDestroyed()
 {
     willDestroyDocumentInFrame();
     resetLocation();
     m_properties.clear();
+    removeAllEventListeners();
 }
 
 void LocalDOMWindow::willDestroyDocumentInFrame()
 {
     for (const auto& domWindowProperty : m_properties)
         domWindowProperty->willDestroyGlobalObjectInFrame();
 }
 
 void LocalDOMWindow::willDetachDocumentFromFrame()
 {
@@ skipping 970 matching lines @@
     RefPtrWillBeRawPtr<Event> event = prpEvent;
 
     event->setTarget(prpTarget ? prpTarget : this);
     event->setCurrentTarget(this);
     event->setEventPhase(Event::AT_TARGET);
 
     TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("devtools.timeline"), "EventDispatch", "data", InspectorEventDispatchEvent::data(*event));
     return fireEventListeners(event.get());
 }
 
-void LocalDOMWindow::removeAllEventListenersInternal(BroadcastListenerRemoval mode)
+void LocalDOMWindow::removeAllEventListeners()
 {
     EventTarget::removeAllEventListeners();
 
-    if (mode == DoBroadcastListenerRemoval) {
-        notifyRemoveAllEventListeners(this);
-        if (frame() && frame()->host())
-            frame()->host()->eventHandlerRegistry().didRemoveAllEventHandlers(*this);
-    }
+    notifyRemoveAllEventListeners(this);
+    if (frame() && frame()->host())
+        frame()->host()->eventHandlerRegistry().didRemoveAllEventHandlers(*this);
 
     removeAllUnloadEventListeners(this);
     removeAllBeforeUnloadEventListeners(this);
 }
 
-void LocalDOMWindow::removeAllEventListeners()
-{
-    removeAllEventListenersInternal(DoBroadcastListenerRemoval);
-}
-
 void LocalDOMWindow::finishedLoading()
 {
     if (m_shouldPrintWhenFinishedLoading) {
         m_shouldPrintWhenFinishedLoading = false;
         print();
     }
 }
 
 void LocalDOMWindow::printErrorMessage(const String& message)
 {
@@ skipping 172 matching lines @@
     return m_frameObserver->frame();
 }
 
 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