Bug 1346880 - part7 : further improve the readability for wakelock test. r=padenot
authoralwu <alwu@mozilla.com>
Tue, 27 Oct 2020 12:32:03 +0000
changeset 554681 62fe8bb937c244cd0ae18275e9009272482b86d3
parent 554680 4d9b9f483e03846d0b4d9bbbbb3d68a7599af7bf
child 554682 c938f6fd4656bdee7c277535a626654f1b32937a
push id37898
push userabutkovits@mozilla.com
push dateWed, 28 Oct 2020 09:24:21 +0000
treeherdermozilla-central@83bf4fd3b1fb [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspadenot
bugs1346880
milestone84.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 1346880 - part7 : further improve the readability for wakelock test. r=padenot This patch would further improve the readability for wakelock test by doing following things. 1. refactor `getWakeLockState()` and rename it to `waitForExpectedWakeLockState()` In the origin test, we have to create a wakelock sentinel object beforehand, and then call `check()` later, which is not really intuitive. Refactor that function to allow us to use it easier. 2. use const variables to store the wakelock topic (audio-playing/video-playing) 3. add a helper function `ensureNeverAcquireVideoWakelock()` to clearly indicate we won't acquire video wakelock for web audio Differential Revision: https://phabricator.services.mozilla.com/D94778
toolkit/content/tests/browser/browser_media_wakelock.js
toolkit/content/tests/browser/browser_media_wakelock_webaudio.js
toolkit/content/tests/browser/head.js
--- a/toolkit/content/tests/browser/browser_media_wakelock.js
+++ b/toolkit/content/tests/browser/browser_media_wakelock.js
@@ -10,16 +10,18 @@
 // Import this in order to use `triggerPictureInPicture()`.
 /* import-globals-from ../../../../toolkit/components/pictureinpicture/tests/head.js */
 Services.scriptloader.loadSubScript(
   "chrome://mochitests/content/browser/toolkit/components/pictureinpicture/tests/head.js",
   this
 );
 
 const LOCATION = "https://example.com/browser/toolkit/content/tests/browser/";
+const AUDIO_WAKELOCK_NAME = "audio-playing";
+const VIDEO_WAKELOCK_NAME = "video-playing";
 
 add_task(async function testCheckWakelockWhenChangeTabVisibility() {
   await checkWakelockWhenChangeTabVisibility({
     description: "playing video",
     url: "file_video.html",
     lockAudio: true,
     lockVideo: true,
   });
@@ -95,44 +97,48 @@ async function checkWakelockWhenChangeTa
   const originalTab = gBrowser.selectedTab;
   info(`start a new tab for '${description}'`);
   const mediaTab = await BrowserTestUtils.openNewForegroundTab(
     window.gBrowser,
     LOCATION + url
   );
 
   info(`wait for media starting playing`);
-  let audioWakeLock = getWakeLockState("audio-playing", lockAudio, true);
-  let videoWakeLock = getWakeLockState("video-playing", lockVideo, true);
   await waitUntilVideoStarted(mediaTab, additionalParams);
-  await audioWakeLock.check();
-  await videoWakeLock.check();
+  await waitForExpectedWakeLockState(AUDIO_WAKELOCK_NAME, {
+    needLock: lockAudio,
+    isForegroundLock: true,
+  });
+  await waitForExpectedWakeLockState(VIDEO_WAKELOCK_NAME, {
+    needLock: lockVideo,
+    isForegroundLock: true,
+  });
 
   info(`switch media tab to background`);
+  await BrowserTestUtils.switchTab(window.gBrowser, originalTab);
   const isPageConsideredAsForeground = !!additionalParams?.elementIdForEnteringPIPMode;
-  audioWakeLock = getWakeLockState(
-    "audio-playing",
-    lockAudio,
-    isPageConsideredAsForeground
-  );
-  videoWakeLock = getWakeLockState(
-    "video-playing",
-    lockVideo,
-    isPageConsideredAsForeground
-  );
-  await BrowserTestUtils.switchTab(window.gBrowser, originalTab);
-  await audioWakeLock.check();
-  await videoWakeLock.check();
+  await waitForExpectedWakeLockState(AUDIO_WAKELOCK_NAME, {
+    needLock: lockAudio,
+    isForegroundLock: isPageConsideredAsForeground,
+  });
+  await waitForExpectedWakeLockState(VIDEO_WAKELOCK_NAME, {
+    needLock: lockVideo,
+    isForegroundLock: isPageConsideredAsForeground,
+  });
 
   info(`switch media tab to foreground again`);
-  audioWakeLock = getWakeLockState("audio-playing", lockAudio, true);
-  videoWakeLock = getWakeLockState("video-playing", lockVideo, true);
   await BrowserTestUtils.switchTab(window.gBrowser, mediaTab);
-  await audioWakeLock.check();
-  await videoWakeLock.check();
+  await waitForExpectedWakeLockState(AUDIO_WAKELOCK_NAME, {
+    needLock: lockAudio,
+    isForegroundLock: true,
+  });
+  await waitForExpectedWakeLockState(VIDEO_WAKELOCK_NAME, {
+    needLock: lockVideo,
+    isForegroundLock: true,
+  });
 
   info(`remove tab`);
   if (mediaTab.PIPWindow) {
     await BrowserTestUtils.closeWindow(mediaTab.PIPWindow);
   }
   BrowserTestUtils.removeTab(mediaTab);
 }
 
--- a/toolkit/content/tests/browser/browser_media_wakelock_webaudio.js
+++ b/toolkit/content/tests/browser/browser_media_wakelock_webaudio.js
@@ -1,88 +1,96 @@
 /**
  * Test if wakelock can be required correctly when we play web audio. The
  * wakelock should only be required when web audio is audible.
  */
-add_task(async function testCheckWakelockWhenChangeTabVisibility() {
+
+const AUDIO_WAKELOCK_NAME = "audio-playing";
+const VIDEO_WAKELOCK_NAME = "video-playing";
+
+add_task(async function testCheckAudioWakelockWhenChangeTabVisibility() {
   await checkWakelockWhenChangeTabVisibility({
     description: "playing audible web audio",
-    lockAudio: true,
-    lockVideo: false,
+    needLock: true,
   });
   await checkWakelockWhenChangeTabVisibility({
     description: "suspended web audio",
     additionalParams: {
       suspend: true,
     },
-    lockAudio: false,
-    lockVideo: false,
+    needLock: false,
   });
 });
 
 add_task(
   async function testBrieflyAudibleAudioContextReleasesAudioWakeLockWhenInaudible() {
     const tab = await BrowserTestUtils.openNewForegroundTab(
       window.gBrowser,
       "about:blank"
     );
 
-    let audioWakeLock = getWakeLockState("audio-playing", true, true);
-    let videoWakeLock = getWakeLockState("video-playing", false, true);
+    info(`make a short noise on web audio`);
+    await Promise.all([
+      // As the sound would only happen for a really short period, calling
+      // checking wakelock first helps to ensure that we won't miss that moment.
+      waitForExpectedWakeLockState(AUDIO_WAKELOCK_NAME, {
+        needLock: true,
+        isForegroundLock: true,
+      }),
+      createWebAudioDocument(tab, { stopTimeOffset: 0.1 }),
+    ]);
+    await ensureNeverAcquireVideoWakelock();
 
-    info(`make a short noise on web audio`);
-    await createWebAudioDocument(tab, { stopTimeOffset: 0.1 });
-    await audioWakeLock.check();
-    await videoWakeLock.check();
-
-    audioWakeLock = getWakeLockState("audio-playing", false, true);
-    videoWakeLock = getWakeLockState("video-playing", false, true);
-
-    await audioWakeLock.check();
-    await videoWakeLock.check();
+    info(`audio wakelock should be released after web audio becomes silent`);
+    await waitForExpectedWakeLockState(AUDIO_WAKELOCK_NAME, false, {
+      needLock: false,
+    });
+    await ensureNeverAcquireVideoWakelock();
 
     await BrowserTestUtils.removeTab(tab);
   }
 );
 
 /**
  * Following are helper functions.
  */
 async function checkWakelockWhenChangeTabVisibility({
   description,
   additionalParams,
-  lockAudio,
-  lockVideo,
+  needLock,
   elementIdForEnteringPIPMode,
 }) {
   const originalTab = gBrowser.selectedTab;
   info(`start a new tab for '${description}'`);
   const mediaTab = await BrowserTestUtils.openNewForegroundTab(
     window.gBrowser,
     "about:blank"
   );
-  let audioWakeLock = getWakeLockState("audio-playing", lockAudio, true);
-  let videoWakeLock = getWakeLockState("video-playing", lockVideo, true);
   await createWebAudioDocument(mediaTab, additionalParams);
-  await audioWakeLock.check();
-  await videoWakeLock.check();
+  await waitForExpectedWakeLockState(AUDIO_WAKELOCK_NAME, {
+    needLock,
+    isForegroundLock: true,
+  });
+  await ensureNeverAcquireVideoWakelock();
 
   info(`switch media tab to background`);
-  audioWakeLock = getWakeLockState("audio-playing", lockAudio, false);
-  videoWakeLock = getWakeLockState("video-playing", lockVideo, false);
   await BrowserTestUtils.switchTab(window.gBrowser, originalTab);
-  await audioWakeLock.check();
-  await videoWakeLock.check();
+  await waitForExpectedWakeLockState(AUDIO_WAKELOCK_NAME, {
+    needLock,
+    isForegroundLock: false,
+  });
+  await ensureNeverAcquireVideoWakelock();
 
   info(`switch media tab to foreground again`);
-  audioWakeLock = getWakeLockState("audio-playing", lockAudio, true);
-  videoWakeLock = getWakeLockState("video-playing", lockVideo, true);
   await BrowserTestUtils.switchTab(window.gBrowser, mediaTab);
-  await audioWakeLock.check();
-  await videoWakeLock.check();
+  await waitForExpectedWakeLockState(AUDIO_WAKELOCK_NAME, {
+    needLock,
+    isForegroundLock: true,
+  });
+  await ensureNeverAcquireVideoWakelock();
 
   info(`remove media tab`);
   BrowserTestUtils.removeTab(mediaTab);
 }
 
 function createWebAudioDocument(tab, { stopTimeOffset, suspend } = {}) {
   return SpecialPowers.spawn(
     tab.linkedBrowser,
@@ -107,8 +115,13 @@ function createWebAudioDocument(tab, { s
           info(`wait until AudioContext starts running`);
           await new Promise(r => (ac.onstatechange = r));
         }
         info("AudioContext is running");
       }
     }
   );
 }
+
+function ensureNeverAcquireVideoWakelock() {
+  // Web audio won't play any video, we never need video wakelock.
+  return waitForExpectedWakeLockState(VIDEO_WAKELOCK_NAME, { needLock: false });
+}
--- a/toolkit/content/tests/browser/head.js
+++ b/toolkit/content/tests/browser/head.js
@@ -356,56 +356,51 @@ function checkVideoDidPlay(browser, args
         "be able to autoplay"
     );
     video.src = "";
     content.document.body.remove(video);
   });
 }
 
 /**
- * Return a wakelock observer that has a `check()` method that allows us to wait
- * until the wakelock state changes to our expected state.
+ * check if current wakelock is equal to expected state, if not, then wait until
+ * the wakelock changes its state to expected state.
  * @param needLock
  *        the wakolock should be locked or not
- * @param isTabInForeground
- *        the wakelock should be in the foreground or not
+ * @param isForegroundLock
+ *        when the lock is on, the wakelock should be in the foreground or not
  */
