Refactor Authenticated Server Requests to use Promises

chrome/browser/resources/google_now/background.js

 // Copyright (c) 2013 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';
 
 /**
  * @fileoverview The event page for Google Now for Chrome implementation.
  * The Google Now event page gets Google Now cards from the server and shows
  * them as Chrome notifications.
@@ skipping 230 matching lines @@
     type: 'histogram-linear',
     min: 1,
     max: GoogleNowEvent.EVENTS_TOTAL,
     buckets: GoogleNowEvent.EVENTS_TOTAL + 1
   };
 
   chrome.metricsPrivate.recordValue(metricDescription, event);
 }
 
 /**
- * Adds authorization behavior to the request.
- * @param {XMLHttpRequest} request Server request.
- * @param {function(boolean)} callbackBoolean Completion callback with 'success'
- *     parameter.
+ * Builds and sends an authenticated request to the notification server.
+ * @param {string} method Request method.
+ * @param {string} handlerName Server handler to send the request to.

[rgustafson, 2014/03/07 00:53:06]

This is more than the handler. It includes params and such in some cases. I'm
kinda stumped on what to name it though.

[robliao, 2014/03/07 01:39:40]

Unfortunately there is no real name for the path+query. We can ponder this
later.
On 2014/03/07 00:53:06, rgustafson wrote:

+ * @param {string=} opt_contentType Value for the Content-type header.
+ * @return {Promise} A promise to issue a request to the server.
+ *     The promise rejects if there is a client-side authentication issue.
  */
