Clean up some OOPIF-related naming and comments in Fullscreen.

third_party/WebKit/Source/core/dom/Fullscreen.cpp

Index: third_party/WebKit/Source/core/dom/Fullscreen.cpp
diff --git a/third_party/WebKit/Source/core/dom/Fullscreen.cpp b/third_party/WebKit/Source/core/dom/Fullscreen.cpp
index 030f37b40bfab05f500cbca80fb27e088fde4520..b58812f486717eeccba4569498fd8fe1e09fcffd 100644
--- a/third_party/WebKit/Source/core/dom/Fullscreen.cpp
+++ b/third_party/WebKit/Source/core/dom/Fullscreen.cpp
@@ -198,7 +198,7 @@ Fullscreen::Fullscreen(Document& document)
     : ContextLifecycleObserver(&document)
     , m_fullScreenLayoutObject(nullptr)
     , m_eventQueueTimer(this, &Fullscreen::eventQueueTimerFired)
-    , m_forCrossProcessAncestor(false)
+    , m_forCrossProcessDescendant(false)
 {
     document.setHasFullscreenSupplement();
 }
@@ -224,11 +224,11 @@ void Fullscreen::contextDestroyed()
 
 }
 
-void Fullscreen::requestFullscreen(Element& element, RequestType requestType, bool forCrossProcessAncestor)
+void Fullscreen::requestFullscreen(Element& element, RequestType requestType, bool forCrossProcessDescendant)
 {
     // Use counters only need to be incremented in the process of the actual
     // fullscreen element.
-    if (!forCrossProcessAncestor) {
+    if (!forCrossProcessDescendant) {
         if (document()->isSecureContext()) {
             UseCounter::count(document(), UseCounter::FullscreenSecureOrigin);
         } else {
@@ -259,10 +259,10 @@ void Fullscreen::requestFullscreen(Element& element, RequestType requestType, bo
         //   - an activation behavior is currently being processed whose click event was trusted, or
         //   - the event listener for a trusted click event is being handled.
         //
-        // If |forCrossProcessAncestor| is true, requestFullscreen was already
-        // called on an element in another process, and getting here means that
-        // it already passed the user gesture check.
-        if (!UserGestureIndicator::utilizeUserGesture() && !forCrossProcessAncestor) {
+        // If |forCrossProcessDescendant| is true, requestFullscreen
+        // was already called on a descendant element in another process, and
+        // getting here means that it already passed the user gesture check.
+        if (!UserGestureIndicator::utilizeUserGesture() && !forCrossProcessDescendant) {
             String message = ExceptionMessages::failedToExecute("requestFullScreen",
                 "Element", "API can only be initiated by a user gesture.");
             document()->addConsoleMessage(
@@ -331,7 +331,7 @@ void Fullscreen::requestFullscreen(Element& element, RequestType requestType, bo
             // 4. Otherwise, do nothing for this document. It stays the same.
         } while (++current != docs.end());
 
-        m_forCrossProcessAncestor = forCrossProcessAncestor;
+        m_forCrossProcessDescendant = forCrossProcessDescendant;
 
         // 5. Return, and run the remaining steps asynchronously.
         // 6. Optionally, perform some animation.
@@ -505,13 +505,18 @@ void Fullscreen::didEnterFullScreenForElement(Element* element)
         m_savedPlaceholderComputedStyle = ComputedStyle::clone(layoutObject->styleRef());
     }
 
-    // TODO(alexmos): When |m_forCrossProcessAncestor| is true, some of
+    // TODO(alexmos): When |m_forCrossProcessDescendant| is true, some of

[foolip, 2016/07/13 08:58:14]

I actually think this comment is wrong, the fullscreen element in each document
needs to get the same treatment. I may be missing something, but it doesn't
matter much, because this would go away with the top layer rewrite.

[alexmos, 2016/07/13 17:52:18]

The thinking behind the comment was that when all frames are in the same
process, the LayoutFullScreen::wrapLayoutObject happens only once in the deepest
document, but with OOPIFs, it currently happens once for the deepest document in
each process.  I thought doing it in the other (ancestor) processes isn't really
necessary (though it doesn't hurt), as these other ancestors are just stretching
the inner fullscreened iframe to fill up their entire viewport (via
setContainsFullScreenElement and iframe:-webkit-full-screen) -- I actually had a
prototype working where I skipped doing it, and things still seemed to work
afaict.  Maybe there are other gotchas with this that I wasn't aware of.  But
you're right that at this point it doesn't matter, as this will go away with top
layer, so it's probably not worth spending time to finish that CL.

     // this layout work has already been done in another process, so it should
     // not be necessary to repeat it here.
     if (m_fullScreenElement != document()->documentElement())
         LayoutFullScreen::wrapLayoutObject(layoutObject, layoutObject ? layoutObject->parent() : 0, document());
 
-    if (m_forCrossProcessAncestor) {
+    // When |m_forCrossProcessDescendant| is true, m_fullScreenElement
+    // corresponds to the HTMLFrameOwnerElement for the out-of-process iframe
+    // that contains the actual fullscreen element.   Hence, it must also set

[foolip, 2016/07/13 08:58:14]

I'm really looking forward to getting rid of :-webkit-full-screen-ancestor :)

[alexmos, 2016/07/13 17:52:18]

That sounds great. :)  I guess that's because it isn't part of the spec anymore?
 Do we have/need counters that existing web content isn't relying on it?  I see
that Mozilla just did this too, which is exciting:
https://www.fxsitecompat.com/en-CA/docs/2016/moz-full-screen-ancestor-has-bee...

[foolip, 2016/07/13 21:40:30]

It was completely necessary before in order to get the ancestor elements out of
the way, but now the ancestor elements won't be ancestor layout objects so there
won't be much useful you can do with :-webkit-full-screen-ancestor. The use
counter is https://www.chromestatus.com/metrics/feature/timeline/popularity/628
but I don't think that's a good predictor of risk, I'd have to search
httparchive before attempting a removal.

The last remaining thing that depends on it is to remove scrollbars, but I think
that could be done without the selector.

+    // the ContainsFullScreenElement flag (so that it gains the
+    // -webkit-full-screen-ancestor style).
+    if (m_forCrossProcessDescendant) {
         DCHECK(m_fullScreenElement->isFrameOwnerElement());
         DCHECK(toHTMLFrameOwnerElement(m_fullScreenElement)->contentFrame()->isRemoteFrame());
         m_fullScreenElement->setContainsFullScreenElement(true);
@@ -543,7 +548,7 @@ void Fullscreen::didExitFullScreenForElement()
 
     m_fullScreenElement->willStopBeingFullscreenElement();
 
-    if (m_forCrossProcessAncestor)
+    if (m_forCrossProcessDescendant)
         m_fullScreenElement->setContainsFullScreenElement(false);
 
     m_fullScreenElement->setContainsFullScreenElementOnAncestorsCrossingFrameBoundaries(false);
@@ -567,7 +572,7 @@ void Fullscreen::didExitFullScreenForElement()
     DCHECK(exitingDocument);
     from(*exitingDocument).m_eventQueueTimer.startOneShot(0, BLINK_FROM_HERE);
 
-    m_forCrossProcessAncestor = false;
+    m_forCrossProcessDescendant = false;
 }
 
 void Fullscreen::setFullScreenLayoutObject(LayoutFullScreen* layoutObject)