-function getWakeLockState(topic, needLock, isTabInForeground) {
+async function waitForExpectedWakeLockState(
+  topic,
+  { needLock, isForegroundLock }
+) {
   const powerManagerService = Cc["@mozilla.org/power/powermanagerservice;1"];
   const powerManager = powerManagerService.getService(
     Ci.nsIPowerManagerService
   );
-  const tabState = isTabInForeground ? "foreground" : "background";
-  return {
-    check: async () => {
-      if (needLock) {
-        const expectedLockState = `locked-${tabState}`;
-        if (powerManager.getWakeLockState(topic) != expectedLockState) {
-          await wakeLockObserved(
-            powerManager,
-            topic,
-            state => state == expectedLockState
-          );
-        }
-        ok(true, `requested '${topic}' wakelock in ${tabState}`);
-      } else {
-        if (powerManager.getWakeLockState(topic) != "unlocked") {
-          await wakeLockObserved(
-            powerManager,
-            topic,
-            state => state == "unlocked"
-          );
-        }
-        ok(
-          powerManager.getWakeLockState(topic) == "unlocked",
-          `doesn't request lock for '${topic}'`
-        );
-      }
-    },
-  };
+  const wakelockState = powerManager.getWakeLockState(topic);
+  let expectedLockState = "unlocked";
+  if (needLock) {
+    expectedLockState = isForegroundLock
+      ? "locked-foreground"
+      : "locked-background";
+  }
+  if (wakelockState != expectedLockState) {
+    info(`wait until wakelock becomes ${expectedLockState}`);
+    await wakeLockObserved(
+      powerManager,
+      topic,
+      state => state == expectedLockState
+    );
+  }
+  is(
+    powerManager.getWakeLockState(topic),
+    expectedLockState,
+    `the wakelock state for '${topic}' is equal to '${expectedLockState}'`
+  );
 }
 
 function wakeLockObserved(powerManager, observeTopic, checkFn) {
   return new Promise(resolve => {
     function wakeLockListener() {}
     wakeLockListener.prototype = {
       QueryInterface: ChromeUtils.generateQI(["nsIDOMMozWakeLockListener"]),
       callback(topic, state) {