Bug 1191491 - Do not dispatch an audio-playback notification when swapping browsers; r=smaug
authorEhsan Akhgari <ehsan@mozilla.com>
Thu, 06 Aug 2015 10:03:24 -0400
changeset 288516 abd120a5b64087e0c8f43b596d60f269b5dff466
parent 288497 ebbd4386dbd3bfb23dc118fe1a6a63c3f51cf4c3
child 288517 5b0683070f035adf021152c7c360b4203a7ea73d
push id5067
push userraliiev@mozilla.com
push dateMon, 21 Sep 2015 14:04:52 +0000
treeherdermozilla-beta@14221ffe5b2f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1191491
milestone42.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 1191491 - Do not dispatch an audio-playback notification when swapping browsers; r=smaug We send a pagehide event during swapping docshell frame loaders, and before this patch we would not be able to differentiate this event with the one that we send when navigating away from a page, so we would incorrectly dispatch an audio-playback notification indicating that audio playback has stopped. This patch adds a flag to the root docshell when the frame loader swapping is in progress and disables the above behavior when that flag is set.
browser/base/content/test/general/browser_audioTabIcon.js
docshell/base/nsDocShell.cpp
docshell/base/nsDocShell.h
dom/base/nsFrameLoader.cpp
dom/html/HTMLMediaElement.cpp
dom/ipc/TabChild.cpp
--- a/browser/base/content/test/general/browser_audioTabIcon.js
+++ b/browser/base/content/test/general/browser_audioTabIcon.js
@@ -110,22 +110,38 @@ function* test_swapped_browser(oldTab, n
   ok(oldTab.hasAttribute("muted"), "Expected the correct muted attribute on the old tab");
   is(oldTab.hasAttribute("soundplaying"), isPlaying, "Expected the correct soundplaying attribute on the old tab");
 
   let newTab = gBrowser.getTabForBrowser(newBrowser);
   let AttrChangePromise = BrowserTestUtils.waitForEvent(newTab, "TabAttrModified", false, event => {
     return (event.detail.changed.indexOf("soundplaying") >= 0 || !isPlaying) &&
            event.detail.changed.indexOf("muted") >= 0;
   });
+  let AudioPlaybackPromise = new Promise(resolve => {
+    let observer = (subject, topic, data) => {
+      ok(false, "Should not see an audio-playback notification");
+    };
+    Services.obs.addObserver(observer, "audio-playback", false);
+    setTimeout(() => {
+      Services.obs.removeObserver(observer, "audio-playback");
+      resolve();
+    }, 100);
+  });
 
   gBrowser.swapBrowsersAndCloseOther(newTab, oldTab);
   yield AttrChangePromise;
 
   ok(newTab.hasAttribute("muted"), "Expected the correct muted attribute on the new tab");
   is(newTab.hasAttribute("soundplaying"), isPlaying, "Expected the correct soundplaying attribute on the new tab");
+
+  // Wait to see if an audio-playback event is dispatched.  This should not happen!
+  yield AudioPlaybackPromise;
+
+  ok(newTab.hasAttribute("muted"), "Expected the correct muted attribute on the new tab");
+  is(newTab.hasAttribute("soundplaying"), isPlaying, "Expected the correct soundplaying attribute on the new tab");
 }
 
 function* test_browser_swapping(tab, browser) {
   // First, test swapping with a playing but muted tab.
   yield ContentTask.spawn(browser, {}, function* () {
     let audio = content.document.querySelector("audio");
     audio.play();
   });
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -895,16 +895,17 @@ nsDocShell::nsDocShell()
   , mIsActive(true)
   , mIsPrerendered(false)
   , mIsAppTab(false)
   , mUseGlobalHistory(false)
   , mInPrivateBrowsing(false)
   , mUseRemoteTabs(false)
   , mDeviceSizeIsPageSize(false)
   , mWindowDraggingAllowed(false)
+  , mInFrameSwap(false)
   , mCanExecuteScripts(false)
   , mFiredUnloadEvent(false)
   , mEODForCurrentDocument(false)
   , mURIResultedInDocument(false)
   , mIsBeingDestroyed(false)
   , mIsExecutingOnLoadHandler(false)
   , mIsPrintingOrPP(false)
   , mSavingOldViewer(false)
@@ -14069,8 +14070,21 @@ nsDocShell::GetInheritedPaymentRequestId
 }
 
 NS_IMETHODIMP
 nsDocShell::GetPaymentRequestId(nsAString& aPaymentRequestId)
 {
   aPaymentRequestId = GetInheritedPaymentRequestId();
   return NS_OK;
 }
+
+bool
+nsDocShell::InFrameSwap()
+{
+  nsRefPtr<nsDocShell> shell = this;
+  do {
+    if (shell->mInFrameSwap) {
+      return true;
+    }
+    shell = shell->GetParentDocshell();
+  } while (shell);
+  return false;
+}
--- a/docshell/base/nsDocShell.h
+++ b/docshell/base/nsDocShell.h
@@ -253,16 +253,22 @@ public:
 
   // Notify Scroll observers when an async panning/zooming transform
   // has started being applied
   void NotifyAsyncPanZoomStarted();
   // Notify Scroll observers when an async panning/zooming transform
   // is no longer applied
   void NotifyAsyncPanZoomStopped();
 
