Bug 1727514 - media playback should stop in the bfcache, r=peterv

SetContainer handling is similar to what DocumentViewer does for the old bfcache implementation.
(The old implementation hides it quite well).
The changes to HTMLMediaElement are needed to ensure page can enter bfcache.

Removed erroneous MOZ_ASSERT in nsPresContext, it is ok to trigger that code path in the new implementation.
And the Run() method of the relevant nsIRunnable already deals with that case.

Differential Revision: https://phabricator.services.mozilla.com/D124684

docshell/base/BrowsingContext.cpp

@@ -2842,8 +2842,14 @@ void BrowsingContext::DidSet(FieldIndex<IDX_IsInBFCache>) {
   });
 
   if (isInBFCache) {
-    PreOrderWalk(
-        [&](BrowsingContext* aContext) { aContext->mIsInBFCache = true; });
+    PreOrderWalk([&](BrowsingContext* aContext) {
+      aContext->mIsInBFCache = true;
+      Document* doc = aContext->GetDocument();
+      if (doc) {
+        // Container needs to be cleared after mIsInBFCache is set to true.
+        doc->SetContainer(nullptr);
+      }
+    });
   }
 }
 

docshell/base/nsDocShell.cpp

@@ -1161,6 +1161,7 @@ void nsDocShell::FirePageHideShowNonRecursive(bool aShow) {
     mFiredUnloadEvent = false;
     RefPtr<Document> doc = contentViewer->GetDocument();
     if (doc) {
+      doc->SetContainer(this);
       RefPtr<nsGlobalWindowInner> inner =
           mScriptGlobal ? mScriptGlobal->GetCurrentInnerWindowInternal()
                         : nullptr;

dom/base/Document.cpp

@@ -6933,8 +6933,11 @@ bool Document::RemoveFromBFCacheSync() {
   }
 
   if (mozilla::SessionHistoryInParent() && XRE_IsContentProcess()) {
-    if (BrowsingContext* bc = GetBrowsingContext()) {
-      if (bc->IsInBFCache()) {
+    // Note, Document::GetBrowsingContext() returns null when the document is in
+    // the bfcache.
+    if (nsPIDOMWindowInner* innerWindow = GetInnerWindow()) {
+      BrowsingContext* bc = innerWindow->GetBrowsingContext();
+      if (bc && bc->IsInBFCache()) {
         ContentChild* cc = ContentChild::GetSingleton();
         // IPC is asynchronous but the caller is supposed to check the return
         // value. The reason for 'Sync' in the method name is that the old

dom/base/Document.h

@@ -3650,6 +3650,8 @@ class Document : public nsINode,
   // Return true.
   bool ConsumeTransientUserGestureActivation();
 
+  // Note, GetBrowsingContext() returns null when the document is in
+  // the bfcache.
   BrowsingContext* GetBrowsingContext() const;
 
   // This document is a WebExtension page, it might be a background page, a

dom/html/HTMLMediaElement.cpp

@@ -2566,10 +2566,6 @@ void HTMLMediaElement::SelectResource() {
           !mIsLoadingFromSourceChildren,
           "Should think we're not loading from source children by default");
 
-      if (!mMediaSource) {
-        OwnerDoc()->AddMediaElementWithMSE();
-      }
-
       RemoveMediaElementFromURITable();
       if (!mSrcMediaSource) {
         mLoadingSrc = uri;
@@ -2579,7 +2575,11 @@ void HTMLMediaElement::SelectResource() {
       mLoadingSrcTriggeringPrincipal = mSrcAttrTriggeringPrincipal;
       DDLOG(DDLogCategory::Property, "loading_src",
             nsCString(NS_ConvertUTF16toUTF8(src)));
+      bool hadMediaSource = !!mMediaSource;
       mMediaSource = mSrcMediaSource;
+      if (mMediaSource && !hadMediaSource) {
+        OwnerDoc()->AddMediaElementWithMSE();
+      }
       DDLINKCHILD("mediasource", mMediaSource.get());
       UpdatePreloadAction();
       if (mPreloadAction == HTMLMediaElement::PRELOAD_NONE && !mMediaSource) {
@@ -2833,16 +2833,16 @@ void HTMLMediaElement::LoadFromSourceChildren() {
       return;
     }
 
-    if (!mMediaSource) {
-      OwnerDoc()->AddMediaElementWithMSE();
-    }
-
     RemoveMediaElementFromURITable();
     mLoadingSrc = uri;
     mLoadingSrcTriggeringPrincipal = childSrc->GetSrcTriggeringPrincipal();
     DDLOG(DDLogCategory::Property, "loading_src",
           nsCString(NS_ConvertUTF16toUTF8(src)));
+    bool hadMediaSource = !!mMediaSource;
     mMediaSource = childSrc->GetSrcMediaSource();
+    if (mMediaSource && !hadMediaSource) {
+      OwnerDoc()->AddMediaElementWithMSE();
+    }
     DDLINKCHILD("mediasource", mMediaSource.get());
     NS_ASSERTION(mNetworkState == NETWORK_LOADING,
                  "Network state should be loading");

dom/media/test/file_playback_and_bfcache.html

@@ -0,0 +1,57 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <script>
+  function init() {
+    if (location.search == "") {
+      let bc1 = new BroadcastChannel("bc1");
+      bc1.onmessage = function(e) {
+        if (e.data == "loadNext") {
+          location.href = location.href + "?page2";
+        } else if (e.data == "forward") {
+          bc1.close();
+          history.forward();
+        }
+      };
+      window.onpageshow = function() {
+        bc1.postMessage("pageshow");
+      };
+    } else {
+      document.body.innerHTML = "<video controls src='owl.mp3' autoplay>";
+      let bc2 = new BroadcastChannel("bc2");
+      bc2.onmessage = function(e) {
+        if (e.data == "back") {
+          history.back();
+        } else if (e.data == "statistics") {
+          bc2.postMessage({ currentTime: document.body.firstChild.currentTime,
+                          duration: document.body.firstChild.duration });
+          bc2.close();
+          window.close();
+        }
+      }
+      window.onpageshow = function(e) {
+        bc2.postMessage({ event: "pageshow", persisted: e.persisted});
+        if (!e.persisted) {
+          // The initial statistics is sent once we know the duration and
+          // have loaded all the data.
+          let mediaElement = document.body.firstChild;
+          mediaElement.onpause = function() {
+            mediaElement.onpause = null;
+            mediaElement.currentTime = 0;
+            mediaElement.onplay = function() {
+              setTimeout(function() {
+                bc2.postMessage({ currentTime: mediaElement.currentTime,
+                                  duration: mediaElement.duration });
+                }, 500);
+            }
+            mediaElement.play();
+          }
+        }
+      };
+    }
+  }
+  </script>
+</head>
+<body onload="init()">
+</body>
+</html>

dom/media/test/mochitest.ini

@@ -1103,6 +1103,8 @@ skip-if = appname == "seamonkey" #  Seamonkey: Bug 598252, bug 1307337, bug 1143
 # a platform removes a lot of coverage.
 [test_playback.html]
 skip-if = toolkit == 'android' # bug 1316177
+[test_playback_and_bfcache.html]
+support-files = file_playback_and_bfcache.html
 [test_playback_errors.html]
 [test_playback_rate.html]
 [test_playback_rate_playpause.html]

dom/media/test/test_playback_and_bfcache.html

@@ -0,0 +1,72 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>Test media playback and bfcache</title>
+  <script src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" href="/tests/SimpleTest/test.css"/>
+  <script>
+    SimpleTest.requestFlakyTimeout("Need some timer to wait for the audio to play");
+    SimpleTest.waitForExplicitFinish();
+    var duration = 0;
+
+    // The test opens a page and another page with a media element is loaded.
+    // The media element plays an audio file and starts again and sends
+    // statistics about it and then history.back() is called. The test waits
+    // for 1s + duration of the audio file and goes forward. The audio playback
+    // shouldn't have progressed while the page was in the bfcache.
+    function test() {
+      let bc1 = new BroadcastChannel("bc1");
+      let pageshow1Count = 0;
+      bc1.onmessage = function(e) {
+        if (e.data == "pageshow") {
+          ++pageshow1Count;
+          info("Page 1 pageshow " + pageshow1Count);
+          if (pageshow1Count == 1) {
+            bc1.postMessage("loadNext");
+          } else if (pageshow1Count == 2) {
+            setTimeout(function() {
+                bc1.postMessage("forward");
+                bc1.close();
+              }, (Math.round(duration) + 1) * 1000);
+          }
+        }
+      };
+
+      let bc2 = new BroadcastChannel("bc2");
+      let pageshow2Count = 0;
+      let statisticsCount = 0;
+      bc2.onmessage = function(e) {
+        if (e.data.event == "pageshow") {
+          ++pageshow2Count;
+          info("Page 2 pageshow " + pageshow2Count);
+          if (pageshow2Count == 2) {
+            ok(e.data.persisted, "Should have persisted the page.");
+            bc2.postMessage("statistics");
+          }
+        } else {
+          ++statisticsCount;
+          if (statisticsCount == 1) {
+            duration = e.data.duration;
+            bc2.postMessage("back");
+          } else {
+            is(statisticsCount, 2, "Should got two play events.");
+            ok(e.data.currentTime < e.data.duration,
+               "Should have stopped the playback while the page was in bfcache." +
+               "currentTime: " + e.data.currentTime + " duration: " + e.data.duration);
+            bc2.close();
+            SimpleTest.finish();
+          }
+        }
+      };
+
+      window.open("file_playback_and_bfcache.html", "", "noopener");
+    }
+  </script>
+</head>
+<body onload="test()">
+<p id="display"></p>
+<div id="content" style="display: none"></div>
+<pre id="test"></pre>
+</body>
+</html>

layout/base/nsPresContext.cpp

@@ -2078,10 +2078,7 @@ class DelayedFireDOMPaintEvent : public Runnable {
         mPresContext(aPresContext),
         mTransactionId(aTransactionId),
         mTimeStamp(aTimeStamp),
-        mList(std::move(aList)) {
-    MOZ_ASSERT(mPresContext->GetContainerWeak(),
-               "DOMPaintEvent requested for a detached pres context");
-  }
+        mList(std::move(aList)) {}
   NS_IMETHOD Run() override {
     // The pres context might have been detached during the delay -
     // that's fine, just don't fire the event.