Bug 1188256 part 7 - Have Element.requestFullscreen return a Promise. r=smaug
authorXidorn Quan <me@upsuper.org>
Fri, 14 Sep 2018 22:44:21 +0000
changeset 492141 0665a323aec7354788b6be532a21ac79da3a3ff8
parent 492140 4f7527b669df5e649cf58d58d940731b6e76a98f
child 492142 e92f3e990b354ba419bfc02206da0254595097a8
push id9984
push userffxbld-merge
push dateMon, 15 Oct 2018 21:07:35 +0000
treeherdermozilla-beta@183d27ea8570 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1188256
milestone64.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1188256 part 7 - Have Element.requestFullscreen return a Promise. r=smaug Depends on D5858 Differential Revision: https://phabricator.services.mozilla.com/D5853
dom/base/Element.cpp
dom/base/Element.h
dom/base/FullscreenRequest.h
dom/base/nsDocument.cpp
dom/html/test/file_fullscreen-api.html
dom/webidl/Element.webidl
testing/web-platform/meta/fullscreen/api/promises-reject.html.ini
testing/web-platform/meta/fullscreen/idlharness.window.js.ini
--- a/dom/base/Element.cpp
+++ b/dom/base/Element.cpp
@@ -3565,32 +3565,34 @@ GetFullscreenError(CallerType aCallerTyp
 {
   if (!nsContentUtils::IsRequestFullscreenAllowed(aCallerType)) {
     return "FullscreenDeniedNotInputDriven";
   }
 
   return nullptr;
 }
 
-void
-Element::RequestFullscreen(CallerType aCallerType, ErrorResult& aError)
+already_AddRefed<Promise>
+Element::RequestFullscreen(CallerType aCallerType, ErrorResult& aRv)
 {
-  auto request = FullscreenRequest::Create(this, aCallerType);
+  auto request = FullscreenRequest::Create(this, aCallerType, aRv);
+  RefPtr<Promise> promise = request->GetPromise();
   // Only grant fullscreen requests if this is called from inside a trusted
   // event handler (i.e. inside an event handler for a user initiated event).
   // This stops the fullscreen from being abused similar to the popups of old,
   // and it also makes it harder for bad guys' script to go fullscreen and
   // spoof the browser chrome/window and phish logins etc.
   // Note that requests for fullscreen inside a web app's origin are exempt
   // from this restriction.
   if (const char* error = GetFullscreenError(aCallerType)) {
     request->Reject(error);
-    return;
-  }
-  OwnerDoc()->AsyncRequestFullscreen(std::move(request));
+  } else {
+    OwnerDoc()->AsyncRequestFullscreen(std::move(request));
+  }
+  return promise.forget();
 }
 
 void
 Element::RequestPointerLock(CallerType aCallerType)
 {
   OwnerDoc()->RequestPointerLock(this, aCallerType);
 }
 
--- a/dom/base/Element.h
+++ b/dom/base/Element.h
@@ -1243,17 +1243,17 @@ public:
 
   void ReleaseCapture()
   {
     if (nsIPresShell::GetCapturingContent() == this) {
       nsIPresShell::SetCapturingContent(nullptr, 0);
     }
   }
 
-  void RequestFullscreen(CallerType aCallerType, ErrorResult& aError);
+  already_AddRefed<Promise> RequestFullscreen(CallerType, ErrorResult&);
   void RequestPointerLock(CallerType aCallerType);
   Attr* GetAttributeNode(const nsAString& aName);
   already_AddRefed<Attr> SetAttributeNode(Attr& aNewAttr,
                                           ErrorResult& aError);
   already_AddRefed<Attr> RemoveAttributeNode(Attr& aOldAttr,
                                              ErrorResult& aError);
   Attr* GetAttributeNodeNS(const nsAString& aNamespaceURI,
                            const nsAString& aLocalName);
--- a/dom/base/FullscreenRequest.h
+++ b/dom/base/FullscreenRequest.h
@@ -10,84 +10,114 @@
 
 #ifndef mozilla_FullscreenRequest_h
 #define mozilla_FullscreenRequest_h
 
 #include "mozilla/LinkedList.h"
 #include "mozilla/PendingFullscreenEvent.h"
 #include "mozilla/UniquePtr.h"
 #include "mozilla/dom/Element.h"