+  void SetInFrameSwap(bool aInSwap)
+  {
+    mInFrameSwap = aInSwap;
+  }
+  bool InFrameSwap();
+
 private:
   // An observed docshell wrapper is created when recording markers is enabled.
   mozilla::UniquePtr<mozilla::ObservedDocShell> mObserved;
   bool IsObserved() const { return !!mObserved; }
 
   // It is necessary to allow adding a timeline marker wherever a docshell
   // instance is available. This operation happens frequently and needs to
   // be very fast, so instead of using a Map or having to search for some
@@ -895,16 +901,17 @@ protected:
   bool mIsActive;
   bool mIsPrerendered;
   bool mIsAppTab;
   bool mUseGlobalHistory;
   bool mInPrivateBrowsing;
   bool mUseRemoteTabs;
   bool mDeviceSizeIsPageSize;
   bool mWindowDraggingAllowed;
+  bool mInFrameSwap;
 
   // Because scriptability depends on the mAllowJavascript values of our
   // ancestors, we cache the effective scriptability and recompute it when
   // it might have changed;
   bool mCanExecuteScripts;
   void RecomputeCanExecuteScripts();
 
   // This boolean is set to true right before we fire pagehide and generally
--- a/dom/base/nsFrameLoader.cpp
+++ b/dom/base/nsFrameLoader.cpp
@@ -982,18 +982,18 @@ nsFrameLoader::SwapWithOtherLoader(nsFra
   bool equal;
   nsresult rv =
     ourContent->NodePrincipal()->Equals(otherContent->NodePrincipal(), &equal);
   if (NS_FAILED(rv) || !equal) {
     // Security problems loom.  Just bail on it all
     return NS_ERROR_DOM_SECURITY_ERR;
   }
 
-  nsCOMPtr<nsIDocShell> ourDocshell = GetExistingDocShell();
-  nsCOMPtr<nsIDocShell> otherDocshell = aOther->GetExistingDocShell();
+  nsRefPtr<nsDocShell> ourDocshell = static_cast<nsDocShell*>(GetExistingDocShell());
+  nsRefPtr<nsDocShell> otherDocshell = static_cast<nsDocShell*>(aOther->GetExistingDocShell());
   if (!ourDocshell || !otherDocshell) {
     // How odd
     return NS_ERROR_NOT_IMPLEMENTED;
   }
 
   // To avoid having to mess with session history, avoid swapping
   // frameloaders that don't correspond to root same-type docshells,
   // unless both roots have session history disabled.
@@ -1115,48 +1115,56 @@ nsFrameLoader::SwapWithOtherLoader(nsFra
       ourDocshell->GetIsApp() != otherDocshell->GetIsApp()) {
       return NS_ERROR_NOT_IMPLEMENTED;
   }
 
   if (mInSwap || aOther->mInSwap) {
     return NS_ERROR_NOT_IMPLEMENTED;
   }
   mInSwap = aOther->mInSwap = true;
+  ourDocshell->SetInFrameSwap(true);
+  otherDocshell->SetInFrameSwap(true);
 
   // Fire pageshow events on still-loading pages, and then fire pagehide
   // events.  Note that we do NOT fire these in the normal way, but just fire
   // them on the chrome event handlers.
   nsContentUtils::FirePageShowEvent(ourDocshell, ourEventTarget, false);
   nsContentUtils::FirePageShowEvent(otherDocshell, otherEventTarget, false);
   nsContentUtils::FirePageHideEvent(ourDocshell, ourEventTarget);
   nsContentUtils::FirePageHideEvent(otherDocshell, otherEventTarget);
   
   nsIFrame* ourFrame = ourContent->GetPrimaryFrame();
   nsIFrame* otherFrame = otherContent->GetPrimaryFrame();
   if (!ourFrame || !otherFrame) {
-    mInSwap = aOther->mInSwap = false;
     nsContentUtils::FirePageShowEvent(ourDocshell, ourEventTarget, true);
     nsContentUtils::FirePageShowEvent(otherDocshell, otherEventTarget, true);
+    mInSwap = aOther->mInSwap = false;
+    ourDocshell->SetInFrameSwap(false);
+    otherDocshell->SetInFrameSwap(false);
     return NS_ERROR_NOT_IMPLEMENTED;
   }
 
   nsSubDocumentFrame* ourFrameFrame = do_QueryFrame(ourFrame);
   if (!ourFrameFrame) {
-    mInSwap = aOther->mInSwap = false;
     nsContentUtils::FirePageShowEvent(ourDocshell, ourEventTarget, true);
     nsContentUtils::FirePageShowEvent(otherDocshell, otherEventTarget, true);
+    mInSwap = aOther->mInSwap = false;
+    ourDocshell->SetInFrameSwap(false);
+    otherDocshell->SetInFrameSwap(false);
     return NS_ERROR_NOT_IMPLEMENTED;
   }
 
   // OK.  First begin to swap the docshells in the two nsIFrames
   rv = ourFrameFrame->BeginSwapDocShells(otherFrame);
   if (NS_FAILED(rv)) {
-    mInSwap = aOther->mInSwap = false;
     nsContentUtils::FirePageShowEvent(ourDocshell, ourEventTarget, true);
     nsContentUtils::FirePageShowEvent(otherDocshell, otherEventTarget, true);
+    mInSwap = aOther->mInSwap = false;
+    ourDocshell->SetInFrameSwap(false);
+    otherDocshell->SetInFrameSwap(false);
     return rv;
   }
 
   // Now move the docshells to the right docshell trees.  Note that this
   // resets their treeowners to null.
   ourParentItem->RemoveChild(ourDocshell);
   otherParentItem->RemoveChild(otherDocshell);
   if (ourType == nsIDocShellTreeItem::typeContent) {
@@ -1253,16 +1261,18 @@ nsFrameLoader::SwapWithOtherLoader(nsFra
 
   ourParentDocument->FlushPendingNotifications(Flush_Layout);
   otherParentDocument->FlushPendingNotifications(Flush_Layout);
 
   nsContentUtils::FirePageShowEvent(ourDocshell, ourEventTarget, true);
   nsContentUtils::FirePageShowEvent(otherDocshell, otherEventTarget, true);
 
   mInSwap = aOther->mInSwap = false;
+  ourDocshell->SetInFrameSwap(false);
+  otherDocshell->SetInFrameSwap(false);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsFrameLoader::Destroy()
 {
   StartDestroy();
   return NS_OK;
--- a/dom/html/HTMLMediaElement.cpp
+++ b/dom/html/HTMLMediaElement.cpp
@@ -98,16 +98,17 @@ static PRLogModuleInfo* gMediaElementEve
 
 #include "nsIContentSecurityPolicy.h"
 
 #include "mozilla/Preferences.h"
 #include "mozilla/FloatingPoint.h"
 
 #include "nsIPermissionManager.h"
 #include "nsContentTypeParser.h"
+#include "nsDocShell.h"
 
 #include "mozilla/EventStateManager.h"
 
 #if defined(MOZ_B2G) && !defined(MOZ_GRAPHENE)
 // This controls the b2g specific of pausing the media element when the
 // AudioChannel tells us to mute it.
 #define PAUSE_MEDIA_ELEMENT_FROM_AUDIOCHANNEL
 #endif
@@ -4008,17 +4009,24 @@ bool HTMLMediaElement::IsBeingDestroyed(
 }
 
 void HTMLMediaElement::NotifyOwnerDocumentActivityChanged()
 {
   bool pauseElement = NotifyOwnerDocumentActivityChangedInternal();
   if (pauseElement && mAudioChannelAgent) {
     // If the element is being paused since we are navigating away from the
     // document, notify the audio channel agent.
-    NotifyAudioChannelAgent(false);
+    // Be careful to ignore this event during a docshell frame swap.
+    auto docShell = static_cast<nsDocShell*>(OwnerDoc()->GetDocShell());
+    if (!docShell) {
+      return;
+    }
+    if (!docShell->InFrameSwap()) {
+      NotifyAudioChannelAgent(false);
+    }
   }
 }
 
 bool
 HTMLMediaElement::NotifyOwnerDocumentActivityChangedInternal()
 {
   nsIDocument* ownerDoc = OwnerDoc();
   if (mDecoder && !IsBeingDestroyed()) {
--- a/dom/ipc/TabChild.cpp
+++ b/dom/ipc/TabChild.cpp
@@ -2387,21 +2387,28 @@ TabChild::RecvSwappedWithOtherRemoteLoad
     return true;
   }
 
   nsCOMPtr<nsPIDOMWindow> ourWindow = ourDocShell->GetWindow();
   if (NS_WARN_IF(!ourWindow)) {
     return true;
   }
 
+  nsRefPtr<nsDocShell> docShell = static_cast<nsDocShell*>(ourDocShell.get());
+
   nsCOMPtr<EventTarget> ourEventTarget = ourWindow->GetParentTarget();
 
+  docShell->SetInFrameSwap(true);
+
   nsContentUtils::FirePageShowEvent(ourDocShell, ourEventTarget, false);
   nsContentUtils::FirePageHideEvent(ourDocShell, ourEventTarget);
   nsContentUtils::FirePageShowEvent(ourDocShell, ourEventTarget, true);
+
+  docShell->SetInFrameSwap(false);
+
   return true;
 }
 
 bool
 TabChild::RecvDestroy()
 {
   MOZ_ASSERT(mDestroyed == false);
   mDestroyed = true;