Oilpan: add [WillBeGarbageCollected] for Node.

Source/core/frame/DOMWindow.cpp

Index: Source/core/frame/DOMWindow.cpp
diff --git a/Source/core/frame/DOMWindow.cpp b/Source/core/frame/DOMWindow.cpp
index 3b2c2b9012b322e5ed2606ea54dbd332fe7d9d8f..b7c3fbdaf6ae62669536c878a4fcfd55c1dd43b0 100644
--- a/Source/core/frame/DOMWindow.cpp
+++ b/Source/core/frame/DOMWindow.cpp
@@ -348,7 +348,7 @@ void DOMWindow::clearDocument()
         m_document->detach();

[haraken, 2014/05/23 20:53:58]

I'm not yet fully convinced about this one. Would you mind again elaborating on
why we don't need to call detach() when the DOMWindow dies but the Document is
still alive?

If we really wants to keep the current behavior as is, I think we need to move
the detach() call into the weak processing for Document::m_domWindow.

[sof, 2014/05/23 21:00:15]

I don't have a better answer than the one given earlier today. Will have to
sleep on it.

[sof, 2014/05/24 05:51:00]

Consider the following steps:

- A DOMWindow can at the earliest be destructed once a (Local)Frame lets go of
the window.
(- A subsequent v8 GC could then finalize and destruct the DOMWindow, assuming
nothing else keeping the window alive.)
- A LocalFrame will only let go of the window in its destructor.
- The LocalFrame destructor will prior to that clear out the 'view', invoking
setView(nullptr).
- Clearing that view will invoke Document::prepareForDestruction().
- prepareForDestruction() detach()es the document.
=> By the time the DOMWindow is a candidate for destruction, the document will
already have been detached.
- When the DOMWindow's dtor does get to run, the window's detached document
(should it be accessible somehow with Oilpan enabled) will be in a Stopped
state. Hence, the conditional detach() in DOMWindow::clearDocument() can never
apply.
- As a result of that being a no-op, the entire call to clearDocument() is not
needed for Oilpan.
- Indeed, as Document::detach() already calls clearEventQueue() on its Window,
the extra call we added to clearEventQueue() in ~DOMWindow for the Oilpan case
is also redundant. Will remove.

And if that detailed set of steps doesn't convince, the detached document
condition is already asserted for in ~DOMWindow (cf. isStopped() /
isDisposed().)
As detailed above, not needed.

     }
 
-    // FIXME: This should be part of ActiveDOM Object shutdown
+    // FIXME: This should be part of ActiveDOMObject shutdown
     clearEventQueue();
 
     m_document->clearDOMWindow();
@@ -489,15 +489,20 @@ DOMWindow::~DOMWindow()
     ASSERT(m_hasBeenReset);
     reset();
 
-    removeAllEventListeners();
-
 #if ENABLE(OILPAN)
-    ASSERT(m_document->isDisposed());

[haraken, 2014/05/23 20:53:58]

Just help me understand: Why do we need to remove this ASSERT?

[sof, 2014/05/24 05:51:00]

It was safe with m_document being a RefPtr<>, no longer so with it being a
Member.

Add an assert to the DOMWindow weak processing on Document instead?

[sof, 2014/05/24 06:53:11]

Done.

+    // Oilpan: the frame host and document objects are
+    // also garbage collected; cannot notify these
+    // when removing event listeners.
+    removeAllEventListeners(DoNotBroadcastListenerRemoval);
+
+    // FIXME: This should be part of ActiveDOMObject shutdown
+    clearEventQueue();
 #else
-    ASSERT(m_document->isStopped());
-#endif
+    removeAllEventListeners(DoBroadcastListenerRemoval);
 
+    ASSERT(m_document->isStopped());
     clearDocument();
+#endif
 }
 
 const AtomicString& DOMWindow::interfaceName() const
@@ -1613,22 +1618,29 @@ bool DOMWindow::dispatchEvent(PassRefPtrWillBeRawPtr<Event> prpEvent, PassRefPtr
     return result;
 }
 
-void DOMWindow::removeAllEventListeners()
+void DOMWindow::removeAllEventListeners(BroadcastListenerRemoval mode)
 {
     EventTarget::removeAllEventListeners();
 
     lifecycleNotifier().notifyRemoveAllEventListeners(this);
 
-    if (m_frame && m_frame->host())
-        m_frame->host()->eventHandlerRegistry().didRemoveAllEventHandlers(*this);
+    if (mode == DoBroadcastListenerRemoval) {
+        if (m_frame && m_frame->host())
+            m_frame->host()->eventHandlerRegistry().didRemoveAllEventHandlers(*this);
 
-    if (Document* document = this->document())
-        document->didClearTouchEventHandlers(document);
+        if (Document* document = this->document())
+            document->didClearTouchEventHandlers(document);
+    }
 
     removeAllUnloadEventListeners(this);
     removeAllBeforeUnloadEventListeners(this);
 }
 
+void DOMWindow::removeAllEventListeners()
+{
+    removeAllEventListeners(DoBroadcastListenerRemoval);
+}
+
 void DOMWindow::finishedLoading()
 {
     if (m_shouldPrintWhenFinishedLoading) {
@@ -1875,6 +1887,7 @@ PassOwnPtr<LifecycleNotifier<DOMWindow> > DOMWindow::createLifecycleNotifier()
 
 void DOMWindow::trace(Visitor* visitor)
 {
+    visitor->trace(m_document);
     visitor->trace(m_screen);
     visitor->trace(m_history);
     visitor->trace(m_locationbar);