Bug 1536209 - Add documentation for PictureInPictureChild, simplify a few things, and clean-up some anonymous functions. r=Felipe
authorMike Conley <mconley@mozilla.com>
Fri, 22 Mar 2019 23:33:53 +0000
changeset 465799 eae1fd2ff0c6f840da830773f851e2adb4e7dc35
parent 465798 f82a40540fd102f649c2a90037744aa83a9834af
child 465800 208cf2d82f55bf31d951e818ca0e3b29ef2f5ebe
push id35746
push usershindli@mozilla.com
push dateSat, 23 Mar 2019 09:46:24 +0000
treeherdermozilla-central@02b7484f316b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersFelipe
bugs1536209
milestone68.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 1536209 - Add documentation for PictureInPictureChild, simplify a few things, and clean-up some anonymous functions. r=Felipe Differential Revision: https://phabricator.services.mozilla.com/D24432
toolkit/actors/PictureInPictureChild.jsm
--- a/toolkit/actors/PictureInPictureChild.jsm
+++ b/toolkit/actors/PictureInPictureChild.jsm
@@ -3,28 +3,38 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 "use strict";
 
 var EXPORTED_SYMBOLS = ["PictureInPictureChild"];
 
 const {ActorChild} = ChromeUtils.import("resource://gre/modules/ActorChild.jsm");
 
+// A weak reference to the most recent <video> in this content
+// process that is being viewed in Picture-in-Picture.
 var gWeakVideo = null;
