Backed out 3 changesets (bug 1519430) for mda failures at test_autoplay_policy_web_audio_notResumePageInvokedSuspendedAudioContext.html
authorCoroiu Cristina <ccoroiu@mozilla.com>
Thu, 17 Jan 2019 09:45:15 +0200
changeset 454243 eff7fd1e573cc0682e3080ee2370eac840e57b9d
parent 454242 59696b74a507d82a85b8ca318cc9a100554038a2
child 454244 00614ec6e765206c325364b51b8810fddf29234d
push id35390
push usershindli@mozilla.com
push dateThu, 17 Jan 2019 16:13:20 +0000
treeherdermozilla-central@f2f4a6eb1576 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
bugs1519430
milestone66.0a1
backs outb7bc38b0f9c5614fb4fd5edf7e4686c9b3b0d179
987aa2594ba62b25ca9a36ae51e8c4d7afb0939e
521176ad7ae8c205307c4447c70a4de546c6cac8
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
Backed out 3 changesets (bug 1519430) for mda failures at test_autoplay_policy_web_audio_notResumePageInvokedSuspendedAudioContext.html Backed out changeset b7bc38b0f9c5 (bug 1519430) Backed out changeset 987aa2594ba6 (bug 1519430) Backed out changeset 521176ad7ae8 (bug 1519430)
dom/base/nsGlobalWindowInner.cpp
dom/media/test/mochitest.ini
dom/media/test/test_autoplay_policy_web_audio_notResumePageInvokedSuspendedAudioContext.html
dom/media/webaudio/AudioContext.cpp
dom/media/webaudio/AudioContext.h
--- a/dom/base/nsGlobalWindowInner.cpp
+++ b/dom/base/nsGlobalWindowInner.cpp
@@ -5396,17 +5396,18 @@ void nsGlobalWindowInner::Suspend() {
   }
 
   SuspendIdleRequests();
 
   mTimeoutManager->Suspend();
 
   // Suspend all of the AudioContexts for this window
   for (uint32_t i = 0; i < mAudioContexts.Length(); ++i) {
-    mAudioContexts[i]->SuspendFromChrome();
+    ErrorResult dummy;
+    RefPtr<Promise> d = mAudioContexts[i]->Suspend(dummy);
   }
 }
 
 void nsGlobalWindowInner::Resume() {
   MOZ_ASSERT(NS_IsMainThread());
 
   // We can only safely resume a window if its the current inner window.  If
   // its not the current inner, then we are in one of two different cases.
@@ -5437,17 +5438,18 @@ void nsGlobalWindowInner::Resume() {
     for (uint32_t i = 0; i < mEnabledSensors.Length(); i++)
       ac->AddWindowListener(mEnabledSensors[i], this);
   }
   EnableGamepadUpdates();
   EnableVRUpdates();
 
   // Resume all of the AudioContexts for this window
   for (uint32_t i = 0; i < mAudioContexts.Length(); ++i) {
-    mAudioContexts[i]->ResumeFromChrome();
+    ErrorResult dummy;
+    RefPtr<Promise> d = mAudioContexts[i]->Resume(dummy);
   }
 
   mTimeoutManager->Resume();
 
   ResumeIdleRequests();
 
   // Resume all of the workers for this window.  We must do this
   // after timeouts since workers may have queued events that can trigger
--- a/dom/media/test/mochitest.ini
+++ b/dom/media/test/mochitest.ini
@@ -719,18 +719,16 @@ skip-if = android_version >= '23' # bug 
 [test_autoplay_policy_key_blacklist.html]
 skip-if = android_version >= '23' || (verify && debug && (os == 'win')) # bug 1424903
 [test_autoplay_policy_unmute_pauses.html]
 skip-if = android_version >= '23' # bug 1424903
 [test_autoplay_policy_play_before_loadedmetadata.html]
 skip-if = android_version >= '23' # bug 1424903
 [test_autoplay_policy_permission.html]
 skip-if = android_version >= '23' # bug 1424903
-[test_autoplay_policy_web_audio_notResumePageInvokedSuspendedAudioContext.html]
-skip-if = android_version >= '23' # bug 1424903
 [test_autoplay_policy_web_audio_mediaElementAudioSourceNode.html]
 skip-if = android_version >= '23' # bug 1424903
 [test_buffered.html]
 skip-if = android_version == '22' # bug 1308388, android(bug 1232305)
 [test_bug448534.html]
 [test_bug463162.xhtml]
 [test_bug465498.html]
 skip-if = toolkit == 'android' # android(bug 1232305)
deleted file mode 100644
--- a/dom/media/test/test_autoplay_policy_web_audio_notResumePageInvokedSuspendedAudioContext.html
+++ /dev/null
@@ -1,95 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<head>
-  <title>Autoplay policy test : do not resume AudioContext which is suspended by page</title>
-  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
-  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
-  <script type="text/javascript" src="manifest.js"></script>
-</head>
-<body>
-<script>
-
-/**
- * This test is used to ensure we won't resume AudioContext which is suspended
- * by page (it means calling suspend() explicitly) when calling
- * `AudioScheduledSourceNode.start()`.
- */
-
-SimpleTest.waitForExplicitFinish();
-
-(async function testNotResumeUserInvokedSuspendedAudioContext() {
-  await setupTestPreferences();
-
-  const nodeTypes = ["AudioBufferSourceNode", "ConstantSourceNode", "OscillatorNode"];
-  for (let nodeType of nodeTypes) {
-    info(`- create an audio context which should not be allowed to start, it's allowed to be created, but it's forbidden to start -`);
-    await createAudioContext();
-
-    info(`- explicitly suspend the AudioContext in the page -`);
-    suspendAudioContext();
-
-    info(`- start an 'AudioScheduledSourceNode', and check that the AudioContext does not start, because it has been explicitly suspended -`);
-    await createAndStartAudioScheduledSourceNode(nodeType);
-  }
-
-  SimpleTest.finish();
-})();
-
-/**
- * Test utility functions
- */
-
-function setupTestPreferences() {
-  return SpecialPowers.pushPrefEnv({"set": [
-    ["media.autoplay.default", SpecialPowers.Ci.nsIAutoplay.BLOCKED],
-    ["media.autoplay.enabled.user-gestures-needed", true],
-    ["media.autoplay.block-webaudio", true],
-    ["media.autoplay.block-event.enabled", true],
-  ]});
-}
-
-async function createAudioContext() {
-  window.ac = new AudioContext();
-  await once(ac, "blocked");
-  is(ac.state, "suspended", `AudioContext is blocked.`);
-}
-
-function suspendAudioContext() {
-  try {
-    ac.suspend();
-  } catch(e) {
-    ok(false, `AudioContext suspend failed!`);
-  }
-}
-
-async function createAndStartAudioScheduledSourceNode(nodeType) {
-  let node;
-  info(`- create ${nodeType} -`);
-  switch (nodeType) {
-    case "AudioBufferSourceNode":
-      node = ac.createBufferSource();
-      break;
-    case "ConstantSourceNode":
-      node = ac.createConstantSource();
-      break;
-    case "OscillatorNode":
-      node = ac.createOscillator();
-      break;
-    default:
-      ok(false, "undefined AudioScheduledSourceNode type");
-      return;
-  }
-  node.connect(ac.destination);
-
-  // activate the document in order to allow autoplay.
-  SpecialPowers.wrap(document).notifyUserGestureActivation();
-  node.start();
-
-  await once(ac, "blocked");
-  is(ac.state, "suspended", `AudioContext should not be resumed.`);
-
-  // reset the activation flag of the document in order not to interfere next test.
-  SpecialPowers.wrap(document).clearUserGestureActivation();
-}
-
-</script>
--- a/dom/media/webaudio/AudioContext.cpp
+++ b/dom/media/webaudio/AudioContext.cpp
@@ -148,17 +148,16 @@ AudioContext::AudioContext(nsPIDOMWindow
       mNumberOfChannels(aNumberOfChannels),
       mIsOffline(aIsOffline),
       mIsStarted(!aIsOffline),
       mIsShutDown(false),
       mCloseCalled(false),
       mSuspendCalled(false),
       mIsDisconnecting(false),
       mWasAllowedToStart(true),
-      mSuspendedByContent(false),
       mWasEverAllowedToStart(false),
       mWasEverBlockedToStart(false),
       mWouldBeAllowedToStart(true) {
   bool mute = aWindow->AddAudioContext(this);
 
   // Note: AudioDestinationNode needs an AudioContext that must already be
   // bound to the window.
   const bool allowedToStart = AutoplayPolicy::IsAllowedToPlay(*this);
@@ -189,21 +188,17 @@ void AudioContext::StartBlockedAudioCont
   // Only try to start AudioContext when AudioContext was not allowed to start.
   if (mWasAllowedToStart) {
     return;
   }
 
   const bool isAllowedToPlay = AutoplayPolicy::IsAllowedToPlay(*this);
   AUTOPLAY_LOG("Trying to start AudioContext %p, IsAllowedToPlay=%d", this,
                isAllowedToPlay);
-
-  // Only start the AudioContext if this resume() call was initiated by content,
-  // not if it was a result of the AudioContext starting after having been
-  // blocked because of the auto-play policy.
-  if (isAllowedToPlay && !mSuspendedByContent) {
+  if (isAllowedToPlay) {
     ResumeInternal();
   } else {
     ReportBlocked();
   }
 }
 
 nsresult AudioContext::Init() {
   if (!mIsOffline) {
@@ -902,32 +897,21 @@ already_AddRefed<Promise> AudioContext::
     return promise.forget();
   }
 
   if (mAudioContextState == AudioContextState::Closed || mCloseCalled) {
     promise->MaybeReject(NS_ERROR_DOM_INVALID_STATE_ERR);
     return promise.forget();
   }
 
-  mSuspendedByContent = true;
   mPromiseGripArray.AppendElement(promise);
   SuspendInternal(promise);
   return promise.forget();
 }
 
-void AudioContext::SuspendFromChrome() {
-  // Not support suspend call for these situations.
-  if (mAudioContextState == AudioContextState::Suspended ||
-      mIsOffline ||
-      (mAudioContextState == AudioContextState::Closed || mCloseCalled)) {
-    return;
-  }
-  SuspendInternal(nullptr);
-}
-
 void AudioContext::SuspendInternal(void* aPromise) {
   Destination()->Suspend();
 
   nsTArray<MediaStream*> streams;
   // If mSuspendCalled is true then we already suspended all our streams,
   // so don't suspend them again (since suspend(); suspend(); resume(); should
   // cancel both suspends). But we still need to do ApplyAudioContextOperation
   // to ensure our new promise is resolved.
@@ -935,26 +919,16 @@ void AudioContext::SuspendInternal(void*
     streams = GetAllStreams();
   }
   Graph()->ApplyAudioContextOperation(DestinationStream(), streams,
                                       AudioContextOperation::Suspend, aPromise);
 
   mSuspendCalled = true;
 }
 
-void AudioContext::ResumeFromChrome() {
-  // Not support resume call for these situations.
-  if (mAudioContextState == AudioContextState::Running ||
-      mIsOffline ||
-      (mAudioContextState == AudioContextState::Closed || mCloseCalled)) {
-    return;
-  }
-  ResumeInternal();
-}
-
 already_AddRefed<Promise> AudioContext::Resume(ErrorResult& aRv) {
   nsCOMPtr<nsIGlobalObject> parentObject = do_QueryInterface(GetParentObject());
   RefPtr<Promise> promise;
   promise = Promise::Create(parentObject, aRv);
   if (aRv.Failed()) {
     return nullptr;
   }
 
@@ -963,17 +937,16 @@ already_AddRefed<Promise> AudioContext::
     return promise.forget();
   }
 
   if (mAudioContextState == AudioContextState::Closed || mCloseCalled) {
     promise->MaybeReject(NS_ERROR_DOM_INVALID_STATE_ERR);
     return promise.forget();
   }
 
-  mSuspendedByContent = false;
   mPendingResumePromises.AppendElement(promise);
 
   const bool isAllowedToPlay = AutoplayPolicy::IsAllowedToPlay(*this);
   AUTOPLAY_LOG("Trying to resume AudioContext %p, IsAllowedToPlay=%d", this,
                isAllowedToPlay);
   if (isAllowedToPlay) {
     ResumeInternal();
   } else {
--- a/dom/media/webaudio/AudioContext.h
+++ b/dom/media/webaudio/AudioContext.h
@@ -196,22 +196,16 @@ class AudioContext final : public DOMEve
   // When back on the main thread, we can resolve or reject the promise, by
   // casting it back to a `Promise*` while asserting we're back on the main
   // thread and removing the reference we added.
   already_AddRefed<Promise> Suspend(ErrorResult& aRv);
   already_AddRefed<Promise> Resume(ErrorResult& aRv);
   already_AddRefed<Promise> Close(ErrorResult& aRv);
   IMPL_EVENT_HANDLER(statechange)
 
-  // These two functions are similar with Suspend() and Resume(), the difference
-  // is they are designed for calling from chrome side, not content side. eg.
-  // calling from inner window, so we won't need to return promise for caller.
-  void SuspendFromChrome();
-  void ResumeFromChrome();
-
   already_AddRefed<AudioBufferSourceNode> CreateBufferSource(ErrorResult& aRv);
 
   already_AddRefed<ConstantSourceNode> CreateConstantSource(ErrorResult& aRv);
 
   already_AddRefed<AudioBuffer> CreateBuffer(uint32_t aNumberOfChannels,
                                              uint32_t aLength,
                                              float aSampleRate,
                                              ErrorResult& aRv);
@@ -381,19 +375,16 @@ class AudioContext final : public DOMEve
   // Close has been called, reject suspend and resume call.
   bool mCloseCalled;
   // Suspend has been called with no following resume.
   bool mSuspendCalled;
   bool mIsDisconnecting;
   // This flag stores the value of previous status of `allowed-to-start`.
   bool mWasAllowedToStart;
 
-  // True if this AudioContext has been suspended by the page.
-  bool mSuspendedByContent;
-
   // These variables are used for telemetry, they're not reflect the actual
   // status of AudioContext, they are based on the "assumption" of enabling
   // blocking web audio. Because we want to record Telemetry no matter user
   // enable blocking autoplay or not.
   // - 'mWasEverAllowedToStart' would be true when AudioContext had ever been
   //   allowed to start if we enable blocking web audio.
   // - 'mWasEverBlockedToStart' would be true when AudioContext had ever been
   //   blocked to start if we enable blocking web audio.