+#include "mozilla/dom/Promise.h"
 #include "nsIDocument.h"
 #include "nsIScriptError.h"
 
 namespace mozilla {
 
 struct FullscreenRequest : public LinkedListElement<FullscreenRequest>
 {
+  typedef dom::Promise Promise;
+
   static UniquePtr<FullscreenRequest> Create(Element* aElement,
-                                             dom::CallerType aCallerType)
+                                             dom::CallerType aCallerType,
+                                             ErrorResult& aRv)
   {
-    return WrapUnique(new FullscreenRequest(aElement, aCallerType, true));
+    RefPtr<Promise> promise = Promise::Create(aElement->GetOwnerGlobal(), aRv);
+    return WrapUnique(new FullscreenRequest(aElement, promise.forget(),
+                                            aCallerType, true));
   }
 
   static UniquePtr<FullscreenRequest> CreateForRemote(Element* aElement)
   {
-    return WrapUnique(
-      new FullscreenRequest(aElement, dom::CallerType::NonSystem, false));
+    return WrapUnique(new FullscreenRequest(aElement, nullptr,
+                                            dom::CallerType::NonSystem,
+                                            false));
   }
 
   FullscreenRequest(const FullscreenRequest&) = delete;
 
   ~FullscreenRequest()
   {
     MOZ_COUNT_DTOR(FullscreenRequest);
+    MOZ_ASSERT_IF(mPromise,
+                  mPromise->State() != Promise::PromiseState::Pending);
   }
 
   dom::Element* Element() const { return mElement; }
   nsIDocument* Document() const { return mDocument; }
+  dom::Promise* GetPromise() const { return mPromise; }
+
+  void MayResolvePromise() const
+  {
+    if (mPromise) {
+      MOZ_ASSERT(mPromise->State() == Promise::PromiseState::Pending);
+      mPromise->MaybeResolveWithUndefined();
+    }
+  }
+
+  void MayRejectPromise() const
+  {
+    if (mPromise) {
+      MOZ_ASSERT(mPromise->State() == Promise::PromiseState::Pending);
+      mPromise->MaybeReject(NS_ERROR_DOM_TYPE_ERR);
+    }
+  }
 
   // Reject the fullscreen request with the given reason.
   // It will dispatch the fullscreenerror event.
   void Reject(const char* aReason) const
   {
     if (nsPresContext* presContext = mDocument->GetPresContext()) {
       auto pendingEvent = MakeUnique<PendingFullscreenEvent>(
           FullscreenEventType::Error, mDocument, mElement);
       presContext->RefreshDriver()->
         ScheduleFullscreenEvent(std::move(pendingEvent));
     }
+    MayRejectPromise();
     nsContentUtils::ReportToConsole(nsIScriptError::warningFlag,
                                     NS_LITERAL_CSTRING("DOM"),
                                     mDocument,
                                     nsContentUtils::eDOM_PROPERTIES,
                                     aReason);
   }
 
 private:
   RefPtr<dom::Element> mElement;
   RefPtr<nsIDocument> mDocument;
+  RefPtr<dom::Promise> mPromise;
 
 public:
   // This value should be true if the fullscreen request is
   // originated from system code.
   const dom::CallerType mCallerType;
   // This value denotes whether we should trigger a NewOrigin event if
   // requesting fullscreen in its document causes the origin which is
   // fullscreen to change. We may want *not* to trigger that event if
   // we're calling RequestFullscreen() as part of a continuation of a
   // request in a subdocument in different process, whereupon the caller
   // need to send some notification itself with the real origin.
   const bool mShouldNotifyNewOrigin;
 
 private:
   FullscreenRequest(dom::Element* aElement,
+                    already_AddRefed<dom::Promise> aPromise,
                     dom::CallerType aCallerType,
                     bool aShouldNotifyNewOrigin)
     : mElement(aElement)
     , mDocument(aElement->OwnerDoc())
+    , mPromise(aPromise)
     , mCallerType(aCallerType)
     , mShouldNotifyNewOrigin(aShouldNotifyNewOrigin)
   {
     MOZ_COUNT_CTOR(FullscreenRequest);
   }
 };
 
 } // namespace mozilla
