Sync fullscreen element removal with the spec

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 db98e5b8d0fdb8bd9bd2c0ee2046d18ae596df35..094a66bce935b2b2bf4e198d6fb64f7d782c28ae 100644
--- a/Source/core/dom/FullscreenElementStack.cpp
+++ b/Source/core/dom/FullscreenElementStack.cpp
@@ -560,29 +560,25 @@ void FullscreenElementStack::eventQueueTimerFired(Timer<FullscreenElementStack>*
     }
 }
 
-void FullscreenElementStack::fullScreenElementRemoved()
+void FullscreenElementStack::elementRemoved(Element& element)
 {
-    m_fullScreenElement->setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(false);
-    fullyExitFullscreen();
-}
-
-void FullscreenElementStack::removeFullScreenElementOfSubtree(Node* node, bool amongChildrenOnly)
-{
-    if (!m_fullScreenElement)
-        return;
+    // If an element element in a fullscreen element stack is removed from a document document, run

[falken, 2014/08/14 04:12:47]

I find these hard to read. How about putting italicized words in pipes like
|element| and |document|.

[philipj_slow, 2014/08/14 08:10:39]

I had done this, but then I started worrying about consistency. Let's do it
everywhere:
https://codereview.chromium.org/473803002/

+    // these steps:
 
-    // If the node isn't in a document it can't have a fullscreen'd child.
-    if (!node->inDocument())
+    // 1. If element was at the top of document's fullscreen element stack, act as if the
+    // exitFullscreen() method was invoked on that document.
+    if (fullscreenElement() == &element) {
+        exitFullscreen();
         return;
+    }
 
-    bool elementInSubtree = false;
-    if (amongChildrenOnly)
-        elementInSubtree = m_fullScreenElement->isDescendantOf(node);
-    else
-        elementInSubtree = (m_fullScreenElement == node) || m_fullScreenElement->isDescendantOf(node);
-
-    if (elementInSubtree)
-        fullScreenElementRemoved();
+    // 2. Otherwise, remove element from document's fullscreen element stack.
+    for (size_t i = 0; i < m_fullScreenElementStack.size(); ++i) {
+        if (m_fullScreenElementStack[i].first.get() == &element) {
+            m_fullScreenElementStack.remove(i);
+            return;
+        }
+    }

[falken, 2014/08/14 04:12:47]

Is it worth doing NOTREACHED() here?

[philipj_slow, 2014/08/14 08:10:39]

It's actually reachable, because elementRemoved will be called without knowing
if the element is in fact in the fullscreen element stack. I could try to use
containsFullScreenElement() to optimize, but I want to remove that flag
eventually because it's only really needed for the :-webkit-full-screen-ancestor
pseudo-class which can hopefully be removed after the top-layer rewrite.

I'll add a comment here, but if you prefer I could determine "in fullscreen
element stack" first and then do the steps per spec.

 }
 
 void FullscreenElementStack::clearFullscreenElementStack()