Convert FullscreenController to use WebCallbacks

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 3c1fe6da5ce35ff84a7934271a59984b176ae802..cd9370491dab55b14cd1db48dc48d0ffa1e79608 100644
--- a/third_party/WebKit/Source/core/dom/Fullscreen.cpp
+++ b/third_party/WebKit/Source/core/dom/Fullscreen.cpp
@@ -48,6 +48,7 @@
 #include "core/svg/SVGSVGElement.h"
 #include "platform/ScopedOrientationChangeIndicator.h"
 #include "platform/UserGestureIndicator.h"
+#include "public/platform/WebFullscreenCallbacks.h"
 
 namespace blink {
 
@@ -186,6 +187,25 @@ HTMLFrameOwnerElement* findContainerForDescendant(const Document& doc,
   return toHTMLFrameOwnerElement(frame->owner());
 }
 
+class FullscreenCallbacks final : public WebFullscreenCallbacks {
+  // FIXME(tasak): When making public/platform classes to use PartitionAlloc,
+  // the following macro should be moved to WebCallbacks defined in
+  // public/platform/WebCallbacks.h.
+  USING_FAST_MALLOC(FullscreenCallbacks);
+  WTF_MAKE_NONCOPYABLE(FullscreenCallbacks);
+
+ public:
+  FullscreenCallbacks(std::unique_ptr<WTF::Closure> success,
+                      std::unique_ptr<WTF::Closure> error)
+      : m_success(std::move(success)), m_error(std::move(error)) {}
+  void onSuccess() override { (*m_success)(); }
+  void onError() override { (*m_error)(); }
+
+ private:
+  std::unique_ptr<WTF::Closure> m_success;
+  std::unique_ptr<WTF::Closure> m_error;
+};
+
 }  // anonymous namespace
 
 const char* Fullscreen::supplementName() {
@@ -298,8 +318,8 @@ void Fullscreen::requestFullscreen(Element& element,
     }
   }
 
-  // Ignore this request if the document is not in a live frame.
-  if (!document.isActive())
+  // Ignore this call if the document is not in a live frame.
+  if (!document.isActive() || !document.frame())
     return;
 
   // If |element| is on top of |doc|'s fullscreen element stack, terminate these
@@ -399,14 +419,22 @@ void Fullscreen::requestFullscreen(Element& element,
 
     // 5. Return, and run the remaining steps asynchronously.
     // 6. Optionally, perform some animation.
-    document.frameHost()->chromeClient().enterFullscreenForElement(&element);
+    document.frameHost()->chromeClient().enterFullscreen(
+        *document.frame(),
+        wrapUnique(new FullscreenCallbacks(

[mlamouri (slow - plz ping), 2016/11/30 09:24:57]

Any particular reason why not keepinp track of `document` and `element` in
`FullscreenCallbacks` and call the right method instead of creating two bind?

[foolip, 2016/11/30 10:23:52]

This is a wart from before when I was using callbacks for exit as well, and I
didn't simplify. I'll see about having FullscreenCallbacks have
Persistent<Document>, Persistent<Element> and a RequestType instead. (One
original reason for this refactoring was that I didn't want to make
FullscreenController keep track of three things like that.)

+            WTF::bind(&Fullscreen::didEnterFullscreenForElement,
+                      wrapPersistent(&from(document)),
+                      wrapPersistent(&element)),
+            WTF::bind(&Fullscreen::enqueueErrorEvent,
+                      wrapPersistent(&from(document)), wrapPersistent(&element),
+                      requestType))));
 
     // 7. Optionally, display a message indicating how the user can exit
     // displaying the context object fullscreen.
     return;
   } while (false);
 
-  from(document).enqueueErrorEvent(element, requestType);
+  from(document).enqueueErrorEvent(&element, requestType);
 }
 
 // https://fullscreen.spec.whatwg.org/#fully-exit-fullscreen
@@ -441,10 +469,11 @@ void Fullscreen::fullyExitFullscreen(Document& document) {
 void Fullscreen::exitFullscreen(Document& document) {
   // The exitFullscreen() method must run these steps:
 
-  // 1. Let doc be the context object. (i.e. "this")
-  if (!document.isActive())
+  // Ignore this call if the document is not in a live frame.
+  if (!document.isActive() || !document.frame())
     return;
 
+  // 1. Let doc be the context object. (i.e. "this")
   // 2. If doc's fullscreen element stack is empty, terminate these steps.
   if (!fullscreenElementFrom(document))
     return;
@@ -454,9 +483,8 @@ void Fullscreen::exitFullscreen(Document& document) {
   // child of the doc is last and the document furthest away from the doc is
   // first.
   HeapDeque<Member<Document>> descendants;
-  for (Frame* descendant =
-           document.frame() ? document.frame()->tree().traverseNext() : nullptr;
-       descendant; descendant = descendant->tree().traverseNext()) {
+  for (Frame* descendant = document.frame()->tree().traverseNext(); descendant;
+       descendant = descendant->tree().traverseNext()) {
     if (!descendant->isLocalFrame())
       continue;
     DCHECK(toLocalFrame(descendant)->document());
@@ -529,15 +557,14 @@ void Fullscreen::exitFullscreen(Document& document) {
   if (!host)
     return;
 
-  // Only exit out of full screen window mode if there are no remaining elements
-  // in the full screen stack.
+  // Only exit fullscreen mode if the fullscreen element stack is empty.
   if (!newTop) {
-    host->chromeClient().exitFullscreen(document.frame());
+    host->chromeClient().exitFullscreen(*document.frame());
     return;
   }
 
-  // Otherwise, notify the chrome of the new full screen element.
-  host->chromeClient().enterFullscreenForElement(newTop);
+  // Otherwise, enter fullscreen for the fullscreen element stack's top element.
+  from(document).didEnterFullscreenForElement(newTop);
 }
 
 // https://fullscreen.spec.whatwg.org/#dom-document-fullscreenenabled
@@ -616,6 +643,13 @@ void Fullscreen::didExitFullscreen() {
   if (!m_currentFullScreenElement)
     return;
 
+  // If fullscreen was canceled by the browser, e.g. if the user pressed Esc,
+  // then |exitFullscreen()| was never called. Let |fullyExitFullscreen()| clear
+  // the fullscreen element stack and fire any events as necessary.
+  // TODO(foolip): Remove this when state changes and events are synchronized
+  // with animation frames. https://crbug.com/402376
+  fullyExitFullscreen(*document());
+
   if (m_forCrossProcessDescendant)
     m_currentFullScreenElement->setContainsFullScreenElement(false);
 
@@ -693,12 +727,14 @@ void Fullscreen::enqueueChangeEvent(Document& document,
   // didEnterFullscreenForElement/didExitFullscreen.
 }
 
-void Fullscreen::enqueueErrorEvent(Element& element, RequestType requestType) {
+void Fullscreen::enqueueErrorEvent(Element* element, RequestType requestType) {
+  DCHECK(element);
+
   Event* event;
   if (requestType == UnprefixedRequest)
-    event = createEvent(EventTypeNames::fullscreenerror, element.document());
+    event = createEvent(EventTypeNames::fullscreenerror, element->document());
   else
-    event = createEvent(EventTypeNames::webkitfullscreenerror, element);
+    event = createEvent(EventTypeNames::webkitfullscreenerror, *element);
   m_eventQueue.append(event);
   m_eventQueueTimer.startOneShot(0, BLINK_FROM_HERE);
 }