Simplify FullscreenElementStack::fullScreenChangeDelayTimerFired()

Source/core/dom/FullscreenElementStack.cpp

 /*
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2001 Dirk Mueller (mueller@kde.org)
  *           (C) 2006 Alexey Proskuryakov (ap@webkit.org)
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved. (http://www.torchmobile.com/)
  * Copyright (C) 2010 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
@@ skipping 478 matching lines @@
     // document will be detached and GC'd. We protect it here to make sure we
     // can finish the function successfully.
     RefPtrWillBeRawPtr<Document> protectDocument(document());
     WillBeHeapDeque<RefPtrWillBeMember<Node> > changeQueue;
     m_fullScreenChangeEventTargetQueue.swap(changeQueue);
     WillBeHeapDeque<RefPtrWillBeMember<Node> > errorQueue;
     m_fullScreenErrorEventTargetQueue.swap(errorQueue);
 
     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));
     }
 
     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));
     }
 }
 
 void FullscreenElementStack::fullScreenElementRemoved()
 {
     m_fullScreenElement->setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(false);
     webkitCancelFullScreen();
@@ skipping 52 matching lines @@
 void FullscreenElementStack::trace(Visitor* visitor)
 {
     visitor->trace(m_fullScreenElement);
     visitor->trace(m_fullScreenElementStack);
     visitor->trace(m_fullScreenChangeEventTargetQueue);
     visitor->trace(m_fullScreenErrorEventTargetQueue);
     DocumentSupplement::trace(visitor);
 }
 
 } // namespace WebCore