Bug 1087608 - eliminating a pref observer leak and fixing test timeout overflow that cause intermittents. r=eeejay

accessible/jsat/AccessFu.jsm

@@ -46,7 +46,7 @@ this.AccessFu = { // jshint ignore:line
           this._enableOrDisable();
         });
         aWindow.navigator.mozSettings.addObserver(
-          SCREENREADER_SETTING, this.handleEvent.bind(this));
+          SCREENREADER_SETTING, this.handleEvent);
       }
     }
 
@@ -68,12 +68,21 @@ this.AccessFu = { // jshint ignore:line
       Services.obs.removeObserver(this, 'Accessibility:Settings');
     } else if (Utils.win.navigator.mozSettings) {
       Utils.win.navigator.mozSettings.removeObserver(
-        SCREENREADER_SETTING, this.handleEvent.bind(this));
+        SCREENREADER_SETTING, this.handleEvent);
     }
     delete this._activatePref;
     Utils.uninit();
   },
 
+  /**
+   * A lazy getter for event handler that binds the scope to AccessFu object.
+   */
+  get handleEvent() {
+    delete this.handleEvent;
+    this.handleEvent = this._handleEvent.bind(this);
+    return this.handleEvent;
+  },
+
   /**
    * Start AccessFu mode, this primarily means controlling the virtual cursor
    * with arrow keys.
@@ -344,7 +353,7 @@ this.AccessFu = { // jshint ignore:line
     }
   },
 
-  handleEvent: function handleEvent(aEvent) {
+  _handleEvent: function _handleEvent(aEvent) {
     switch (aEvent.type) {
       case 'TabOpen':
       {

accessible/jsat/Gestures.jsm

@@ -156,6 +156,14 @@ this.GestureSettings = { // jshint ignore:line
   maxConsecutiveGestureDelay:
     MAX_CONSECUTIVE_GESTURE_DELAY * TIMEOUT_MULTIPLIER,
 
+  /**
+   * A maximum time we wait for a next pointer down event to consider a sequence
+   * a multi-action gesture.
+   * @type {Number}
+   */
+  maxGestureResolveTimeout:
+    MAX_CONSECUTIVE_GESTURE_DELAY * TIMEOUT_MULTIPLIER,
+
   /**
    * Delay before tap turns into dwell
    * @type {Number}
@@ -359,7 +367,7 @@ Gesture.prototype = {
     let delay = this._getDelay(aTimeStamp);
     let handler = () => {
       Logger.gesture('timer handler');
-      delete this._timer;
+      this.clearTimer();
       if (!this._inProgress) {
         this._deferred.reject();
       } else if (this._rejectToOnWait) {
@@ -502,6 +510,7 @@ Gesture.prototype = {
     }
     Logger.gesture('Resolving', this.id, 'gesture.');
     this.isComplete = true;
+    this.clearTimer();
     let detail = this.compile();
     if (detail) {
       this._emit(detail);
@@ -526,6 +535,7 @@ Gesture.prototype = {
     }
     Logger.gesture('Rejecting', this.id, 'gesture.');
     this.isComplete = true;
+    this.clearTimer();
     return {
       id: this.id,
       gestureType: aRejectTo
@@ -692,11 +702,12 @@ TapGesture.prototype.pointerup = function TapGesture_pointerup(aPoints) {
       let complete = this._update(aPoints, 'pointerup', false, true);
       if (complete) {
         this.clearTimer();
-        if (GestureSettings.maxConsecutiveGestureDelay) {
+        if (GestureSettings.maxGestureResolveTimeout) {
           this._pointerUpTimer = setTimeout(() => {
+            clearTimeout(this._pointerUpTimer);
             delete this._pointerUpTimer;
             this._deferred.resolve();
-          }, GestureSettings.maxConsecutiveGestureDelay);
+          }, GestureSettings.maxGestureResolveTimeout);
         } else {
           this._deferred.resolve();
         }

accessible/tests/mochitest/jsat/dom_helper.js

@@ -7,9 +7,7 @@
 var Ci = Components.interfaces;
 var Cu = Components.utils;
 
-Cu.import('resource://gre/modules/accessibility/Utils.jsm');
 Cu.import('resource://gre/modules/Geometry.jsm');
-Cu.import("resource://gre/modules/accessibility/Gestures.jsm");
 
 var win = getMainChromeWindow(window);
 
@@ -100,11 +98,6 @@ var eventMap = {
   touchmove: sendTouchEvent
 };
 
-var originalDwellThreshold = GestureSettings.dwellThreshold;
-var originalSwipeMaxDuration = GestureSettings.swipeMaxDuration;
-var originalConsecutiveGestureDelay =
-  GestureSettings.maxConsecutiveGestureDelay;
-
 /**
  * Attach a listener for the mozAccessFuGesture event that tests its
  * type.
@@ -158,9 +151,11 @@ function setTimers(aTimeStamp, aRemoveDwellThreshold, aRemoveSwipeMaxDuration) {
   GestureTracker.current.startTimer(aTimeStamp);
 }
 
-function resetTimers() {
-  GestureSettings.dwellThreshold = originalDwellThreshold;
-  GestureSettings.swipeMaxDuration = originalSwipeMaxDuration;
+function resetTimers(aRemoveGestureResolveDelay) {
+  GestureSettings.dwellThreshold = AccessFuTest.dwellThreshold;
+  GestureSettings.swipeMaxDuration = AccessFuTest.swipeMaxDuration;
+  GestureSettings.maxGestureResolveTimeout = aRemoveGestureResolveDelay ?
+    0 : AccessFuTest.maxGestureResolveTimeout;
 }
 
 /**
@@ -179,10 +174,7 @@ AccessFuTest.addSequence = function AccessFuTest_addSequence(aSequence) {
         type: aEvent.type
       };
       var timeStamp = Date.now();
-      resetTimers();
-      GestureSettings.maxConsecutiveGestureDelay =
-        aEvent.removeConsecutiveGestureDelay ?
-        0 : originalConsecutiveGestureDelay;
+      resetTimers(aEvent.removeGestureResolveDelay);
       GestureTracker.handle(event, timeStamp);
       setTimers(timeStamp, aEvent.removeDwellThreshold,
         aEvent.removeSwipeMaxDuration);

accessible/tests/mochitest/jsat/gestures.json

@@ -3,7 +3,7 @@
     "events": [
       {"type": "pointerdown", "points": [{"x": 1, "y": 1, "identifier": 1}]},
       {"type": "pointerup", "points": [{"x": 1, "y": 1, "identifier": 1}],
-       "removeConsecutiveGestureDelay": true }
+       "removeGestureResolveDelay": true }
     ],
     "expectedGestures": [{ "type": "tap" }]
   },
@@ -13,7 +13,7 @@
       {"type": "pointermove",
         "points": [{"x": 1.03, "y": 1.03, "identifier": 1}]},
       {"type": "pointerup", "points": [{"x": 1.03, "y": 1.03, "identifier": 1}],
-       "removeConsecutiveGestureDelay": true }
+       "removeGestureResolveDelay": true }
     ],
     "expectedGestures": [{ "type": "tap" }]
   },
@@ -37,7 +37,7 @@
         "points": [{"x": 0.97, "y": 1.01, "identifier": 1}]},
       {"type": "pointerup",
         "points": [{"x": 0.97, "y": 1.01, "identifier": 1}],
-        "removeConsecutiveGestureDelay": true }
+        "removeGestureResolveDelay": true }
     ],
     "expectedGestures": [{ "type": "doubletap" }]
   },
@@ -49,7 +49,7 @@
       {"type": "pointerup", "points": [{"x": 1, "y": 1, "identifier": 1}]},
       {"type": "pointerdown", "points": [{"x": 1, "y": 1, "identifier": 1}]},
       {"type": "pointerup", "points": [{"x": 1, "y": 1, "identifier": 1}],
-       "removeConsecutiveGestureDelay": true }
+       "removeGestureResolveDelay": true }
     ],
     "expectedGestures": [{ "type": "tripletap" }]
   },
@@ -292,7 +292,7 @@
         {"y": 1.30098, "x": 1.52602, "identifier": 0},
         {"y": 1.94093, "x": 1.02672, "identifier": 1},
         {"y": 2.67229, "x": 0.75246, "identifier": 2}], "type": "pointerup",
-       "removeConsecutiveGestureDelay": false}],
+       "removeGestureResolveDelay": true}],
     "expectedGestures": [{ "type": "tripletap", "fingers": 3 }]
   },
   {
@@ -343,14 +343,8 @@
        "points": [{"identifier": 0, "x": 1.4375, "y": 2.59375},
                   {"identifier": 2, "x": 2.15625, "y": 1.489583}]},
       {"type": "pointerup",
-       "points": [{"identifier": 1, "x": 1.083333, "y": 3.71875}]},
-      {"type": "pointermove",
-       "points": [{"identifier": 0, "x": 1.427083, "y": 2.59375}]},
-      {"type": "pointerup",
-       "points": [{"identifier": 0, "x": 1.427083, "y": 2.59375}]},
-      {"type": "pointerup",
-       "points": [{"identifier": 2, "x": 2.15625, "y": 1.489583}],
-       "removeConsecutiveGestureDelay": false}
+       "points": [{"identifier": 1, "x": 1.083333, "y": 3.71875}],
+       "removeGestureResolveDelay": true}
     ],
     "expectedGestures": [{ "type": "tripletap", "fingers": 3 }]
   }

accessible/tests/mochitest/jsat/jsatcommon.js

@@ -19,17 +19,6 @@ Components.utils.import("resource://gre/modules/accessibility/Utils.jsm");
 Components.utils.import("resource://gre/modules/accessibility/EventManager.jsm");
 Components.utils.import("resource://gre/modules/accessibility/Gestures.jsm");
 
-const dwellThreshold = GestureSettings.dwellThreshold;
-const swipeMaxDuration = GestureSettings.swipeMaxDuration;
-const maxConsecutiveGestureDelay = GestureSettings.maxConsecutiveGestureDelay;
-
-// https://bugzilla.mozilla.org/show_bug.cgi?id=1001945 - sometimes
-// SimpleTest.executeSoon timeout is bigger than the timer settings in
-// GestureSettings that causes intermittents.
-GestureSettings.dwellThreshold = dwellThreshold * 10;
-GestureSettings.swipeMaxDuration = swipeMaxDuration * 10;
-GestureSettings.maxConsecutiveGestureDelay = maxConsecutiveGestureDelay * 10;
-
 var AccessFuTest = {
 
   addFunc: function AccessFuTest_addFunc(aFunc) {
@@ -111,9 +100,13 @@ var AccessFuTest = {
     Logger.test = false;
     Logger.logLevel = Logger.INFO;
     // Reset Gesture Settings.
-    GestureSettings.dwellThreshold = dwellThreshold;
-    GestureSettings.swipeMaxDuration = swipeMaxDuration;
-    GestureSettings.maxConsecutiveGestureDelay = maxConsecutiveGestureDelay;
+    GestureSettings.dwellThreshold = this.dwellThreshold =
+      this.originalDwellThreshold;
+    GestureSettings.swipeMaxDuration = this.swipeMaxDuration =
+      this.originalSwipeMaxDuration;
+    GestureSettings.maxGestureResolveTimeout =
+      this.maxGestureResolveTimeout =
+      this.originalMaxGestureResolveTimeout;
     // Finish through idle callback to let AccessFu._disable complete.
     SimpleTest.executeSoon(function () {
       AccessFu.detach();
@@ -160,6 +153,20 @@ var AccessFuTest = {
       ['dom.mozSettings.enabled', true]];
     prefs.push.apply(prefs, aAdditionalPrefs);
 
+    this.originalDwellThreshold = GestureSettings.dwellThreshold;
+    this.originalSwipeMaxDuration = GestureSettings.swipeMaxDuration;
+    this.originalMaxGestureResolveTimeout =
+      GestureSettings.maxGestureResolveTimeout;
+    // https://bugzilla.mozilla.org/show_bug.cgi?id=1001945 - sometimes
+    // SimpleTest.executeSoon timeout is bigger than the timer settings in
+    // GestureSettings that causes intermittents.
+    this.dwellThreshold = GestureSettings.dwellThreshold =
+      GestureSettings.dwellThreshold * 10;
+    this.swipeMaxDuration = GestureSettings.swipeMaxDuration =
+      GestureSettings.swipeMaxDuration * 10;
+    this.maxGestureResolveTimeout = GestureSettings.maxGestureResolveTimeout =
+      GestureSettings.maxGestureResolveTimeout * 10;
+
     SpecialPowers.pushPrefEnv({ 'set': prefs }, function () {
       if (AccessFuTest._waitForExplicitFinish) {
         // Run all test functions asynchronously.

accessible/tests/mochitest/jsat/test_live_regions.html

@@ -20,7 +20,7 @@
 
     function stopAccessFu() {
       SpecialPowers.setIntPref("accessibility.accessfu.activate", 0);
-      AccessFuTest.once_log("EventManager.stop", AccessFuTest.finish);
+      AccessFuTest.once_log("EventManager.stop", () => AccessFuTest.finish());
     }
 
     function hide(id) {

accessible/tests/mochitest/jsat/test_quicknav_modes.html

@@ -68,7 +68,7 @@
     // Listen for initial 'EventManager.start' and disable AccessFu.
     function prefStop() {
       ok(AccessFu._enabled, "AccessFu was started via preference.");
-      AccessFuTest.once_log("EventManager.stop", AccessFuTest.finish);
+      AccessFuTest.once_log("EventManager.stop", () => AccessFuTest.finish());
       SpecialPowers.setIntPref("accessibility.accessfu.activate", 0);
     }