Bug 1874406 - Refactor, optimize and cleanup event page idle management r=robwu

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

toolkit/components/extensions/ExtensionParent.sys.mjs

@@ -1206,7 +1206,7 @@ ParentAPIManager = {
         !context.extension.persistentBackground
       ) {
         context.extension.emit("background-script-reset-idle", {
-          reason: "parentApiCall",
+          reason: "parentapicall",
           path: data.path,
         });
       }

toolkit/components/extensions/parent/ext-backgroundPage.js

@@ -497,8 +497,8 @@ class BackgroundContextOwner {
       EventManager.clearPrimedListeners(this.extension, false);
     }
 
-    // Ensure there is no backgroundTimer running
+    // Ensure any idle background timer is not running.
-    this.backgroundBuilder.clearIdleTimer();
+    this.backgroundBuilder.idleManager.clearTimer();
 
     const bgInstance = this.bgInstance;
     if (bgInstance) {
@@ -664,6 +664,7 @@ class BackgroundBuilder {
   constructor(extension) {
     this.extension = extension;
     this.backgroundContextOwner = new BackgroundContextOwner(this, extension);
+    this.idleManager = new IdleManager(extension);
   }
 
   async build() {
@@ -718,26 +719,6 @@ class BackgroundBuilder {
     }
   }
 
-  observe(subject, topic) {
-    if (topic == "timer-callback") {
-      let { extension } = this;
-      this.clearIdleTimer();
-      extension?.terminateBackground();
-    }
-  }
-
-  clearIdleTimer() {
-    this.backgroundTimer?.cancel();
-    this.backgroundTimer = null;
-  }
-
-  resetIdleTimer() {
-    this.clearIdleTimer();
-    let timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
-    timer.init(this, backgroundIdleTimeout, Ci.nsITimer.TYPE_ONE_SHOT);
-    this.backgroundTimer = timer;
-  }
-
   primeBackground(isInStartup = true) {
     let { extension } = this;
 
@@ -779,79 +760,45 @@ class BackgroundBuilder {
       return bgStartupPromise;
     };
 
-    let resetBackgroundIdle = (eventName, resetIdleDetails) => {
+    let resetBackgroundIdle = (event, { reason }) => {
-      this.clearIdleTimer();
-      if (!this.extension || extension.persistentBackground) {
-        // Extension was already shut down or is persistent and
-        // does not idle timout.
-        return;
-      }
-      // TODO remove at an appropriate point in the future prior
-      // to general availability.  There may be some racy conditions
-      // with idle timeout between an event starting and the event firing
-      // but we still want testing with an idle timeout.
-      if (
-        !Services.prefs.getBoolPref("extensions.background.idle.enabled", true)
-      ) {
-        return;
-      }
-
       if (
         extension.backgroundState == BACKGROUND_STATE.SUSPENDING &&
         // After we begin suspending the background, parent API calls from
         // runtime.onSuspend listeners shouldn't cancel the suspension.
-        resetIdleDetails?.reason !== "parentApiCall"
+        reason !== "parentapicall"
       ) {
         extension.backgroundState = BACKGROUND_STATE.RUNNING;
-        // call runtime.onSuspendCanceled
         extension.emit("background-script-suspend-canceled");
       }
 
-      this.resetIdleTimer();
+      this.idleManager.resetTimer();
 
-      if (
+      if (this.isWorker) {
-        eventName === "background-script-reset-idle" &&
+        // TODO(Bug 1790087): record similar telemetry for service workers.
-        // TODO(Bug 1790087): record similar telemetry for background service worker.
+        return;
-        !this.isWorker
-      ) {
-        // Record the reason for resetting the event page idle timeout
-        // in a idle result histogram, with the category set based
-        // on the reason for resetting (defaults to 'reset_other'
-        // if resetIdleDetails.reason is missing or not mapped into the
-        // telemetry histogram categories).
-        //
-        // Keep this in sync with the categories listed in Histograms.json
-        // for "WEBEXT_EVENTPAGE_IDLE_RESULT_COUNT".
-        let category = "reset_other";
-        switch (resetIdleDetails?.reason) {
-          case "event":
-            category = "reset_event";
-            return; // not break; because too frequent, see bug 1868960.
-          case "hasActiveNativeAppPorts":
-            category = "reset_nativeapp";
-            break;
-          case "hasActiveStreamFilter":
-            category = "reset_streamfilter";
-            break;
-          case "pendingListeners":
-            category = "reset_listeners";
-            break;
-          case "parentApiCall":
-            category = "reset_parentapicall";
-            return; // not break; because too frequent, see bug 1868960.
-        }
-
-        ExtensionTelemetry.eventPageIdleResult.histogramAdd({
-          extension,
-          category,
-        });
       }
+      if (reason === "event" || reason === "parentapicall") {
+        // Bug 1868960: not recording these because too frequent.
+        return;
+      }
+
+      // Keep in sync with categories in WEBEXT_EVENTPAGE_IDLE_RESULT_COUNT.
+      let KNOWN = ["nativeapp", "streamfilter", "listeners"];
+      ExtensionTelemetry.eventPageIdleResult.histogramAdd({
+        extension,
+        category: `reset_${KNOWN.includes(reason) ? reason : "other"}`,
+      });
     };
 
-    // Listen for events from the EventManager
+    if (!extension.persistentBackground) {
-    extension.on("background-script-reset-idle", resetBackgroundIdle);
+      // Listen for events from the EventManager
-    // After the background is started, initiate the first timer
+      extension.on("background-script-reset-idle", resetBackgroundIdle);
-    extension.once("background-script-started", resetBackgroundIdle);
+
+      // After the background is started, initiate the first timer
+      extension.once("background-script-started", () => {
+        this.idleManager.resetTimer();
+      });
+    }
 
     // TODO bug 1844488: terminateBackground should account for externally
     // triggered background restarts. It does currently performs various
@@ -889,9 +836,7 @@ class BackgroundBuilder {
         !disableResetIdleForTest &&
         extension.backgroundContext?.hasActiveNativeAppPorts
       ) {
-        extension.emit("background-script-reset-idle", {
+        extension.emit("background-script-reset-idle", { reason: "nativeapp" });
-          reason: "hasActiveNativeAppPorts",
-        });
         return;
       }
 
@@ -900,7 +845,7 @@ class BackgroundBuilder {
         extension.backgroundContext?.pendingRunListenerPromisesCount
       ) {
         extension.emit("background-script-reset-idle", {
-          reason: "pendingListeners",
+          reason: "listeners",
           pendingListeners:
             extension.backgroundContext.pendingRunListenerPromisesCount,
         });
@@ -941,7 +886,7 @@ class BackgroundBuilder {
           });
         if (!disableResetIdleForTest && hasActiveStreamFilter) {
           extension.emit("background-script-reset-idle", {
-            reason: "hasActiveStreamFilter",
+            reason: "streamfilter",
           });
           return;
         }
@@ -956,7 +901,7 @@ class BackgroundBuilder {
       }
 
       extension.backgroundState = BACKGROUND_STATE.SUSPENDING;
-      this.clearIdleTimer();
+      this.idleManager.clearTimer();
       // call runtime.onSuspend
       await extension.emit("background-script-suspend");
       // If in the meantime another event fired, state will be RUNNING,
@@ -1027,6 +972,53 @@ class BackgroundBuilder {
   }
 }
 
+/**
+ * Times the suspension of the background page, acts like a 3-state machine:
+ *  - suspended (or uninitialized)
+ *  - waiting for a timeout (now() < sleepTime)
+ *  - TODO: waiting on a promise (keepAwake.size > 0)
+ */
+var IdleManager = class IdleManager {
+  sleepTime = 0;
+  /** @type {nsITimer} */
+  timer = null;
+  /** @type {Map<promise, string>} */
+  keepAlive = new Map();
+
+  constructor(extension) {
+    this.extension = extension;
+  }
+
+  clearTimer() {
+    this.timer?.cancel();
+    this.timer = null;
+  }
+
+  resetTimer() {
+    this.sleepTime = Cu.now() + backgroundIdleTimeout;
+    if (!this.timer) {
+      this.createTimer();
+    }
+  }
+
+  createTimer() {
+    let timeLeft = this.sleepTime - Cu.now();
+    this.timer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
+    this.timer.init(() => this.timeout(), timeLeft, Ci.nsITimer.TYPE_ONE_SHOT);
+  }
+
+  timeout() {
+    this.clearTimer();
+    if (!this.keepAlive.size) {
+      if (Cu.now() < this.sleepTime) {
+        this.createTimer();
+      } else {
+        this.extension.terminateBackground();
+      }
+    }
+  }
+};
+
 this.backgroundPage = class extends ExtensionAPI {
   async onManifestEntry() {
     let { extension } = this;

toolkit/components/extensions/test/xpcshell/test_ext_dnr_static_rules_limits.js

@@ -36,7 +36,7 @@ add_setup(async () => {
   // e.g. see Bug 1803801) have a higher chance that the test extension may have hit the
   // idle timeout and being suspended by the time the test is going to trigger API method
   // calls through test API events (which do not expect the lifetime of the event page).
-  Services.prefs.setBoolPref("extensions.background.idle.enabled", false);
+  Services.prefs.setIntPref("extensions.background.idle.timeout", 300_000);
 
   // NOTE: reduce the static rules limits to reduce the amount of time needed to run
   // this xpcshell test.

toolkit/components/extensions/test/xpcshell/test_ext_eventpage_idle.js

@@ -208,7 +208,7 @@ add_task(
       "background-script-reset-idle"
     );
 
-    equal(resetData.reason, "parentApiCall", "Got the expected idle reset.");
+    equal(resetData.reason, "parentapicall", "Got the expected idle reset.");
 
     await promiseExtensionEvent(extension, "shutdown-background-script");
 
@@ -640,7 +640,7 @@ add_task(
     Assert.deepEqual(
       resetIdleData,
       {
-        reason: "pendingListeners",
+        reason: "listeners",
         pendingListeners: 2,
       },
       "Got the expected idle reset reason and pendingListeners count"
@@ -754,7 +754,7 @@ add_task(
     Assert.deepEqual(
       resetIdleData,
       {
-        reason: "pendingListeners",
+        reason: "listeners",
         pendingListeners: 1,
       },
       "Got the expected idle reset reason and pendingListeners count"

toolkit/components/extensions/test/xpcshell/test_ext_webRequest_eventPage_StreamFilter.js

@@ -325,7 +325,9 @@ async function test_create_new_streamfilter_while_suspending({
   await extension.awaitMessage("suspend-listener");
 
   info("Simulated idle timeout canceled");
-  extension.extension.emit("background-script-reset-idle");
+  extension.extension.emit("background-script-reset-idle", {
+    reason: "other/simulate-idle-reset",
+  });
   await extension.awaitMessage("suspend-canceled-listener");
 
   await extension.unload();