Revert of Fix flaky remoting webapp javascript unittest. (patchset #1 id:1 of...

Revert of Fix flaky remoting webapp javascript unittest. (patchset #1 id:1 of https://codereview.chromium.org/1633273002/ )

Reason for revert:
This is still quite flaky on Linux. It has failed the 3 times in a row now.

https://build.chromium.org/p/chromium.linux/builders/Linux%20Tests

Original issue's description:
> Fix flaky remoting webapp javascript unittest.
>
> After forcing tests to fail after a 10 seconds timeout, turns out the
> spy promise tests are the culprit.
> I have removed them from our tests as they are originally intended to
> test XHR's and we now have a better API for that.
>
> BUG=504204
>
> Committed: https://crrev.com/aeb109ffd9b79927edb6a75f8cef7e72792fb356
> Cr-Commit-Position: refs/heads/master@{#371658}

TBR=jamiewalch@chromium.org,kelvinp@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=504204

Review URL: https://codereview.chromium.org/1643823002

Cr-Commit-Position: refs/heads/master@{#371934}

chrome/test/remoting/webapp_javascript_unittest.cc

@@ -12,8 +12,15 @@
 
 namespace remoting {
 
+// Flakily times out on Win7 Tests (dbg) and Linux Tests (dbg)(1):
+// https://crbug.com/504204.
+#if (defined(OS_WIN) || defined(OS_LINUX)) && !defined(NDEBUG)
+#define MAYBE_Remoting_Webapp_Js_Unittest DISABLED_Remoting_Webapp_Js_Unittest
+#else
+#define MAYBE_Remoting_Webapp_Js_Unittest Remoting_Webapp_Js_Unittest
+#endif
 IN_PROC_BROWSER_TEST_F(QUnitBrowserTestRunner,
-                       Remoting_Webapp_Js_Unittest) {
+                       MAYBE_Remoting_Webapp_Js_Unittest) {
   base::FilePath base_dir;
   ASSERT_TRUE(PathService::Get(base::DIR_EXE, &base_dir));
 

remoting/remoting_webapp_files.gypi

@@ -80,6 +80,9 @@
     ],
     # The unit test cases for the webapp
     'remoting_webapp_unittests_js_files': [
+      # TODO(jrw): Move spy_promise to base.
+      'webapp/unittests/spy_promise.js',
+      'webapp/unittests/spy_promise_unittest.js',
       'webapp/base/js/base_unittest.js',
       'webapp/base/js/base_event_hook_unittest.js',
       'webapp/base/js/base_inherits_unittest.js',

remoting/webapp/base/js/dns_blackhole_checker_unittest.js

@@ -74,7 +74,7 @@ QUnit.test('success',
       assert.equal(checker.getState(), state);
     }
 
-    return Promise.resolve().then(function() {
+    return base.SpyPromise.run(function() {
       fakeXhr.respond(200);
     }).then(function() {
       sinon.assert.notCalled(onStateChange);
@@ -98,7 +98,7 @@ QUnit.test('http response after connected',
 
     // Verify that DnsBlackholeChecker stays in HANDSHAKE state even if the
     // signal strategy has connected.
-    return Promise.resolve().then(function() {
+    return base.SpyPromise.run(function() {
       signalStrategy.setStateForTesting(
           remoting.SignalStrategy.State.CONNECTED);
     }).then(function() {
@@ -107,7 +107,7 @@ QUnit.test('http response after connected',
 
       // Verify that DnsBlackholeChecker goes to CONNECTED state after the
       // the HTTP request has succeeded.
-      return Promise.resolve().then(function() {
+      return base.SpyPromise.run(function() {
         fakeXhr.respond(200);
       });
     }).then(function() {
@@ -123,7 +123,7 @@ QUnit.test('connect failed',
       sinon.assert.calledWith(onStateChange, state);
     };
 
-    return Promise.resolve().then(function() {
+    return base.SpyPromise.run(function() {
       fakeXhr.respond(200);
     }).then(function() {
       sinon.assert.notCalled(onStateChange);
@@ -146,7 +146,7 @@ QUnit.test('blocked',
           'checker state is still FAILED');
     };
 
-    return Promise.resolve().then(function() {
+    return base.SpyPromise.run(function() {
       fakeXhr.respond(400);
     }).then(function() {
       sinon.assert.calledWith(
@@ -175,7 +175,7 @@ QUnit.test('blocked after connected',
 
     // Verify that DnsBlackholeChecker stays in HANDSHAKE state even
     // if the signal strategy has connected.
-    return Promise.resolve().then(function() {
+    return base.SpyPromise.run(function() {
       signalStrategy.setStateForTesting(
           remoting.SignalStrategy.State.CONNECTED);
     }).then(function() {
@@ -184,7 +184,7 @@ QUnit.test('blocked after connected',
 
       // Verify that DnsBlackholeChecker goes to FAILED state after it
       // gets the blocked HTTP response.
-      return Promise.resolve().then(function() {
+      return base.SpyPromise.run(function() {
         fakeXhr.respond(400);
       });
     }).then(function() {

remoting/webapp/files.gni

@@ -77,6 +77,9 @@ remoting_webapp_unittests_exclude_js_files = [
 
 # The unit test cases for the webapp
 remoting_webapp_unittests_js_files = [
+  # TODO(jrw): Move spy_promise to base.
+  "unittests/spy_promise.js",
+  "unittests/spy_promise_unittest.js",
   "base/js/base_unittest.js",
   "base/js/base_event_hook_unittest.js",
   "base/js/base_inherits_unittest.js",

remoting/webapp/unittests/spy_promise.js

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+'use strict';
+
+/** @suppress {duplicate} */
+var base = base || {};
+
+(function() {
+/**
+ * A wrapper around a Promise object that keeps track of all
+ * outstanding promises.  This function is written to serve as a
+ * drop-in replacement for the native Promise constructor.  To create
+ * a SpyPromise from an existing native Promise, use
+ * SpyPromise.resolve.
+ *
+ * Note that this is a pseudo-constructor that actually returns a
+ * regular promise with appropriate handlers attached.  This detail
+ * should be transparent when SpyPromise.activate has been called.
+ *
+ * The normal way to use this class is within a call to
+ * SpyPromise.run, for example:
+ *
+ *   base.SpyPromise.run(function() {
+ *     myCodeThatUsesPromises();
+ *   });
+ *   base.SpyPromise.settleAll().then(function() {
+ *     console.log('All promises have been settled!');
+ *   });
+ *
+ * @constructor
+ * @extends {Promise}
+ * @param {function(function(?):?, function(*):?):?} func A function
+ *     of the same type used as an argument to the native Promise
+ *     constructor, in other words, a function which is called
+ *     immediately, and whose arguments are a resolve function and a
+ *     reject function.
+ */
+base.SpyPromise = function(func) {
+  var unsettled = new RealPromise(func);
+  var unsettledId = remember(unsettled);
+  return unsettled.then(function(/** * */value) {
+    forget(unsettledId);
+    return value;
+  }, function(error) {
+    forget(unsettledId);
+    throw error;
+  });
+};
+
+/**
+ * The real promise constructor.  Needed because it is normally hidden
+ * by SpyPromise.activate or SpyPromise.run.
+ * @const
+ */
+var RealPromise = Promise;
+
+/**
+ * The real window.setTimeout method.  Needed because some test
+ * frameworks like to replace this method with a fake implementation.
+ * @const
+ */
+var realSetTimeout = window.setTimeout.bind(window);
+
+/**
+ * The number of unsettled promises.
+ * @type {number}
+ */
+base.SpyPromise.unsettledCount;  // initialized by reset()
+
+/**
+ * A collection of all unsettled promises.
+ * @type {!Object<number,!Promise>}
+ */
+var unsettled;  // initialized by reset()
+
+/**
+ * A counter used to assign ID numbers to new SpyPromise objects.
+ * @type {number}
+ */
+var nextPromiseId;  // initialized by reset()
+
+/**
+ * A promise returned by SpyPromise.settleAll.
+ * @type {Promise<null>}
+ */
+var settleAllPromise;  // initialized by reset()
+
+/**
+ * Records an unsettled promise.
+ *
+ * @param {!Promise} unsettledPromise
+ * @return {number} The ID number to be passed to forget_.
+ */
+function remember(unsettledPromise) {
+  var id = nextPromiseId++;
+  if (unsettled[id] != null) {
+    throw Error('Duplicate ID: ' + id);
+  }
+  base.SpyPromise.unsettledCount++;
+  unsettled[id] = unsettledPromise;
+  return id;
+};
+
+/**
+ * Forgets a promise.  Called after the promise has been settled.
+ *
+ * @param {number} id
+ * @private
+ */
+function forget(id) {
+  console.assert(unsettled[id] != null, 'No such Promise: ' + id + '.');
+  base.SpyPromise.unsettledCount--;
+  delete unsettled[id];
+};
+
+/**
+ * Forgets about all unsettled promises.
+ */
+base.SpyPromise.reset = function() {
+  base.SpyPromise.unsettledCount = 0;
+  unsettled = {};
+  nextPromiseId = 0;
+  settleAllPromise = null;
+};
+
+// Initialize static variables.
+base.SpyPromise.reset();
+
+/**
+ * Tries to wait until all promises has been settled.
+ *
+ * @param {number=} opt_maxTimeMs The maximum number of milliseconds
+ *     (approximately) to wait (default: 1000).
+ * @return {!Promise<null>} A real promise that is resolved when all
+ *     SpyPromises have been settled, or rejected after opt_maxTimeMs
+ *     milliseconds have elapsed.
+ */
+base.SpyPromise.settleAll = function(opt_maxTimeMs) {
+  if (settleAllPromise) {
+    return settleAllPromise;
+  }
+
+  var maxDelay = opt_maxTimeMs == null ? 1000 : opt_maxTimeMs;
+
+  /**
+   * @param {number} count
+   * @param {number} totalDelay
+   * @return {!Promise<null>}
+   */
+  function loop(count, totalDelay) {
+    return new RealPromise(function(resolve, reject) {
+      if (base.SpyPromise.unsettledCount == 0) {
+        settleAllPromise = null;
+        resolve(null);
+      } else if (totalDelay > maxDelay) {
+        settleAllPromise = null;
+        base.SpyPromise.reset();
+        reject(new Error('base.SpyPromise.settleAll timed out'));
+      } else {
+        // This implements quadratic backoff according to Euler's
+        // triangular number formula.
+        var delay = count;
+
+        // Must jump through crazy hoops to get a real timer in a unit test.
+        realSetTimeout(function() {
+          resolve(loop(
+              count + 1,
+              delay + totalDelay));
+        }, delay);
+      }
+    });
+  };
+
+  // An extra promise needed here to prevent the loop function from
+  // finishing before settleAllPromise is set.  If that happens,
+  // settleAllPromise will never be reset to null.
+  settleAllPromise = RealPromise.resolve().then(function() {
+    return loop(0, 0);
+  });
+  return settleAllPromise;
+};
+
+/**
+ * Only for testing this class.  Do not use.
+ * @returns {boolean} True if settleAll is executing.
+ */
+base.SpyPromise.isSettleAllRunning = function() {
+  return settleAllPromise != null;
+};
+
+/**
+ * Wrapper for Promise.resolve.
+ *
+ * @param {*} value
+ * @return {!base.SpyPromise}
+ */
+base.SpyPromise.resolve = function(value) {
+  return new base.SpyPromise(function(resolve, reject) {
+    resolve(value);
+  });
+};
+
+/**
+ * Wrapper for Promise.reject.
+ *
+ * @param {*} value
+ * @return {!base.SpyPromise}
+ */
+base.SpyPromise.reject = function(value) {
+  return new base.SpyPromise(function(resolve, reject) {
+    reject(value);
+  });
+};
+
+/**
+ * Wrapper for Promise.all.
+ *
+ * @param  {!Array<Promise>} promises
+ * @return {!base.SpyPromise}
+ */
+base.SpyPromise.all = function(promises) {
+  return base.SpyPromise.resolve(RealPromise.all(promises));
+};
+
+/**
+ * Wrapper for Promise.race.
+ *
+ * @param {!Array<Promise>} promises
+ * @return {!base.SpyPromise}
+ */
+base.SpyPromise.race = function(promises) {
+  return base.SpyPromise.resolve(RealPromise.race(promises));
+};
+
+/**
+ * Sets Promise = base.SpyPromise.  Must not be called more than once
+ * without an intervening call to restore().
+ */
+base.SpyPromise.activate = function() {
+  if (settleAllPromise) {
+    throw Error('called base.SpyPromise.activate while settleAll is running');
+  }
+  if (Promise === base.SpyPromise) {
+    throw Error('base.SpyPromise is already active');
+  }
+  Promise = /** @type {function(new:Promise)} */(base.SpyPromise);
+};
+
+/**
+ * Restores the original value of Promise.
+ */
+base.SpyPromise.restore = function() {
+  if (settleAllPromise) {
+    throw Error('called base.SpyPromise.restore while settleAll is running');
+  }
+  if (Promise === base.SpyPromise) {
+    Promise = RealPromise;
+  } else if (Promise === RealPromise) {
+    throw new Error('base.SpyPromise is not active.');
+  } else {
+    throw new Error('Something fishy is going on.');
+  }
+};
+
+/**
+ * Calls func with Promise equal to base.SpyPromise.
+ *
+ * @param {function():void} func A function which is expected to
+ *     create one or more promises.
+ * @param {number=} opt_timeoutMs An optional timeout specifying how
+ *     long to wait for promise chains started in func to be settled.
+ *     (default: 1000 ms)
+ * @return {!Promise<null>} A promise that is resolved after every
+ *     promise chain started in func is fully settled, or rejected
+ *     after a opt_timeoutMs.  In any case, the original value of the
+ *     Promise constructor is restored before this promise is settled.
+ */
+base.SpyPromise.run = function(func, opt_timeoutMs) {
+  base.SpyPromise.activate();
+  try {
+    func();
+  } finally {
+    return base.SpyPromise.settleAll(opt_timeoutMs).then(function() {
+      base.SpyPromise.restore();
+      return null;
+    }, function(error) {
+      base.SpyPromise.restore();
+      throw error;
+    });
+  }
+};
+})();

remoting/webapp/unittests/spy_promise_unittest.js

+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+(function() {
+
+'use strict';
+
+var originalGlobalPromise = Promise;
+
+QUnit.module('spy_promise', {
+  beforeEach: function(/** QUnit.Assert*/ assert) {
+    assertInitialState(assert);
+    base.SpyPromise.reset(); // Defend against broken tests.
+  },
+  afterEach: function(/** QUnit.Assert*/ assert) {
+    assertInitialState(assert);
+  }
+});
+
+function assertInitialState(/** QUnit.Assert */ assert) {
+  assert.equal(Promise, originalGlobalPromise);
+  assert.ok(
+      !base.SpyPromise.isSettleAllRunning(),
+      'settleAll should not be running');
+  assert.equal(
+      base.SpyPromise.unsettledCount, 0,
+      'base.SpyPromise.unsettledCount should be zero ' +
+        'before/after any test finishes');
+}
+
+/**
+ * @param {!QUnit.Assert} assert
+ * @return {!Promise}
+ */
+function finish(assert) {
+  return base.SpyPromise.settleAll().then(function() {
+    assert.equal(
+        base.SpyPromise.unsettledCount, 0,
+        'base.SpyPromise.unsettledCount should be zero ' +
+          'after settleAll finishes.');
+  });
+}
+
+QUnit.test('run', function(assert) {
+  var done = assert.async();
+  assert.notEqual(base.SpyPromise, originalGlobalPromise);
+  return base.SpyPromise.run(function() {
+    assert.equal(Promise, base.SpyPromise);
+    assert.equal(base.SpyPromise.unsettledCount, 0);
+    var dummy1 = new Promise(function(resolve) { resolve(null); });
+    assert.equal(base.SpyPromise.unsettledCount, 1);
+  }).then(function() {
+    assert.equal(Promise, originalGlobalPromise);
+    assert.equal(base.SpyPromise.unsettledCount, 0);
+    done();
+  });
+});
+
+QUnit.test('activate/restore', function(assert) {
+  assert.notEqual(base.SpyPromise, originalGlobalPromise);
+  base.SpyPromise.activate();
+  assert.notEqual(base.SpyPromise, originalGlobalPromise);
+  assert.equal(base.SpyPromise.unsettledCount, 0);
+  var dummy1 = new Promise(function(resolve) { resolve(null); });
+  assert.equal(base.SpyPromise.unsettledCount, 1);
+  base.SpyPromise.restore();
+  assert.equal(Promise, originalGlobalPromise);
+  return finish(assert);
+});
+
+QUnit.test('new/then', function(assert) {
+  var done = assert.async();
+  new base.SpyPromise(function(resolve, reject) {
+    resolve('hello');
+  }).then(function(/**string*/ value) {
+    assert.equal(base.SpyPromise.unsettledCount, 0);
+    assert.equal(value, 'hello');
+    done();
+  });
+  assert.equal(base.SpyPromise.unsettledCount, 1);
+  return finish(assert);
+});
+
+QUnit.test('new/catch', function(assert) {
+  var done = assert.async();
+  new base.SpyPromise(function(resolve, reject) {
+    reject('hello');
+  }).catch(function(/**string*/ value) {
+    assert.equal(base.SpyPromise.unsettledCount, 0);
+    assert.equal(value, 'hello');
+    done();
+  });
+  assert.equal(base.SpyPromise.unsettledCount, 1);
+  return finish(assert);
+});
+
+QUnit.test('new+throw/catch', function(assert) {
+  var done = assert.async();
+  new base.SpyPromise(function(resolve, reject) {
+    throw 'hello';
+  }).catch(function(/**string*/ value) {
+    assert.equal(base.SpyPromise.unsettledCount, 0);
+    assert.equal(value, 'hello');
+    done();
+  });
+  assert.equal(base.SpyPromise.unsettledCount, 1);
+  return finish(assert);
+});
+
+QUnit.test('resolve/then', function(assert) {
+  var done = assert.async();
+  base.SpyPromise.resolve('hello').then(function(/**string*/ value) {
+    assert.equal(base.SpyPromise.unsettledCount, 0);
+    assert.equal(value, 'hello');
+    done();
+  });
+  assert.equal(base.SpyPromise.unsettledCount, 1);
+  return finish(assert);
+});
+
+QUnit.test('reject/then', function(assert) {
+  var done = assert.async();
+  base.SpyPromise.reject('hello').then(null, function(/**string*/ value) {
+    assert.equal(base.SpyPromise.unsettledCount, 0);
+    assert.equal(value, 'hello');
+    done();
+  });
+  assert.equal(base.SpyPromise.unsettledCount, 1);
+  return finish(assert);
+});
+
+QUnit.test('reject/catch', function(assert) {
+  var done = assert.async();
+  base.SpyPromise.reject('hello').catch(function(/**string*/ value) {
+    assert.equal(base.SpyPromise.unsettledCount, 0);
+    assert.equal(value, 'hello');
+    done();
+  });
+  assert.equal(base.SpyPromise.unsettledCount, 1);
+  return finish(assert);
+});
+
+QUnit.test('all', function(assert) {
+  var done = assert.async();
+  base.SpyPromise.all([Promise.resolve(1), Promise.resolve(2)]).
+      then(
+          /** @param {string} value */
+          function(value) {
+            assert.equal(base.SpyPromise.unsettledCount, 0);
+            assert.deepEqual(value, [1, 2]);
+            done();
+          });
+  assert.equal(base.SpyPromise.unsettledCount, 1);
+  return finish(assert);
+});
+
+QUnit.test('race', function(assert) {
+  var done = assert.async();
+  var fast = Promise.resolve('fast');
+  var slow = new Promise(function() {}); // never settled
+  base.SpyPromise.race([fast, slow]).
+      then(function(/**string*/ value) {
+        assert.equal(base.SpyPromise.unsettledCount, 0);
+        assert.equal(value, 'fast');
+        done();
+      });
+  assert.equal(base.SpyPromise.unsettledCount, 1);
+  return finish(assert);
+});
+
+QUnit.test('resolve/then/then', function(assert) {
+  var done = assert.async();
+  base.SpyPromise.resolve('hello').then(function(/**string*/ value) {
+    assert.equal(value, 'hello');
+    return 'goodbye';
+  }).then(function(/**string*/ value) {
+    assert.equal(value, 'goodbye');
+    done();
+  });
+  return finish(assert);
+});
+
+
+QUnit.test('resolve/then+throw/catch', function(assert) {
+  var done = assert.async();
+  base.SpyPromise.resolve('hello').then(function(/**string*/ value) {
+    assert.equal(value, 'hello');
+    throw 'goodbye';
+  }).catch(function(/**string*/ value) {
+    assert.equal(value, 'goodbye');
+    done();
+  });
+  return finish(assert);
+});
+
+QUnit.test('reject/catch/then', function(assert) {
+  var done = assert.async();
+  base.SpyPromise.reject('hello').catch(function(/**string*/ value) {
+    assert.equal(value, 'hello');
+    return 'goodbye';
+  }).then(function(/**string*/ value) {
+    assert.equal(value, 'goodbye');
+    done();
+  });
+  return finish(assert);
+});
+
+
+QUnit.test('reject/catch+throw/catch', function(assert) {
+  var done = assert.async();
+  base.SpyPromise.reject('hello').catch(function(/**string*/ value) {
+    assert.equal(value, 'hello');
+    throw 'goodbye';
+  }).catch(function(/**string*/ value) {
+    assert.equal(value, 'goodbye');
+    done();
+  });
+  return finish(assert);
+});
+
+QUnit.test('settleAll timeout = 100', function(assert) {
+  var done = assert.async();
+  var startTime = Date.now();
+  var neverResolved = new base.SpyPromise(function() {});
+  return base.SpyPromise.settleAll(100).catch(function(error) {
+    assert.ok(error instanceof Error);
+    assert.ok(startTime + 200 < Date.now());
+    done();
+  });
+});
+
+QUnit.test('settleAll timeout = 500', function(assert) {
+  var done = assert.async();
+  var startTime = Date.now();
+  var neverResolved = new base.SpyPromise(function() {});
+  return base.SpyPromise.settleAll(500).catch(function(error) {
+    assert.ok(startTime + 750 < Date.now());
+    done();
+  });
+});
+
+QUnit.test('settleAll timeout = 1000', function(assert) {
+  var done = assert.async();
+  var startTime = Date.now();
+  var neverResolved = new base.SpyPromise(function() {});
+  return base.SpyPromise.settleAll(1000).catch(function(error) {
+    assert.ok(startTime + 1500 < Date.now());
+    done();
+  });
+});
+
+})();
\ No newline at end of file