Expand addDocumentToFullScreenChangeEventQueue at the call sites for clarity

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 210 matching lines @@
             // 1. Let following document be the document after document in docs, or null if there is no
             // such document.
             Document* currentDoc = *current;
             Document* followingDoc = following != docs.end() ? *following : 0;
 
             // 2. If following document is null, push context object on document's fullscreen element
             // stack, and queue a task to fire an event named fullscreenchange with its bubbles attribute
             // set to true on the document.
             if (!followingDoc) {
                 from(*currentDoc).pushFullscreenElementStack(element);
-                addDocumentToFullScreenChangeEventQueue(*currentDoc);
+                enqueueChangeEvent(element);
                 continue;
             }
 
             // 3. Otherwise, if document's fullscreen element stack is either empty or its top element
             // is not following document's browsing context container,
             Element* topElement = fullscreenElementFrom(*currentDoc);
             if (!topElement || topElement != followingDoc->ownerElement()) {
                 // ...push following document's browsing context container on document's fullscreen element
                 // stack, and queue a task to fire an event named fullscreenchange with its bubbles attribute
                 // set to true on document.
-                from(*currentDoc).pushFullscreenElementStack(*followingDoc->ownerElement());
-                addDocumentToFullScreenChangeEventQueue(*currentDoc);
+                Element& ownerElement = *followingDoc->ownerElement();
+                from(*currentDoc).pushFullscreenElementStack(ownerElement);
+                enqueueChangeEvent(ownerElement);
                 continue;
             }
 
             // 4. Otherwise, do nothing for this document. It stays the same.
         } while (++current != docs.end());
 
         // 5. Return, and run the remaining steps asynchronously.
         // 6. Optionally, perform some animation.
         m_areKeysEnabledInFullScreen = requestType != PrefixedMozillaRequest && requestType != PrefixedVideoRequest;
         document()->frameHost()->chrome().client().enterFullScreenForElement(&element);
@@ skipping 47 matching lines @@
         ASSERT(toLocalFrame(descendant)->document());
         if (fullscreenElementFrom(*toLocalFrame(descendant)->document()))
             descendants.prepend(toLocalFrame(descendant)->document());
     }
 
     // 4. For each descendant in descendants, empty descendant's fullscreen element stack, and queue a
     // task to fire an event named fullscreenchange with its bubbles attribute set to true on descendant.
     for (WillBeHeapDeque<RefPtrWillBeMember<Document> >::iterator i = descendants.begin(); i != descendants.end(); ++i) {
         ASSERT(*i);
         from(**i).clearFullscreenElementStack();
-        addDocumentToFullScreenChangeEventQueue(**i);
+        enqueueChangeEvent(**i);

[philipj_slow, 2014/06/29 01:21:24]

Bah, adding some asserts revealed that this change was not equivalent, although
apparently there's no test coverage for the case where an element becomes the
target instead of the document. (That's possible because even though the stack
is empty, m_fullScreenElement may be non-null.)

That makes this CL useless. The unprefixed events should always target document
and not bubble, so the code paths will have to be split in any case, but I was
hoping to actually understand the existing logic first. Neither case in this
function makes much sense to me, one element could get the fullscreenchange
event but if there were others on the stack they won't get an event, so scripts
can't count on the event anyway?

[falken, 2014/06/30 02:29:41]

I don't really understand this FullscreenElementStack stuff either. It's
entirely possible the existing code has some oversights like this.

     }
 
     // 5. While doc is not null, run these substeps:
     Element* newTop = 0;
     while (currentDoc) {
         // 1. Pop the top element of doc's fullscreen element stack.
         from(*currentDoc).popFullscreenElementStack();
 
         //    If doc's fullscreen element stack is non-empty and the element now at the top is either
         //    not in a document or its node document is not doc, repeat this substep.
         newTop = fullscreenElementFrom(*currentDoc);
         if (newTop && (!newTop->inDocument() || newTop->document() != currentDoc))
             continue;
 
         // 2. Queue a task to fire an event named fullscreenchange with its bubbles attribute set to true
         // on doc.
-        addDocumentToFullScreenChangeEventQueue(*currentDoc);
+        Node* target = newTop;
+        if (!target)
+            target = currentFullScreenElementFrom(*currentDoc);
+        if (!target)
+            target = currentDoc;
+        enqueueChangeEvent(*target);
 
         // 3. If doc's fullscreen element stack is empty and doc's browsing context has a browsing context
         // container, set doc to that browsing context container's node document.
         if (!newTop && currentDoc->ownerElement()) {
             currentDoc = &currentDoc->ownerElement()->document();
             continue;
         }
 
         // 4. Otherwise, set doc to null.
         currentDoc = 0;
@@ skipping 131 matching lines @@
     ASSERT(!m_fullScreenRenderer);
 
     m_fullScreenRenderer = renderer;
 }
 
 void FullscreenElementStack::fullScreenRendererDestroyed()
 {
     m_fullScreenRenderer = 0;
 }
 
+void FullscreenElementStack::enqueueChangeEvent(Node& node)
+{
+    m_fullScreenChangeEventTargetQueue.append(&node);
+}
+
 void FullscreenElementStack::fullScreenChangeDelayTimerFired(Timer<FullscreenElementStack>*)
 {
     // Since we dispatch events in this function, it's possible that the
     // 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);
@@ skipping 67 matching lines @@
         return;
 
     m_fullScreenElementStack.removeLast();
 }
 
 void FullscreenElementStack::pushFullscreenElementStack(Element& element)
 {
     m_fullScreenElementStack.append(&element);
 }
 
-void FullscreenElementStack::addDocumentToFullScreenChangeEventQueue(Document& doc)
-{
-    Node* target = 0;
-    if (FullscreenElementStack* fullscreen = fromIfExists(doc)) {
-        target = fullscreen->webkitFullscreenElement();
-        if (!target)
-            target = fullscreen->webkitCurrentFullScreenElement();
-    }
-
-    if (!target)
-        target = &doc;
-    m_fullScreenChangeEventTargetQueue.append(target);
-}
-
 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