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

This version doesn't change SetContainer handling, since it seems to be tricky for the top level page.
So only activity change notification is fired and IsActive() is updated.
The comment about IsActive() was wrong even with the old bfcache implementation.
(I did check that it returned false when the page was in bfcache and many of the activity observers rely on that)

The changes to HTMLMediaElement are needed to ensure page can enter bfcache..

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) {
+        // Notifying needs to happen after mIsInBFCache is set to true.
+        doc->NotifyActivityChanged();
+      }
+    });
   }
 }
 

docshell/base/nsDocShell.cpp

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

dom/base/Document.cpp

@@ -7405,7 +7405,7 @@ bool Document::ContainsMSEContent() {
   return containsMSE;
 }
 
-static void NotifyActivityChanged(nsISupports* aSupports) {
+static void NotifyActivityChangedCallback(nsISupports* aSupports) {
   nsCOMPtr<nsIContent> content(do_QueryInterface(aSupports));
   if (auto mediaElem = HTMLMediaElement::FromNodeOrNull(content)) {
     mediaElem->NotifyOwnerDocumentActivityChanged();
@@ -7431,6 +7431,10 @@ static void NotifyActivityChanged(nsISupports* aSupports) {
   }
 }
 
+void Document::NotifyActivityChanged() {
+  EnumerateActivityObservers(NotifyActivityChangedCallback);
+}
+
 bool Document::IsTopLevelWindowInactive() const {
   if (BrowsingContext* bc = GetBrowsingContext()) {
     return !bc->GetIsActiveBrowserWindow();
@@ -7449,7 +7453,7 @@ void Document::SetContainer(nsDocShell* aContainer) {
   mInChromeDocShell =
       aContainer && aContainer->GetBrowsingContext()->IsChrome();
 
-  EnumerateActivityObservers(NotifyActivityChanged);
+  NotifyActivityChanged();
 
   // IsTopLevelWindowInactive depends on the docshell, so
   // update the cached value now that it's available.
@@ -11273,7 +11277,7 @@ void Document::RemovedFromDocShell() {
   if (mRemovedFromDocShell) return;
 
   mRemovedFromDocShell = true;
-  EnumerateActivityObservers(NotifyActivityChanged);
+  NotifyActivityChanged();
 
   for (nsIContent* child = GetFirstChild(); child;
        child = child->GetNextSibling()) {
@@ -11543,7 +11547,7 @@ void Document::OnPageShow(bool aPersisted, EventTarget* aDispatchStartTarget,
     UpdateVisibilityState();
   }
 
-  EnumerateActivityObservers(NotifyActivityChanged);
+  NotifyActivityChanged();
 
   auto notifyExternal = [aPersisted](Document& aExternalResource) {
     aExternalResource.OnPageShow(aPersisted, nullptr);
@@ -11667,7 +11671,7 @@ void Document::OnPageHide(bool aPersisted, EventTarget* aDispatchStartTarget,
     return CallState::Continue;
   };
   EnumerateExternalResources(notifyExternal);
-  EnumerateActivityObservers(NotifyActivityChanged);
+  NotifyActivityChanged();
 
   ClearPendingFullscreenRequests(this);
   if (GetUnretargetedFullScreenElement()) {
@@ -12503,6 +12507,11 @@ void Document::SetSuppressedEventListener(EventListener* aListener) {
   EnumerateSubDocuments(setOnSubDocs);
 }
 
+bool Document::IsActive() const {
+  return mDocumentContainer && !mRemovedFromDocShell && GetBrowsingContext() &&
+         !GetBrowsingContext()->IsInBFCache();
+}
+
 nsISupports* Document::GetCurrentContentSink() {
   return mParser ? mParser->GetContentSink() : nullptr;
 }
@@ -14921,7 +14930,7 @@ void Document::UpdateVisibilityState(DispatchVisibilityChange aDispatchEvent) {
                                            u"visibilitychange"_ns,
                                            CanBubble::eYes, Cancelable::eNo);
     }
-    EnumerateActivityObservers(NotifyActivityChanged);
+    NotifyActivityChanged();
     if (mVisibilityState == dom::VisibilityState::Visible) {
       MaybeActiveMediaComponents();
     }
@@ -15566,7 +15575,7 @@ void Document::IncLazyLoadImageReachViewport(bool aLoading) {
 }
 
 void Document::NotifyLayerManagerRecreated() {
-  EnumerateActivityObservers(NotifyActivityChanged);
+  NotifyActivityChanged();
   EnumerateSubDocuments([](Document& aSubDoc) {
     aSubDoc.NotifyLayerManagerRecreated();
     return CallState::Continue;

dom/base/Document.h

@@ -2654,11 +2654,11 @@ class Document : public nsINode,
 
   /**
    * Return true when this document is active, i.e., an active document
-   * in a content viewer.  Note that this will return true for bfcached
-   * documents, so this does NOT match the "active document" concept in
-   * the WHATWG spec - see IsCurrentActiveDocument.
+   * in a content viewer and not in the bfcache.
+   * This does NOT match the "active document" concept in the WHATWG spec -
+   * see IsCurrentActiveDocument.
    */
-  bool IsActive() const { return mDocumentContainer && !mRemovedFromDocShell; }
+  bool IsActive() const;
 
   /**
    * Return true if this is the current active document for its
@@ -2701,6 +2701,8 @@ class Document : public nsINode,
   using ActivityObserverEnumerator = FunctionRef<void(nsISupports*)>;
   void EnumerateActivityObservers(ActivityObserverEnumerator aEnumerator);
 
+  void NotifyActivityChanged();
+
   // Indicates whether mAnimationController has been (lazily) initialized.
   // If this returns true, we're promising that GetAnimationController()
   // will have a non-null return value.

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>