Bug 1551570 - Ensure Picture-in-Picture player window is closed before updating internal state. r=JSON_voorhees
authorMike Conley <mconley@mozilla.com>
Fri, 17 May 2019 15:28:45 +0000
changeset 474342 c9f22236d5a52ae59692b8d5c12d63806a987ff3
parent 474341 65747386d437720e71bf5a57eb637dc56e343dd2
child 474343 edafe8d51cbf5f1585db25785ef204936777392e
push id36030
push userrgurzau@mozilla.com
push dateFri, 17 May 2019 21:41:01 +0000
treeherdermozilla-central@7c540586aedb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersJSON_voorhees
bugs1551570
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 1551570 - Ensure Picture-in-Picture player window is closed before updating internal state. r=JSON_voorhees Differential Revision: https://phabricator.services.mozilla.com/D31081
toolkit/components/pictureinpicture/PictureInPicture.jsm
toolkit/components/pictureinpicture/content/player.js
--- a/toolkit/components/pictureinpicture/PictureInPicture.jsm
+++ b/toolkit/components/pictureinpicture/PictureInPicture.jsm
@@ -47,35 +47,38 @@ var PictureInPicture = {
         if (controls) {
           controls.classList.remove("playing");
         }
         break;
       }
     }
   },
 
-  focusTabAndClosePip() {
+  async focusTabAndClosePip() {
     let gBrowser = this.browser.ownerGlobal.gBrowser;
     let tab = gBrowser.getTabForBrowser(this.browser);
     gBrowser.selectedTab = tab;
-    this.unload();
-    this.closePipWindow();
+    await this.closePipWindow();
   },
 
   /**
    * Find and close any pre-existing Picture in Picture windows.
    */
-  closePipWindow() {
+  async closePipWindow() {
     // This uses an enumerator, but there really should only be one of
     // these things.
     for (let win of Services.wm.getEnumerator(WINDOW_TYPE)) {
       if (win.closed) {
         continue;
       }
+      let closedPromise = new Promise(resolve => {
+        win.addEventListener("unload", resolve, {once: true});
+      });
       win.close();
+      await closedPromise;
     }
   },
 
   /**
    * A request has come up from content to open a Picture in Picture
    * window.
    *
    * @param browser (xul:browser)
@@ -90,19 +93,21 @@ var PictureInPicture = {
    *   videoWidth (int):
    *     The preferred width of the video.
    *
    * @returns Promise
    *   Resolves once the Picture in Picture window has been created, and
    *   the player component inside it has finished loading.
    */
   async handlePictureInPictureRequest(browser, videoData) {
-    this.browser = browser;
+    // If there's a pre-existing PiP window, close it first.
+    await this.closePipWindow();
+
     let parentWin = browser.ownerGlobal;
-    this.closePipWindow();
+    this.browser = browser;
     let win = await this.openPipWindow(parentWin, videoData);
     let controls = win.document.getElementById("controls");
     this.weakPipControls = Cu.getWeakReference(controls);
     if (videoData.playing) {
       controls.classList.add("playing");
     }
     win.setupPlayer(browser, videoData);
   },
--- a/toolkit/components/pictureinpicture/content/player.js
+++ b/toolkit/components/pictureinpicture/content/player.js
@@ -36,17 +36,17 @@ async function setupPlayer(originatingBr
   });
 
   // If the content process hosting the video crashes, let's
   // just close the window for now.
   browser.addEventListener("oop-browser-crashed", () => {
     window.close();
   });
 
-  browser.addEventListener("unload", () => {
+  window.addEventListener("unload", () => {
     PictureInPicture.unload();
   });
 
   let close = document.getElementById("close");
   close.addEventListener("click", () => {
     window.close();
   });