+// A weak reference to the content window of the most recent
+// Picture-in-Picture window for this content process.
 var gWeakPlayerContent = null;
 
 class PictureInPictureChild extends ActorChild {
   handleEvent(event) {
     switch (event.type) {
       case "MozTogglePictureInPicture": {
         if (event.isTrusted) {
           this.togglePictureInPicture(event.target);
         }
         break;
       }
+      case "pagehide": {
+        // The originating video's content document has unloaded,
+        // so close Picture-in-Picture.
+        this.closePictureInPicture();
+        break;
+      }
     }
   }
 
   get weakVideo() {
     if (gWeakVideo) {
       return gWeakVideo.get();
     }
     return null;
@@ -32,36 +42,73 @@ class PictureInPictureChild extends Acto
 
   get weakPlayerContent() {
     if (gWeakPlayerContent) {
       return gWeakPlayerContent.get();
     }
     return null;
   }
 
+  /**
+   * Tells the parent to open a Picture-in-Picture window hosting
+   * a clone of the passed video. If we know about a pre-existing
+   * Picture-in-Picture window existing, this tells the parent to
+   * close it before opening the new one.
+   *
+   * @param {Element} video The <video> element to view in a Picture
+   * in Picture window.
+   *
+   * @return {Promise}
+   * @resolves {undefined} Once the new Picture-in-Picture window
+   * has been requested.
+   */
   async togglePictureInPicture(video) {
     if (this.inPictureInPicture(video)) {
       await this.closePictureInPicture();
     } else {
       if (this.weakVideo) {
         // There's a pre-existing Picture-in-Picture window for a video
         // in this content process. Send a message to the parent to close
         // the Picture-in-Picture window.
         await this.closePictureInPicture();
       }
 
-      this.requestPictureInPicture(video);
+      gWeakVideo = Cu.getWeakReference(video);
+      this.mm.sendAsyncMessage("PictureInPicture:Request", {
+        videoHeight: video.videoHeight,
+        videoWidth: video.videoWidth,
+      });
     }
   }
 
+  /**
+   * Returns true if the passed video happens to be the one that this
+   * content process is running in a Picture-in-Picture window.
+   *
+   * @param {Element} video The <video> element to check.
+   *
+   * @return {Boolean}
+   */
   inPictureInPicture(video) {
     return this.weakVideo === video;
   }
 
+  /**
+   * Tells the parent to close a pre-existing Picture-in-Picture
+   * window.
+   *
+   * @return {Promise}
+   *
+   * @resolves {undefined} Once the pre-existing Picture-in-Picture
+   * window has unloaded.
+   */
   async closePictureInPicture() {
+    if (this.weakVideo) {
+      this.untrackOriginatingVideo(this.weakVideo);
+    }
 
     this.mm.sendAsyncMessage("PictureInPicture:Close", {
       browingContextId: this.docShell.browsingContext.id,
     });
 
     if (this.weakPlayerContent) {
       await new Promise(resolve => {
         this.weakPlayerContent.addEventListener("unload", resolve,
@@ -70,33 +117,62 @@ class PictureInPictureChild extends Acto
       // Nothing should be holding a reference to the Picture-in-Picture
       // player window content at this point, but just in case, we'll
       // clear the weak reference directly so nothing else can get a hold
       // of it from this angle.
       gWeakPlayerContent = null;
     }
   }
 
-  requestPictureInPicture(video) {
-    gWeakVideo = Cu.getWeakReference(video);
-    this.mm.sendAsyncMessage("PictureInPicture:Request", {
-      videoHeight: video.videoHeight,
-      videoWidth: video.videoWidth,
-    });
-  }
-
   receiveMessage(message) {
     switch (message.name) {
       case "PictureInPicture:SetupPlayer": {
         this.setupPlayer();
         break;
       }
     }
   }
 
+  /**
+   * Keeps an eye on the originating video's document. If it ever
+   * goes away, this will cause the Picture-in-Picture window for any
+   * of its content to go away as well.
+   */
+  trackOriginatingVideo(originatingVideo) {
+    let originatingWindow = originatingVideo.ownerGlobal;
+    if (originatingWindow) {
+      originatingWindow.addEventListener("pagehide", this);
+    }
+  }
+
+  /**
+   * Stops tracking the originating video's document. This should
+   * happen once the Picture-in-Picture window goes away (or is about
+   * to go away), and we no longer care about hearing when the originating
+   * window's document unloads.
+   */
+  untrackOriginatingVideo(originatingVideo) {
+    let originatingWindow = originatingVideo.ownerGlobal;
+    if (originatingWindow) {
+      originatingWindow.removeEventListener("pagehide", this);
+    }
+  }
+
+  /**
+   * Runs in an instance of PictureInPictureChild for the
+   * player window's content, and not the originating video
+   * content. Sets up the player so that it clones the originating
+   * video. If anything goes wrong during set up, a message is
+   * sent to the parent to close the Picture-in-Picture window.
+   *
+   * @return {Promise}
+   * @resolves {undefined} Once the player window has been set up
+   * properly, or a pre-existing Picture-in-Picture window has gone
+   * away due to an unexpected error.
+   */
   async setupPlayer() {
     let originatingVideo = this.weakVideo;
     if (!originatingVideo) {
       // If the video element has gone away before we've had a chance to set up
       // Picture-in-Picture for it, tell the parent to close the Picture-in-Picture
       // window.
       await this.closePictureInPicture();
       return;
@@ -132,24 +208,20 @@ class PictureInPictureChild extends Acto
     playerVideo.style.margin = "0";
     doc.body.style.overflow = "hidden";
     doc.body.style.margin = "0";
 
     doc.body.appendChild(playerVideo);
 
     originatingVideo.cloneElementVisually(playerVideo);
 
-    let originatingWindow = originatingVideo.ownerGlobal;
-    originatingWindow.addEventListener("unload", (e) => {
-      this.closePictureInPicture(originatingVideo);
-    }, { once: true });
+    this.trackOriginatingVideo(originatingVideo);
 
     this.content.addEventListener("unload", () => {
-      let video = gWeakVideo && gWeakVideo.get();
-      if (video) {
-        video.stopCloningElementVisually();
+      if (this.weakVideo) {
+        this.weakVideo.stopCloningElementVisually();
       }
       gWeakVideo = null;
     }, { once: true });
 
     gWeakPlayerContent = Cu.getWeakReference(this.content);
   }
 }