Add a nextLocalAncestor helper for 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 ce9f4d9eb6ad74df28a9132e97f666b25422695e..b42c543281df21bd7e757841eb15812bfebe196d 100644
--- a/third_party/WebKit/Source/core/dom/Fullscreen.cpp
+++ b/third_party/WebKit/Source/core/dom/Fullscreen.cpp
@@ -194,20 +194,37 @@ Event* createEvent(const AtomicString& type, EventTarget& target) {
   return event;
 }
 
+// Walks the frame tree and returns the first local ancestor frame, if any.
+LocalFrame* nextLocalAncestor(Frame& frame) {
+  Frame* parent = frame.tree().parent();
+  if (!parent)
+    return nullptr;
+  if (parent->isLocalFrame())
+    return toLocalFrame(parent);
+  return nextLocalAncestor(*parent);
+}
+
+// Walks the document's frame tree and returns the document of the first local
+// ancestor frame, if any.
+Document* nextLocalAncestor(Document& document) {
+  if (LocalFrame* frame = document.frame()) {
+    if (LocalFrame* next = nextLocalAncestor(*frame)) {
+      DCHECK(next->document());
+      return next->document();
+    }
+  }
+  return nullptr;

[mlamouri (slow - plz ping), 2016/12/12 13:36:56]

Feel free to ignore but I think it would read better as:
```
if (!document.frame())
  return nullptr;

LocalFrame* next = nextLocalAncestor(*document.frame()))
return next ? next->document() : nullptr;
```

[foolip, 2016/12/12 16:56:31]

The ternary part of that was how I started out, but I worried about whether it
might be null, and think the DCHECK there has some value.

I'll go with consistent early return, which is a little bit longer, but will
probably get compiled to roughly the same thing.

+}
+
 // Helper to walk the ancestor chain and return the Document of the topmost
 // local ancestor frame. Note that this is not the same as the topmost frame's
 // Document, which might be unavailable in OOPIF scenarios. For example, with
 // OOPIFs, when called on the bottom frame's Document in a A-B-C-B hierarchy in
 // process B, this will skip remote frame C and return this frame: A-[B]-C-B.
 Document& topmostLocalAncestor(Document& document) {
-  Document* topmost = &document;
-  Frame* frame = document.frame();
-  while (frame) {
-    frame = frame->tree().parent();
-    if (frame && frame->isLocalFrame())
-      topmost = toLocalFrame(frame)->document();
-  }
-  return *topmost;
+  if (Document* next = nextLocalAncestor(document))
+    return topmostLocalAncestor(*next);
+  return document;
 }
 
 // Helper to find the browsing context container in |doc| that embeds the
@@ -383,13 +400,8 @@ void Fullscreen::requestFullscreen(Element& element,
     // process, where the message should be queued up and processed after
     // the IPC that dispatches fullscreenchange.
     HeapDeque<Member<Document>> docs;
-
-    docs.prepend(&document);
-    for (Frame* frame = document.frame()->tree().parent(); frame;
-         frame = frame->tree().parent()) {
-      if (frame->isLocalFrame())
-        docs.prepend(toLocalFrame(frame)->document());
-    }
+    for (Document* doc = &document; doc; doc = nextLocalAncestor(*doc))
+      docs.prepend(doc);
 
     // 4. For each document in docs, run these substeps:
     HeapDeque<Member<Document>>::iterator current = docs.begin(),
@@ -541,15 +553,8 @@ void Fullscreen::exitFullscreen(Document& document) {
     // browsing context container with a local document.
     // TODO(alexmos): Deal with nested fullscreen cases, see
     // https://crbug.com/617369.
-    if (!newTop) {
-      Frame* frame = currentDoc->frame()->tree().parent();
-      while (frame && frame->isRemoteFrame())
-        frame = frame->tree().parent();
-      if (frame) {
-        currentDoc = toLocalFrame(frame)->document();
-        continue;
-      }
-    }
+    if (!newTop)
+      currentDoc = nextLocalAncestor(*currentDoc);
 
     // 4. Otherwise, set doc to null.
     currentDoc = nullptr;