Bug 1391531 - Make TelemetrySession idle callbacks cancellable. r=Dexter
authorChris H-C <chutten@mozilla.com>
Wed, 13 Sep 2017 13:24:31 -0400
changeset 668464 2f9ad07e528a5e758605123e69b342b1057f2506
parent 668463 c58ffaa1e4108d645c79bf24ec612f77999576f3
child 668465 e4f1ed133c6826aabc73672cd379037d5c05952b
push id81062
push usergszorc@mozilla.com
push dateThu, 21 Sep 2017 18:37:24 +0000
reviewersDexter
bugs1391531
milestone58.0a1
Bug 1391531 - Make TelemetrySession idle callbacks cancellable. r=Dexter TelemetrySession schedules some activities on idle, to avoid hogging resources. Sometimes we're forced to handle these situations on a more rigid timeline than "wait for idle". In those cases we should cancel the idle timeout so it doesn't double-perform the operations. Unfortunately idleDispatchToMainThread doesn't have a cancel mechanism, so we need to bolt one over it. MozReview-Commit-ID: JpUfb9rK2Od
toolkit/components/telemetry/TelemetrySession.jsm
--- a/toolkit/components/telemetry/TelemetrySession.jsm
+++ b/toolkit/components/telemetry/TelemetrySession.jsm
@@ -388,38 +388,76 @@ var TelemetryScheduler = {
         // This is needed because sleep time does not count towards timeouts
         // on Mac & Linux - see bug 1262386, bug 1204823 et al.
         return this._onSchedulerTick(true);
     }
     return undefined;
   },
 
   /**
+   * Creates an object with a method `dispatch` that will call `dispatchFn` unless
+   * the method `cancel` is called beforehand.
+   *
+   * This is used to wrap main thread idle dispatch since it does not provide a
+   * cancel mechanism.
+   */
+  _makeIdleDispatch(dispatchFn) {
+    this._log.trace("_makeIdleDispatch");
+    let fn = dispatchFn;
+    let l = (msg) => this._log.trace(msg); // need to bind `this`
+    return {
+      cancel() {
+        fn = undefined;
+      },
+      dispatch(resolve, reject) {
+        l("_makeIdleDispatch.dispatch - !!fn: " + !!fn);
+        if (!fn) {
+          return Promise.resolve().then(resolve, reject);
+        }
+        return fn(resolve, reject);
+      },
+    };
+  },
+
+  /**
    * Performs a scheduler tick. This function manages Telemetry recurring operations.
    * @param {Boolean} [dispatchOnIdle=false] If true, the tick is dispatched in the
    *                  next idle cycle of the main thread.
    * @return {Promise} A promise, only used when testing, resolved when the scheduled
    *                   operation completes.
    */
   _onSchedulerTick(dispatchOnIdle = false) {
+    this._log.trace("_onSchedulerTick - dispatchOnIdle: " + dispatchOnIdle);
     // This call might not be triggered from a timeout. In that case we don't want to
     // leave any previously scheduled timeouts pending.
     this._clearTimeout();
 
+    if (this._idleDispatch) {
+      this._idleDispatch.cancel();
+    }
+
     if (this._shuttingDown) {
       this._log.warn("_onSchedulerTick - already shutdown.");
       return Promise.reject(new Error("Already shutdown."));
     }
 
     let promise = Promise.resolve();
     try {
       if (dispatchOnIdle) {
+        this._idleDispatch = this._makeIdleDispatch((resolve, reject) => {
+          this._log.trace("_onSchedulerTick - ildeDispatchToMainThread dispatch");
+          return this._schedulerTickLogic().then(resolve, reject);
+        });
         promise = new Promise((resolve, reject) =>
-          Services.tm.idleDispatchToMainThread(() => this._schedulerTickLogic().then(resolve, reject)),
-                                               SCHEDULER_TICK_MAX_IDLE_DELAY_MS);
+          Services.tm.idleDispatchToMainThread(() => {
+            return this._idleDispatch
+              ? this._idleDispatch.dispatch(resolve, reject)
+              : Promise.resolve().then(resolve, reject)
+            },
+            SCHEDULER_TICK_MAX_IDLE_DELAY_MS));
       } else {
         promise = this._schedulerTickLogic();
       }
     } catch (e) {
       Telemetry.getHistogramById("TELEMETRY_SCHEDULER_TICK_EXCEPTION").add(1);
       this._log.error("_onSchedulerTick - There was an exception", e);
     } finally {
       this._rescheduleTimeout();