Simplify FullscreenElementStack::fullScreenChangeDelayTimerFired()

Source/core/dom/FullscreenElementStack.cpp

Index: Source/core/dom/FullscreenElementStack.cpp
diff --git a/Source/core/dom/FullscreenElementStack.cpp b/Source/core/dom/FullscreenElementStack.cpp
index d153ed3d1dd4a2d29b7c9fa328098d5d350f7cf1..934b0eb7e4fe1de75081d73e22c1443ddb4e6114 100644
--- a/Source/core/dom/FullscreenElementStack.cpp
+++ b/Source/core/dom/FullscreenElementStack.cpp
@@ -496,15 +496,9 @@ void FullscreenElementStack::fullScreenChangeDelayTimerFired(Timer<FullscreenEle
 
     while (!changeQueue.isEmpty()) {
         RefPtrWillBeRawPtr<Node> node = changeQueue.takeFirst();
-        if (!node)
-            node = document()->documentElement();
-        // The dispatchEvent below may have blown away our documentElement.

[falken, 2014/07/03 01:51:42]

Hmm, why is this no longer possible? Can we add documentElement to the queue,
then call dispatchEvent and it gets removed/deleted/invalidated? Or is the idea
that adding it to the changeQueue protects it by a RefPtr so it won't be blown
away?

[philipj_slow, 2014/07/03 13:53:00]

Since we never push null to the queue and the queue entries are refcounted, we
won't ever pop null from the queue, so neither of these if (!node) branches will
ever be taken.

However, it is possible that the documentElement is removed from the document in
the first dispatchEvent, and if a new documentElement was inserted, that one
would be added to the queue as well. This could continue in a loop forever, but
AFAICT that was already the case.

[falken, 2014/07/03 14:48:38]

OK I see. lgtm

-        if (!node)
-            continue;
 
-        // If the element was removed from our tree, also message the documentElement. Since we may
-        // have a document hierarchy, check that node isn't in another document.
-        if (!document()->contains(node.get()) && !node->inDocument())

[falken, 2014/07/03 01:51:42]

nice catch!

+        // If the element was removed from our tree, also message the documentElement.
+        if (!node->inDocument() && document()->documentElement())
             changeQueue.append(document()->documentElement());
 
         node->dispatchEvent(Event::createBubble(EventTypeNames::webkitfullscreenchange));
@@ -512,15 +506,9 @@ void FullscreenElementStack::fullScreenChangeDelayTimerFired(Timer<FullscreenEle
 
     while (!errorQueue.isEmpty()) {
         RefPtrWillBeRawPtr<Node> node = errorQueue.takeFirst();
-        if (!node)
-            node = document()->documentElement();
-        // The dispatchEvent below may have blown away our documentElement.
-        if (!node)
-            continue;
 
-        // If the element was removed from our tree, also message the documentElement. Since we may
-        // have a document hierarchy, check that node isn't in another document.
-        if (!document()->contains(node.get()) && !node->inDocument())
+        // If the element was removed from our tree, also message the documentElement.
+        if (!node->inDocument() && document()->documentElement())
             errorQueue.append(document()->documentElement());
 
         node->dispatchEvent(Event::createBubble(EventTypeNames::webkitfullscreenerror));