Updated Google Now to Check the Geolocation Access Preference

chrome/browser/resources/google_now/background_unittest.gtestjs

Index: chrome/browser/resources/google_now/background_unittest.gtestjs
diff --git a/chrome/browser/resources/google_now/background_unittest.gtestjs b/chrome/browser/resources/google_now/background_unittest.gtestjs
index a0a8f00a9d9340f66e776ca127b92902bef90711..8631cfd77dbcdc9341792ae00ccbbb53cda7ed53 100644
--- a/chrome/browser/resources/google_now/background_unittest.gtestjs
+++ b/chrome/browser/resources/google_now/background_unittest.gtestjs
@@ -30,14 +30,22 @@ TEST_F('GoogleNowBackgroundUnitTest', 'AreTasksConflicting', function() {
   testTaskPair(UPDATE_CARDS_TASK_NAME, UPDATE_CARDS_TASK_NAME, true);
   testTaskPair(UPDATE_CARDS_TASK_NAME, DISMISS_CARD_TASK_NAME, false);
   testTaskPair(UPDATE_CARDS_TASK_NAME, RETRY_DISMISS_TASK_NAME, false);
+  testTaskPair(UPDATE_CARDS_TASK_NAME, STATE_CHANGED_TASK_NAME, false);
 
   testTaskPair(DISMISS_CARD_TASK_NAME, UPDATE_CARDS_TASK_NAME, false);
   testTaskPair(DISMISS_CARD_TASK_NAME, DISMISS_CARD_TASK_NAME, false);
   testTaskPair(DISMISS_CARD_TASK_NAME, RETRY_DISMISS_TASK_NAME, false);
+  testTaskPair(DISMISS_CARD_TASK_NAME, STATE_CHANGED_TASK_NAME, false);
 
   testTaskPair(RETRY_DISMISS_TASK_NAME, UPDATE_CARDS_TASK_NAME, true);
   testTaskPair(RETRY_DISMISS_TASK_NAME, DISMISS_CARD_TASK_NAME, true);
   testTaskPair(RETRY_DISMISS_TASK_NAME, RETRY_DISMISS_TASK_NAME, true);
+  testTaskPair(RETRY_DISMISS_TASK_NAME, STATE_CHANGED_TASK_NAME, false);
+
+  testTaskPair(STATE_CHANGED_TASK_NAME, UPDATE_CARDS_TASK_NAME, false);
+  testTaskPair(STATE_CHANGED_TASK_NAME, DISMISS_CARD_TASK_NAME, false);
+  testTaskPair(STATE_CHANGED_TASK_NAME, RETRY_DISMISS_TASK_NAME, false);
+  testTaskPair(STATE_CHANGED_TASK_NAME, STATE_CHANGED_TASK_NAME, false);
 });
 
 /**
@@ -49,32 +57,84 @@ function mockInitializeDependencies(fixture) {
       'recordEvent',
       'showWelcomeToast',
       'startPollingCards']);
-  fixture.makeAndRegisterMockApis(
-      ['storage.get', 'chrome.identity.getAuthToken']);
+  fixture.makeAndRegisterMockApis([
+      'chrome.identity.getAuthToken',
+      'chrome.notifications.getAll',
+      'chrome.preferencesPrivate.googleGeolocationAccessEnabled.get',
+      'storage.get']);
+}
+
+/**
+ * Sets up the test to expect the state machine calls and send

[vadimt, 2013/07/25 20:48:01]

Please mention which tested method this is for.

[robliao, 2013/07/25 22:35:48]

Done.

+ * the specified state machine state.
+ * @param {object} mockApisObj Mock APIs Object.
+ * @param {string} testIdentityToken getAuthToken callback token.
+ * @param {boolean} testGeolocationPref Geolocation Preference callback value.
+ * @param {boolean} testUserRespondedToToast User Response to toast
+ &     callback value.
+ */
+function expectStateMachineCalls(
+    mockApisObj,
+    testIdentityToken,
+    testGeolocationPref,
+    testUserRespondedToToast) {
+  var chromeIdentityGetAuthTokenSavedArgs = new SaveMockArguments();
+  mockApisObj.expects(once()).
+      chrome_identity_getAuthToken(
+          chromeIdentityGetAuthTokenSavedArgs.match(
+              eqJSON({interactive: false})),
+          chromeIdentityGetAuthTokenSavedArgs.match(ANYTHING)).
+      will(invokeCallback(
+          chromeIdentityGetAuthTokenSavedArgs, 1, testIdentityToken));
+
+  var googleGeolocationPrefGetSavedArgs = new SaveMockArguments();
+  mockApisObj.expects(once()).
+      chrome_preferencesPrivate_googleGeolocationAccessEnabled_get(
+          googleGeolocationPrefGetSavedArgs.match(eqJSON({})),
+          googleGeolocationPrefGetSavedArgs.match(ANYTHING)).
+      will(invokeCallback(
+          googleGeolocationPrefGetSavedArgs, 1, {value: testGeolocationPref}));
+
+  var storageGetSavedArgs = new SaveMockArguments();
+  mockApisObj.expects(once()).
+      storage_get(
+          storageGetSavedArgs.match(eq('userRespondedToToast')),
+          storageGetSavedArgs.match(ANYTHING)).
+      will(invokeCallback(storageGetSavedArgs, 1, testUserRespondedToToast));
 }
 
 TEST_F(
     'GoogleNowBackgroundUnitTest',
     'Initialize_ToastStateEmpty1',
     function() {
-      // Tests the case when toast state is empty and NOTIFICATION_CARDS_URL is
-      // not set. In this case, the function should quietly exit after finding
-      // out that NOTIFICATION_CARDS_URL is empty.
+      // Tests the case when the user isn't signed in and NOTIFICATION_CARDS_URL
+      // is not set. Since NOTIFICATION_CARDS_URL is empty,
+      // nothing should start.
 
       // Setup and expectations.
-      var testToastState = {};
       NOTIFICATION_CARDS_URL = undefined;
+      var testIdentityToken = undefined;
+      var testGeolocationPref = false;
+      var testUserRespondedToToast = {};
 
       mockInitializeDependencies(this);
 
       this.mockGlobals.expects(once()).recordEvent(
           DiagnosticEvent.EXTENSION_START);
-      var storageGetSavedArgs = new SaveMockArguments();
-      this.mockApis.expects(once()).
-          storage_get(
-              storageGetSavedArgs.match(eq('toastState')),
-              storageGetSavedArgs.match(ANYTHING)).
-          will(invokeCallback(storageGetSavedArgs, 1, testToastState));
+
+      expectStateMachineCalls(
+          this.mockApis,
+          testIdentityToken,
+          testGeolocationPref,
+          testUserRespondedToToast);
+
+      var chromeNotificationGetAllSavedArgs = new SaveMockArguments();
+      this.mockApis.expects(exactly(2)).
+          chrome_notifications_getAll(
+              chromeNotificationGetAllSavedArgs.match(ANYTHING)).
+          will(
+              invokeCallback(chromeNotificationGetAllSavedArgs, 0, {}),
+              invokeCallback(chromeNotificationGetAllSavedArgs, 0, {}));
 
       // Invoking the tested function.
       initialize();

[vadimt, 2013/07/25 20:48:01]

Unit tests typically mock all callees and only check that first-level callees
are invoked in a right order and with right args. So, we need to have a test for
initialize() that checks that recordEvent, stopPollingCards and onStateChange
are called, and also tests for these functions. Please do either:
1. Add a TODO comment to split unit tests later.
OR
2. Do this in this CL if you wish.

[robliao, 2013/07/25 22:35:48]

Done.

@@ -84,34 +144,35 @@ TEST_F(
     'GoogleNowBackgroundUnitTest',
     'Initialize_ToastStateEmpty2',
     function() {
-      // Tests the case when toast state is empty and NOTIFICATION_CARDS_URL is
-      // set, but getAuthToken fails most likely because the user is not signed
-      // in. In this case, the function should quietly exit after finding out
-      // that getAuthToken fails.
+      // Tests the case when NOTIFICATION_CARDS_URL is but getAuthToken fails
+      // most likely because the user is not signed in. In this case, the
+      // function should quietly exit after finding out that getAuthToken fails.
 
       // Setup and expectations.
-      var testToastState = {};
       NOTIFICATION_CARDS_URL = 'https://some.server.url.com';
       var testIdentityToken = undefined;
+      var testGeolocationPref = false;
+      var testUserRespondedToToast = {};
 
       mockInitializeDependencies(this);
 
       this.mockGlobals.expects(once()).recordEvent(
           DiagnosticEvent.EXTENSION_START);
-      var storageGetSavedArgs = new SaveMockArguments();
-      this.mockApis.expects(once()).
-          storage_get(
-              storageGetSavedArgs.match(eq('toastState')),
-              storageGetSavedArgs.match(ANYTHING)).
-          will(invokeCallback(storageGetSavedArgs, 1, testToastState));
-      var chromeIdentityGetAuthTokenSavedArgs = new SaveMockArguments();
-      this.mockApis.expects(once()).
-          chrome_identity_getAuthToken(
-              chromeIdentityGetAuthTokenSavedArgs.match(
-                  eqJSON({interactive: false})),
-              chromeIdentityGetAuthTokenSavedArgs.match(ANYTHING)).
-          will(invokeCallback(
-              chromeIdentityGetAuthTokenSavedArgs, 1, testIdentityToken));
+
+      expectStateMachineCalls(
+          this.mockApis,
+          testIdentityToken,
+          testGeolocationPref,
+          testUserRespondedToToast);
+
+      var chromeNotificationGetAllSavedArgs = new SaveMockArguments();
+      this.mockApis.expects(exactly(2)).
+          chrome_notifications_getAll(
+              chromeNotificationGetAllSavedArgs.match(ANYTHING)).
+          will(
+              invokeCallback(chromeNotificationGetAllSavedArgs, 0, {}),
+              invokeCallback(chromeNotificationGetAllSavedArgs, 0, {}));
+

[vadimt, 2013/07/25 20:48:01]

Remove empty line

[robliao, 2013/07/25 22:35:48]

Done.

 
       // Invoking the tested function.
       initialize();
@@ -121,33 +182,35 @@ TEST_F(
     'GoogleNowBackgroundUnitTest',
     'Initialize_ToastStateEmpty3',
     function() {
-      // Tests the case when toast state is empty and NOTIFICATION_CARDS_URL is
-      // set, and getAuthToken succeeds. In this case, the function should
-      // invoke showWelcomeToast().
+      // Tests the case when NOTIFICATION_CARDS_URL is set, getAuthToken
+      // succeeds, and the user has never responded to the toast.
+      // In this case, the function should invoke showWelcomeToast().
 
       // Setup and expectations.
-      var testToastState = {};
       NOTIFICATION_CARDS_URL = 'https://some.server.url.com';
       var testIdentityToken = 'some identity token';
+      var testGeolocationPref = false;
+      var testUserRespondedToToast = {};
 
       mockInitializeDependencies(this);
 
       this.mockGlobals.expects(once()).recordEvent(
           DiagnosticEvent.EXTENSION_START);
-      var storageGetSavedArgs = new SaveMockArguments();
-      this.mockApis.expects(once()).
-          storage_get(
-              storageGetSavedArgs.match(eq('toastState')),
-              storageGetSavedArgs.match(ANYTHING)).
-          will(invokeCallback(storageGetSavedArgs, 1, testToastState));
-      var chromeIdentityGetAuthTokenSavedArgs = new SaveMockArguments();
-      this.mockApis.expects(once()).
-          chrome_identity_getAuthToken(
-              chromeIdentityGetAuthTokenSavedArgs.match(
-                  eqJSON({interactive: false})),
-              chromeIdentityGetAuthTokenSavedArgs.match(ANYTHING)).
-          will(invokeCallback(
-              chromeIdentityGetAuthTokenSavedArgs, 1, testIdentityToken));
+
+      expectStateMachineCalls(
+          this.mockApis,
+          testIdentityToken,
+          testGeolocationPref,
+          testUserRespondedToToast);
+
+      var chromeNotificationGetAllSavedArgs = new SaveMockArguments();
+      this.mockApis.expects(exactly(2)).
+          chrome_notifications_getAll(
+              chromeNotificationGetAllSavedArgs.match(ANYTHING)).
+          will(
+              invokeCallback(chromeNotificationGetAllSavedArgs, 0, {}),
+              invokeCallback(chromeNotificationGetAllSavedArgs, 0, {}));
+
       this.mockGlobals.expects(once()).showWelcomeToast();
 
       // Invoking the tested function.
@@ -155,46 +218,72 @@ TEST_F(
     });
 
 TEST_F('GoogleNowBackgroundUnitTest', 'Initialize_ToastStateYes', function() {

[vadimt, 2013/07/25 20:48:01]

The name and comment seem out of date. We don't store Yes vs. No result.

[robliao, 2013/07/25 22:35:48]

The comment is still valid. I've updated the name.

We store the Yes vs. No answer in Geolocation Prefs in this case.
On 2013/07/25 20:48:01, vadimt wrote:

[vadimt, 2013/07/26 01:00:37]

The name and comment are still confusing. The user may have just  manually
checked Geolocation box after answering No or not answering at all. Why not say
in name/comments that this is about geolocation box state, not about what user
answered in the past?

[robliao, 2013/07/26 01:12:17]

This is the core user scenario we care about with the toast.

I went ahead and changed the name.
On 2013/07/26 01:00:37, vadimt wrote:

-  // Tests the case when the user has answered "yes" to the toast in the past.
-  // In this case, the function should invoke startPollingCards().
+    // Tests the case when the user has answered "yes" to the toast in the past.
+    // In this case, the function should invoke startPollingCards().
 
-  // Setup and expectations.
-  var testToastState = {toastState: ToastOptionResponse.CHOSE_YES};
+    // Setup and expectations.
+    NOTIFICATION_CARDS_URL = 'https://some.server.url.com';
+    var testIdentityToken = 'some identity token';
+    var testGeolocationPref = true;
+    var testUserRespondedToToast = {userRespondedToToast: true};
 
-  mockInitializeDependencies(this);
+    mockInitializeDependencies(this);
 
-  this.mockGlobals.expects(once()).recordEvent(DiagnosticEvent.EXTENSION_START);
-  var storageGetSavedArgs = new SaveMockArguments();
-  this.mockApis.expects(once()).
-      storage_get(
-          storageGetSavedArgs.match(eq('toastState')),
-          storageGetSavedArgs.match(ANYTHING)).
-      will(invokeCallback(storageGetSavedArgs, 1, testToastState));
-  this.mockGlobals.expects(once()).startPollingCards();
+    this.mockGlobals.expects(once()).recordEvent(
+          DiagnosticEvent.EXTENSION_START);
+
+    expectStateMachineCalls(
+        this.mockApis,
+        testIdentityToken,
+        testGeolocationPref,
+        testUserRespondedToToast);
+
+    var chromeNotificationGetAllSavedArgs = new SaveMockArguments();
+    this.mockApis.expects(exactly(2)).
+        chrome_notifications_getAll(
+            chromeNotificationGetAllSavedArgs.match(ANYTHING)).
+        will(
+            invokeCallback(chromeNotificationGetAllSavedArgs, 0, {}),
+            invokeCallback(chromeNotificationGetAllSavedArgs, 0, {}));
 
-  // Invoking the tested function.
-  initialize();
+    this.mockGlobals.expects(once()).startPollingCards();
+
+    // Invoking the tested function.
+    initialize();
 });
 
 TEST_F('GoogleNowBackgroundUnitTest', 'Initialize_ToastStateNo', function() {
-  // Tests the case when the user has answered "no" to the toast in the past.
-  // In this case, the function should do nothing.
+    // Tests the case when the user has answered "no" to the toast in the past.
+    // In this case, the function should do nothing.
 
-  // Setup and expectations.
-  var testToastState = {toastState: ToastOptionResponse.CHOSE_NO};
+    // Setup and expectations.
+    // Setup and expectations.
+    NOTIFICATION_CARDS_URL = 'https://some.server.url.com';
+    var testIdentityToken = 'some identity token';
+    var testGeolocationPref = false;
+    var testUserRespondedToToast = {userRespondedToToast: true};
 
-  mockInitializeDependencies(this);
+    mockInitializeDependencies(this);
 
-  this.mockGlobals.expects(once()).recordEvent(DiagnosticEvent.EXTENSION_START);
-  var storageGetSavedArgs = new SaveMockArguments();
-  this.mockApis.expects(once()).
-      storage_get(
-          storageGetSavedArgs.match(eq('toastState')),
-          storageGetSavedArgs.match(ANYTHING)).
-      will(invokeCallback(storageGetSavedArgs, 1, testToastState));
+    this.mockGlobals.expects(once()).recordEvent(
+          DiagnosticEvent.EXTENSION_START);
 
-  // Invoking the tested function.
-  initialize();
+    expectStateMachineCalls(
+      this.mockApis,
+      testIdentityToken,
+      testGeolocationPref,
+      testUserRespondedToToast);
+
+    var chromeNotificationGetAllSavedArgs = new SaveMockArguments();
+    this.mockApis.expects(exactly(2)).
+        chrome_notifications_getAll(
+            chromeNotificationGetAllSavedArgs.match(ANYTHING)).
+        will(
+            invokeCallback(chromeNotificationGetAllSavedArgs, 0, {}),
+            invokeCallback(chromeNotificationGetAllSavedArgs, 0, {}));
+
+    // Invoking the tested function.
+    initialize();
 });
 
 /**