Expand addDocumentToFullScreenChangeEventQueue at the call sites for clarity

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 239fe6a6a818725700a660821d6efb5922a8b9b2..ea5439317d6ea900cb41fd95bfd5bffad09fdc7c 100644
--- a/Source/core/dom/FullscreenElementStack.cpp
+++ b/Source/core/dom/FullscreenElementStack.cpp
@@ -228,7 +228,7 @@ void FullscreenElementStack::requestFullScreenForElement(Element& element, Reque
             // set to true on the document.
             if (!followingDoc) {
                 from(*currentDoc).pushFullscreenElementStack(element);
-                addDocumentToFullScreenChangeEventQueue(*currentDoc);
+                enqueueChangeEvent(element);
                 continue;
             }
 
@@ -239,8 +239,9 @@ void FullscreenElementStack::requestFullScreenForElement(Element& element, Reque
                 // ...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;
             }
 
@@ -308,7 +309,7 @@ void FullscreenElementStack::webkitExitFullscreen()
     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:
@@ -325,7 +326,12 @@ void FullscreenElementStack::webkitExitFullscreen()
 
         // 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.
@@ -477,6 +483,11 @@ 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
@@ -564,20 +575,6 @@ 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);