Revert of Convert Google Now's Authentication Manager to use Promises...

Revert of Convert Google Now's Authentication Manager to use Promises (https://codereview.chromium.org/162273002/)

Reason for revert:
Promise.catch may not always be called back, breaking an assumption about the task tracking system.

Original issue's description:
> Convert Google Now's Authentication Manager to use Promises
> 
> BUG=164227
> R=rgustafson@chromium.org, skare@chromium.org, vadimt@chromium.org
> 
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=252109

TBR=skare@chromium.org,rgustafson@chromium.org,vadimt@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=164227

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@252134 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/resources/google_now/background.js

@@ -265,7 +265,12 @@ function recordEvent(event) {
  *     parameter.
  */
 function setAuthorization(request, callbackBoolean) {
-  authenticationManager.getAuthToken().then(function(token) {
+  authenticationManager.getAuthToken(function(token) {
+    if (!token) {
+      callbackBoolean(false);
+      return;
+    }
+
     request.setRequestHeader('Authorization', 'Bearer ' + token);
 
     // Instrument onloadend to remove stale auth tokens.
@@ -273,7 +278,7 @@ function setAuthorization(request, callbackBoolean) {
     request.onloadend = wrapper.wrapCallback(function(event) {
       if (request.status == HTTP_FORBIDDEN ||
           request.status == HTTP_UNAUTHORIZED) {
-        authenticationManager.removeToken(token).then(function() {
+        authenticationManager.removeToken(token, function() {
           originalOnLoadEnd(event);
         });
       } else {
@@ -282,8 +287,6 @@ function setAuthorization(request, callbackBoolean) {
     });
 
     callbackBoolean(true);
-  }).catch(function() {
-    callbackBoolean(false);
   });
 }
 
@@ -1085,7 +1088,7 @@ function updateRunningState(
 function onStateChange() {
   tasks.add(STATE_CHANGED_TASK_NAME, function() {
     Promise.all([
-        authenticationManager.isSignedIn(),
+        isSignedIn(),
         isGeolocationEnabled(),
         canEnableBackground(),
         isNotificationsEnabled(),
@@ -1096,6 +1099,18 @@ function onStateChange() {
   });
 }
 
+/**
+ * Determines if the user is signed in.
+ * @return {Promise} A promise to evaluate the signed in state.
+ */
+function isSignedIn() {
+  return new Promise(function(resolve) {
+    authenticationManager.isSignedIn(function(signedIn) {
+      resolve(signedIn);
+    });
+  });
+}
+
 /**
  * Gets the geolocation enabled preference.
  * @return {Promise} A promise to get the geolocation enabled preference.

chrome/browser/resources/google_now/background_unittest.gtestjs

@@ -70,7 +70,6 @@ function mockInitializeDependencies(fixture) {
     'instrumented.notifications.getPermissionLevel',
     'instrumented.preferencesPrivate.googleGeolocationAccessEnabled.get',
     'instrumented.storage.local.get',
-    'instrumented.webstorePrivate.getBrowserLogin',
     'tasks.add',
     'updateCardsAttempts.isRunning',
     'updateCardsAttempts.stop'
@@ -100,11 +99,14 @@ function expectStateMachineCalls(
     testExperimentVariationParams,
     testNotificationPermissionLevel,
     testGoogleNowEnabled) {
+  var authenticationManagerIsSignedInSavedArgs = new SaveMockArguments();
   fixture.mockApis.expects(once()).
-      authenticationManager_isSignedIn().
-      will(returnValue(new Promise(function(resolve) {
-        resolve(!!testIdentityToken);
-      })));
+      authenticationManager_isSignedIn(
+          authenticationManagerIsSignedInSavedArgs.match(ANYTHING)).
+      will(invokeCallback(
+          authenticationManagerIsSignedInSavedArgs,
+          0,
+          testIdentityToken));
 
   var getVariationParamsSavedArgs = new SaveMockArguments();
   fixture.mockApis.expects(once()).
@@ -131,13 +133,13 @@ function expectStateMachineCalls(
           0,
           testNotificationPermissionLevel))
 
-   var storageGetSavedArgs = new SaveMockArguments();
-   fixture.mockApis.expects(once()).
-       instrumented_storage_local_get(
-           storageGetSavedArgs.match(eq('googleNowEnabled')),
-           storageGetSavedArgs.match(ANYTHING)).
+   var storageGetSavedArgs = new SaveMockArguments();	 
+   fixture.mockApis.expects(once()).	 
+       instrumented_storage_local_get(	 
+           storageGetSavedArgs.match(eq('googleNowEnabled')),	 
+           storageGetSavedArgs.match(ANYTHING)).	 
        will(invokeCallback(
-          storageGetSavedArgs, 1, {googleNowEnabled: testGoogleNowEnabled}));
+          storageGetSavedArgs, 1, {googleNowEnabled: testGoogleNowEnabled}));	 
 
   fixture.mockGlobals.expects(once()).
       setBackgroundEnable(ANYTHING);
@@ -595,7 +597,7 @@ TEST_F(
       // Check the output value.
       var expectedCombinedCards = {
         'EXISTING CARD': [
-          1,
+          1, 
           {
             receivedNotification: receivedNotificationNoShowTime,
             hideTime: 11000

chrome/browser/resources/google_now/common_test_util.js

@@ -82,35 +82,16 @@ function getMockHandlerContainer(eventIdentifier) {
 var Promise = function() {
   function PromisePrototypeObject(asyncTask) {
     var result;
-    var resolved = false;
     asyncTask(
         function(asyncResult) {
           result = asyncResult;
-          resolved = true;
         },
-        function(asyncFailureResult) {
-          result = asyncFailureResult;
-          resolved = false;
-        });
+        function() {}); // Errors are unsupported.
 
     function then(callback) {
-      if (resolved) {
-        callback.call(null, result);
-      }
-      return this;
-    }
-
-    // Promises use the function name "catch" to call back error handlers.
-    // We can't use "catch" since function or variable names cannot use the word
-    // "catch".
-    function catchFunc(callback) {
-      if (!resolved) {
-        callback.call(null, result);
-      }
-      return this;
+      callback.call(null, result);
     }
-
-    return {then: then, catch: catchFunc, isPromise: true};
+    return {then: then, isPromise: true};
   }
 
   function all(arrayOfPromises) {

chrome/browser/resources/google_now/utility.js

@@ -437,24 +437,13 @@ wrapper.instrumentChromeApiFunction('identity.removeCachedAuthToken', 1);
 wrapper.instrumentChromeApiFunction('webstorePrivate.getBrowserLogin', 0);
 
 /**
- * Add task tracking support to Promise.then.
+ * Add task tracking support to Promises.
  * @override
  */
 Promise.prototype.then = function() {
   var originalThen = Promise.prototype.then;
   return function(callback) {
-    return originalThen.call(this, wrapper.wrapCallback(callback, false));
-  }
-}();
-
-/**
- * Add task tracking support to Promise.catch.
- * @override
- */
-Promise.prototype.catch = function() {
-  var originalCatch = Promise.prototype.catch;
-  return function(callback) {
-    return originalCatch.call(this, wrapper.wrapCallback(callback, false));
+    originalThen.call(this, wrapper.wrapCallback(callback, false));
   }
 }();
 
@@ -737,46 +726,36 @@ function buildAuthenticationManager() {
 
   /**
    * Gets an OAuth2 access token.
-   * @return {Promise} A promise to get the authentication token. If there is
-   *     no token, the request is rejected.
+   * @param {function(string=)} callback Called on completion.
+   *     The string contains the token. It's undefined if there was an error.
    */
-  function getAuthToken() {
-    return new Promise(function(resolve, reject) {
-      instrumented.identity.getAuthToken({interactive: false}, function(token) {
-        if (chrome.runtime.lastError || !token) {
-          reject();
-        } else {
-          resolve(token);
-        }
-      });
+  function getAuthToken(callback) {
+    instrumented.identity.getAuthToken({interactive: false}, function(token) {
+      token = chrome.runtime.lastError ? undefined : token;
+      callback(token);
     });
   }
 
   /**
    * Determines whether there is an account attached to the profile.
-   * @return {Promise} A promise to determine if there is an account attached
-   *     to the profile.
+   * @param {function(boolean)} callback Called on completion.
    */
-  function isSignedIn() {
-    return new Promise(function(resolve) {
-      instrumented.webstorePrivate.getBrowserLogin(function(accountInfo) {
-        resolve(!!accountInfo.login);
-      });
+  function isSignedIn(callback) {
+    instrumented.webstorePrivate.getBrowserLogin(function(accountInfo) {
+      callback(!!accountInfo.login);
     });
   }
 
   /**
    * Removes the specified cached token.
    * @param {string} token Authentication Token to remove from the cache.
-   * @return {Promise} A promise that resolves on completion.
+   * @param {function()} callback Called on completion.
    */
-  function removeToken(token) {
-    return new Promise(function(resolve) {
-      instrumented.identity.removeCachedAuthToken({token: token}, function() {
-        // Let Chrome know about a possible problem with the token.
-        getAuthToken();
-        resolve();
-      });
+  function removeToken(token, callback) {
+    instrumented.identity.removeCachedAuthToken({token: token}, function() {
+      // Let Chrome now about a possible problem with the token.
+      getAuthToken(function() {});
+      callback();
     });
   }
 
@@ -796,7 +775,7 @@ function buildAuthenticationManager() {
    * If it doesn't, it notifies the listeners of the change.
    */
   function checkAndNotifyListeners() {
-    isSignedIn().then(function(signedIn) {
+    isSignedIn(function(signedIn) {
       instrumented.storage.local.get('lastSignedInState', function(items) {
         items = items || {};
         if (items.lastSignedInState != signedIn) {