-function setAuthorization(request, callbackBoolean) {
-  authenticationManager.getAuthToken().then(function(token) {
+function requestFromServer(method, handlerName, opt_contentType) {
+  return authenticationManager.getAuthToken().then(function(token) {
+    var request = buildServerRequest(method, handlerName, opt_contentType);
     request.setRequestHeader('Authorization', 'Bearer ' + token);
-
-    // Instrument onloadend to remove stale auth tokens.
-    var originalOnLoadEnd = request.onloadend;
-    request.onloadend = wrapper.wrapCallback(function(event) {
+    var requestPromise = new Promise(function(resolve) {
+      request.addEventListener('loadend', function() {

[rgustafson, 2014/03/07 00:53:06]

any difference between this and doing request.onloadend =?

[robliao, 2014/03/07 01:39:40]

It doesn't overwrite any existing onloadend that was set.
On 2014/03/07 00:53:06, rgustafson wrote:

+        resolve(request);
+      }, false);

[rgustafson, 2014/03/07 00:53:06]

This parameter is optional and looks like it defaults to false. Would have saved
me a trip to documentation.

[robliao, 2014/03/07 01:39:40]

Given that the default is not consistent (false in Chrome, true in other
browsers), I'm keeping it here for clarity. The web examples also use this.
On 2014/03/07 00:53:06, rgustafson wrote:

[rgustafson, 2014/03/11 21:29:30]

It's false for Firefox:
https://developer.mozilla.org/en-US/docs/Web/API/EventTarget.addEventListener
The IE docs aren't very clear, but link to the w3 spec
(http://www.w3.org/TR/DOM-Level-3-Events/#widl-EventTarget-addEventListener)
which says:
void    addEventListener (DOMString type, EventListener? listener, optional
boolean useCapture = false);
so I really really really hope none of the browsers default to true.

Web examples use it since it used to be required, but I believe removing it will
make it easier to comprehend without wondering what that false is.

[robliao, 2014/03/12 00:13:11]

Instead we'll wonder what the default is :-) for those aware that the default
isn't always false.

On 2014/03/11 21:29:30, rgustafson wrote:

+      request.send();
+    });
+    requestPromise.then(function() {
       if (request.status == HTTP_FORBIDDEN ||
           request.status == HTTP_UNAUTHORIZED) {
-        authenticationManager.removeToken(token).then(function() {
-          originalOnLoadEnd(event);
-        });
-      } else {
-        originalOnLoadEnd(event);
+        authenticationManager.removeToken(token);
       }
     });
-
-    callbackBoolean(true);
-  }).catch(function() {
-    callbackBoolean(false);
+    return requestPromise;
   });
 }
 
 /**
  * Shows parsed and combined cards as notifications.
  * @param {Object.<string, StoredNotificationGroup>} notificationGroups Map from
  *     group name to group information.
  * @param {Object.<ChromeNotificationId, CombinedCard>} cards Map from
  *     chromeNotificationId to the combined card, containing cards to show.
  * @param {function()} onSuccess Called on success.
@@ skipping 274 matching lines @@
   }
 
   groupNames.forEach(function(groupName) {
     requestParameters += ('&requestTypes=' + groupName);
   });
 
   requestParameters += '&uiLocale=' + navigator.language;
 
   console.log('requestNotificationGroups: request=' + requestParameters);
 
-  var request = buildServerRequest('GET', 'notifications' + requestParameters);
-
-  request.onloadend = function(event) {
-    console.log('requestNotificationGroups-onloadend ' + request.status);
-    if (request.status == HTTP_OK) {
-      recordEvent(GoogleNowEvent.REQUEST_FOR_CARDS_SUCCESS);
-      processServerResponse(
-          JSON.parse(request.responseText), cardShownCallback);
-    }
-  };
-
-  setAuthorization(request, function(success) {
-    if (success)
-      request.send();
-  });
+  requestFromServer('GET', 'notifications' + requestParameters).then(
+    function(request) {
+      console.log('requestNotificationGroups-received ' + request.status);
+      if (request.status == HTTP_OK) {
+        recordEvent(GoogleNowEvent.REQUEST_FOR_CARDS_SUCCESS);
+        processServerResponse(
+            JSON.parse(request.responseText), cardShownCallback);
+      }
+    });
 }
 
 /**
  * Requests the account opted-in state from the server.
  * @param {function()} optedInCallback Function that will be called if
  *     opted-in state is 'true'.
  */
 function requestOptedIn(optedInCallback) {
   console.log('requestOptedIn from ' + NOTIFICATION_CARDS_URL);
 
-  var request = buildServerRequest('GET', 'settings/optin');
-
-  request.onloadend = function(event) {
+  requestFromServer('GET', 'settings/optin').then(function(request) {
     console.log(
-        'requestOptedIn-onloadend ' + request.status + ' ' + request.response);
+        'requestOptedIn-received ' + request.status + ' ' + request.response);
     if (request.status == HTTP_OK) {
       var parsedResponse = JSON.parse(request.responseText);
       if (parsedResponse.value) {
         chrome.storage.local.set({googleNowEnabled: true});
         optedInCallback();
         // Google Now was disabled, now it's enabled. This is a state change.
         onStateChange();
       } else {
         scheduleNextPoll({}, false);
       }
     }
-  };
-
-  setAuthorization(request, function(success) {
-    if (success)
-      request.send();
   });
 }
 
 /**
  * Requests notification cards from the server.
  */
 function requestNotificationCards() {
   console.log('requestNotificationCards');
 
   fillFromChromeLocalStorage({
@@ skipping 78 matching lines @@
   var requestParameters = 'notifications/' + dismissalData.notificationId +
       '?age=' + dismissalAge +
       '&chromeNotificationId=' + chromeNotificationId;
 
   for (var paramField in dismissalData.parameters)
     requestParameters += ('&' + paramField +
     '=' + dismissalData.parameters[paramField]);
 
   console.log('requestCardDismissal: requestParameters=' + requestParameters);
 
-  var request = buildServerRequest('DELETE', requestParameters);
-  request.onloadend = function(event) {
+  requestFromServer('DELETE', requestParameters).then(function(request) {
     console.log('requestDismissingCard-onloadend ' + request.status);
     if (request.status == HTTP_NOCONTENT)
       recordEvent(GoogleNowEvent.DISMISS_REQUEST_SUCCESS);
 
     // A dismissal doesn't require further retries if it was successful or
     // doesn't have a chance for successful completion.
     var done = request.status == HTTP_NOCONTENT ||
         request.status == HTTP_BAD_REQUEST ||
         request.status == HTTP_METHOD_NOT_ALLOWED;
     callbackBoolean(done);
-  };
-
-  setAuthorization(request, function(success) {
-    if (success)
-      request.send();
-    else
-      callbackBoolean(false);
+  }).catch(function() {
+    callbackBoolean(false);
   });
 }
 
 /**
  * Tries to send dismiss requests for all pending dismissals.
  * @param {function(boolean)} callbackBoolean Completion callback with 'success'
  *     parameter. Success means that no pending dismissals are left.
  */
 function processPendingDismissals(callbackBoolean) {
   fillFromChromeLocalStorage({
@@ skipping 421 matching lines @@
             lastPollNowPayloads: items.lastPollNowPayloads,
             notificationGroups: items.notificationGroups
           });
 
           requestCards();
         }
       });
     });
   }
 });