--- a/dom/base/nsDocument.cpp
+++ b/dom/base/nsDocument.cpp
@@ -11140,17 +11140,23 @@ GetFullscreenError(nsIDocument* aDoc, Ca
   }
   return nullptr;
 }
 
 bool
 nsIDocument::FullscreenElementReadyCheck(const FullscreenRequest& aRequest)
 {
   Element* elem = aRequest.Element();
+  // Strictly speaking, this isn't part of the fullscreen element ready
+  // check in the spec, but per steps in the spec, when an element which
+  // is already the fullscreen element requests fullscreen, nothing
+  // should change and no event should be dispatched, but we still need
+  // to resolve the returned promise.
   if (elem == FullscreenStackTop()) {
+    aRequest.MayResolvePromise();
     return false;
   }
   if (!elem->IsInComposedDoc()) {
     aRequest.Reject("FullscreenDeniedNotInDocument");
     return false;
   }
   if (elem->OwnerDoc() != this) {
     aRequest.Reject("FullscreenDeniedMovedDocument");
@@ -11185,16 +11191,17 @@ nsIDocument::FullscreenElementReadyCheck
   if (!nsContentUtils::IsChromeDoc(this) && !IsInActiveTab(this)) {
     aRequest.Reject("FullscreenDeniedNotFocusedTab");
     return false;
   }
   // Deny requests when a windowed plugin is focused.
   nsFocusManager* fm = nsFocusManager::GetFocusManager();
   if (!fm) {
     NS_WARNING("Failed to retrieve focus manager in fullscreen request.");
+    aRequest.MayRejectPromise();
     return false;
   }
   if (nsContentUtils::HasPluginWithUncontrolledEventDispatch(fm->GetFocusedElement())) {
     aRequest.Reject("FullscreenDeniedFocusedPlugin");
     return false;
   }
   return true;
 }
@@ -11261,19 +11268,20 @@ public:
       return thisRequest;
     }
     void SkipToNextMatch()
     {
       while (mCurrent) {
         nsCOMPtr<nsIDocShellTreeItem>
           docShell = mCurrent->Document()->GetDocShell();
         if (!docShell) {
-          // Always automatically drop documents which has been
-          // detached from the doc shell.
-          TakeAndNextInternal();
+          // Always automatically drop fullscreen requests which is from
+          // a document detached from the doc shell.
+          UniquePtr<FullscreenRequest> request = TakeAndNextInternal();
+          request->MayRejectPromise();
         } else {
           while (docShell && docShell != mRootShellForIteration) {
             docShell->GetParent(getter_AddRefs(docShell));
           }
           if (!docShell) {
             // We've gone over the root, but haven't find the target
             // ancestor, so skip this item.
             mCurrent = mCurrent->getNext();
@@ -11341,16 +11349,17 @@ ShouldApplyFullscreenDirectly(nsIDocumen
   }
 }
 
 void
 nsIDocument::RequestFullscreen(UniquePtr<FullscreenRequest> aRequest)
 {
   nsCOMPtr<nsPIDOMWindowOuter> rootWin = GetRootWindow(this);
   if (!rootWin) {
+    aRequest->MayRejectPromise();
     return;
   }
 
   if (ShouldApplyFullscreenDirectly(this, rootWin)) {
     ApplyFullscreen(std::move(aRequest));
     return;
   }
 
@@ -11400,17 +11409,18 @@ nsIDocument::HandlePendingFullscreenRequ
 }
 
 static void
 ClearPendingFullscreenRequests(nsIDocument* aDoc)
 {
   PendingFullscreenRequestList::Iterator iter(
     aDoc, PendingFullscreenRequestList::eInclusiveDescendants);
   while (!iter.AtEnd()) {
-    iter.TakeAndNext();
+    UniquePtr<FullscreenRequest> request = iter.TakeAndNext();
+    request->MayRejectPromise();
   }
 }
 
 bool
 nsIDocument::ApplyFullscreen(UniquePtr<FullscreenRequest> aRequest)
 {
   if (!FullscreenElementReadyCheck(*aRequest)) {
     return false;
@@ -11500,16 +11510,17 @@ nsIDocument::ApplyFullscreen(UniquePtr<F
   }
 
   // Dispatch "fullscreenchange" events. Note that the loop order is
   // reversed so that events are dispatched in the tree order as
   // indicated in the spec.
   for (nsIDocument* d : Reversed(changed)) {
     DispatchFullscreenChange(d, d->FullscreenStackTop());
   }
+  aRequest->MayResolvePromise();
   return true;
 }
 
 bool
 nsIDocument::FullscreenEnabled(CallerType aCallerType)
 {
   return !GetFullscreenError(this, aCallerType);
 }
--- a/dom/html/test/file_fullscreen-api.html
+++ b/dom/html/test/file_fullscreen-api.html
@@ -46,17 +46,33 @@ var inDocElement = null;
 var container = null;
 var button = null;
 
 
 function sendMouseClick(element) {
   synthesizeMouseAtCenter(element, {});
 }
 
+function assertPromiseResolved(promise, msg) {
+  let { state, value } = SpecialPowers.PromiseDebugging.getState(promise);
+  is(state, "fulfilled", "Promise should have been resolved " + msg);
+  is(value, undefined, "Promise should be resolved with undefined " + msg);
+}
+
+function assertPromiseRejected(promise, msg) {
+  let { state, reason } = SpecialPowers.PromiseDebugging.getState(promise);
+  is(state, "rejected", "Promise should have been rejected " + msg);
+  // XXX Actually we should be testing "instanceof TypeError", but it
+  // doesn't work as expected currently. See bug 1412856.
+  is(reason.name, "TypeError",
+     "Promise should be rejected with TypeError " + msg);
+}
+
 const FULLSCREEN_ELEMENT = document.getElementById("fullscreen-element");
+let promise;
 
 function enter1(event) {
   is(event.target, FULLSCREEN_ELEMENT,
      "Event target should be the fullscreen element #1");
   ok(document.fullscreen, "Document should be in fullscreen");
   is(document.fullscreenElement, FULLSCREEN_ELEMENT,
      "Full-screen element should be div element.");
   ok(document.fullscreenElement.matches(":fullscreen"),
@@ -98,24 +114,25 @@ function enter2(event) {
 
 function exit2(event) {
   is(document.fullscreenElement, null,
      "Full-screen element should have rolled back.");
   is(iframe.contentDocument.fullscreenElement, null,
      "Full-screen element in subframe should be null");
   
   addFullscreenChangeContinuation("enter", enter3);
-  FULLSCREEN_ELEMENT.requestFullscreen();
+  promise = FULLSCREEN_ELEMENT.requestFullscreen();
 }
 
 function enter3(event) {
   is(event.target, FULLSCREEN_ELEMENT,
      "Event target should be the fullscreen element #3");
   is(document.fullscreenElement, FULLSCREEN_ELEMENT,
      "Full-screen element should be div.");
+  assertPromiseResolved(promise, "in enter3");
   
   // Transplant the FSE into subdoc. Should exit full-screen.
   addFullscreenChangeContinuation("exit", exit3);
   var _innerFrame = iframe.contentDocument.getElementById("inner-frame");
   _innerFrame.contentDocument.body.appendChild(FULLSCREEN_ELEMENT);
   is(document.fullscreenElement, null,
      "Full-screen element transplanted, should be null.");
   is(iframe.contentDocument.fullscreenElement, null,
@@ -130,22 +147,23 @@ function exit3(event) {
   is(document.fullscreenElement, null, "Full-screen element should be null.");
   document.body.removeChild(iframe);
   iframe = null;
 
   // Do a request out of document. It should be denied.
   // Continue test in the following fullscreenerror handler.
   outOfDocElement = document.createElement("div");
   addFullscreenErrorContinuation(error1);    
-  outOfDocElement.requestFullscreen();
+  promise = outOfDocElement.requestFullscreen();
 }
 
 function error1(event) {
   ok(!document.fullscreenElement,
      "Requests for full-screen from not-in-doc elements should fail.");
+  assertPromiseRejected(promise, "in error1");
   container = document.createElement("div");
   inDocElement = document.createElement("div");
   container.appendChild(inDocElement);
   FULLSCREEN_ELEMENT.appendChild(container);
 
   addFullscreenChangeContinuation("enter", enter4);
   inDocElement.requestFullscreen();
 }
--- a/dom/webidl/Element.webidl
+++ b/dom/webidl/Element.webidl
@@ -257,19 +257,19 @@ Element implements ChildNode;
 Element implements NonDocumentTypeChildNode;
 Element implements ParentNode;
 Element implements Animatable;
 Element implements GeometryUtils;
 
 // https://fullscreen.spec.whatwg.org/#api
 partial interface Element {
   [Throws, Func="nsDocument::IsUnprefixedFullscreenEnabled", NeedsCallerType]
-  void requestFullscreen();
+  Promise<void> requestFullscreen();
   [Throws, BinaryName="requestFullscreen", NeedsCallerType]
-  void mozRequestFullScreen();
+  Promise<void> mozRequestFullScreen();
 
   // Events handlers
   [Func="nsDocument::IsUnprefixedFullscreenEnabled"]
   attribute EventHandler onfullscreenchange;
   [Func="nsDocument::IsUnprefixedFullscreenEnabled"]
   attribute EventHandler onfullscreenerror;
 };
 
--- a/testing/web-platform/meta/fullscreen/api/promises-reject.html.ini
+++ b/testing/web-platform/meta/fullscreen/api/promises-reject.html.ini
@@ -1,7 +1,4 @@
 [promises-reject.html]
-  [Promises#reject]
-    expected: FAIL
-
   [Promises#reject 1]
     expected: FAIL
 
--- a/testing/web-platform/meta/fullscreen/idlharness.window.js.ini
+++ b/testing/web-platform/meta/fullscreen/idlharness.window.js.ini
@@ -1,7 +1,4 @@
 [idlharness.window.html]
-  [Element interface: operation requestFullscreen()]
-    expected: FAIL
-
   [Document interface: operation exitFullscreen()]
     expected: FAIL