Add Finch Checks to the State Machine

chrome/browser/resources/google_now/background.js

Index: chrome/browser/resources/google_now/background.js
diff --git a/chrome/browser/resources/google_now/background.js b/chrome/browser/resources/google_now/background.js
index d1d896aeac47868d8d22319306c5269cb8e01df8..94f31669ed15bb27b9af48e7ef0b99f8202ef809 100644
--- a/chrome/browser/resources/google_now/background.js
+++ b/chrome/browser/resources/google_now/background.js
@@ -118,6 +118,7 @@ var tasks = buildTaskManager(areTasksConflicting);
 
 // Add error processing to API calls.
 tasks.instrumentChromeApiFunction('location.onLocationUpdate.addListener', 0);
+tasks.instrumentChromeApiFunction('metricsPrivate.getFieldTrial', 1);
 tasks.instrumentChromeApiFunction('notifications.create', 2);
 tasks.instrumentChromeApiFunction('notifications.update', 2);
 tasks.instrumentChromeApiFunction('notifications.getAll', 0);
@@ -132,6 +133,9 @@ tasks.instrumentChromeApiFunction(
 tasks.instrumentChromeApiFunction(
     'preferencesPrivate.googleGeolocationAccessEnabled.onChange.addListener',
     0);
+tasks.instrumentChromeApiFunction('permissions.contains', 1);
+tasks.instrumentChromeApiFunction('permissions.remove', 1);
+tasks.instrumentChromeApiFunction('permissions.request', 1);
 tasks.instrumentChromeApiFunction('runtime.onInstalled.addListener', 0);
 tasks.instrumentChromeApiFunction('runtime.onStartup.addListener', 0);
 tasks.instrumentChromeApiFunction('tabs.create', 1);
@@ -753,6 +757,36 @@ function setToastVisible(visibleRequest, callback) {
 }
 
 /**
+ * Enables or disables the Google Now background permission.
+ * @param {boolean} backgroundEnable true to run in the background.
+ *     false to not run in the background.
+ * @param {function} callback Called on completion.
+ */
+function setBackgroundEnable(backgroundEnable, callback) {
+  instrumented.permissions.contains({permissions: ['background']},
+      function(hasPermission) {
+        if (backgroundEnable != hasPermission) {
+          console.log('Action Taken setBackgroundEnable=' + backgroundEnable);
+          if (backgroundEnable)
+            instrumented.permissions.request(
+                {permissions: ['background']},
+                function() {
+                  callback();
+                });
+          else
+            instrumented.permissions.remove(
+                {permissions: ['background']},
+                function() {
+                  callback();
+                });
+        } else {
+          console.log('Action Ignored setBackgroundEnable=' + backgroundEnable);
+          callback();
+        }
+      });
+}
+
+/**
  * Does the actual work of deciding what Google Now should do
  * based off of the current state of Chrome.
  * @param {boolean} signedIn true if the user is signed in.
@@ -760,23 +794,31 @@ function setToastVisible(visibleRequest, callback) {
  *     the geolocation option is enabled.
  * @param {boolean} userRespondedToToast true if
  *     the user has responded to the toast.
+ * @param {boolean} enableExperiment true if
+ *     this extension should be running.
+ * @param {boolean} enableBackground true if
+ *     the background permission should be requested.
  * @param {function()} callback Call this function on completion.
  */
 function updateRunningState(
     signedIn,
     geolocationEnabled,
     userRespondedToToast,
+    enableExperiment,

[vadimt, 2013/08/09 22:13:39]

You are not using this param, so you can remove it.

[robliao, 2013/08/09 22:16:47]

This param is used to gate the starting of Google Now.
On 2013/08/09 22:13:39, vadimt wrote:

[vadimt, 2013/08/09 22:21:27]

Ah, correct.
Still, you don't need this.
The gating happens in C++ code, where you check that the group prefix is
Enabled. You don't need to gate once again here.
I hope this also addresses the other note.

[robliao, 2013/08/09 22:24:40]

If we're comfortable that once we load the extension code we're supposed to be
here, then we can just check the background mode.
On 2013/08/09 22:21:27, vadimt wrote:

[vadimt, 2013/08/09 22:28:26]

Finch parameters don't change while Chrome is running. They get updated on
startup. There are no plans to change this. Rather, they consider forcefully
restarting Chrome if needed.

[robliao, 2013/08/09 22:37:40]

Done.

+    enableBackground,
     callback) {
 
   console.log(
       'State Update signedIn=' + signedIn + ' ' +
       'geolocationEnabled=' + geolocationEnabled + ' ' +
-      'userRespondedToToast=' + userRespondedToToast);
+      'userRespondedToToast=' + userRespondedToToast + ' ' +
+      'enableExperiment=' + enableExperiment);
 
   var shouldSetToastVisible = false;
   var shouldPollCards = false;
+  var shouldSetBackground = false;
 
-  if (signedIn) {
+  if (signedIn && enableExperiment) {
     if (geolocationEnabled) {
       if (!userRespondedToToast) {
         // If the user enabled geolocation independently of Google Now,
@@ -785,6 +827,9 @@ function updateRunningState(
         chrome.storage.local.set({userRespondedToToast: true});
       }
 
+      if (enableBackground)
+        shouldSetBackground = true;
+
       shouldPollCards = true;
     } else {
       if (userRespondedToToast) {
@@ -798,11 +843,14 @@ function updateRunningState(
   }
 
   console.log(
-      'Requested Actions setToastVisible=' + shouldSetToastVisible + ' ' +
+      'Requested Actions shouldSetBackground=' + shouldSetBackground + ' ' +
+      'setToastVisible=' + shouldSetToastVisible + ' ' +
       'setShouldPollCards=' + shouldPollCards);
 
-  setToastVisible(shouldSetToastVisible, function() {
-    setShouldPollCards(shouldPollCards, callback);
+  setBackgroundEnable(shouldSetBackground, function() {
+    setToastVisible(shouldSetToastVisible, function() {
+      setShouldPollCards(shouldPollCards, callback);
+    });
   });
 }
 
@@ -815,24 +863,37 @@ function onStateChange() {
     tasks.debugSetStepName('onStateChange-isSignedIn');
     authenticationManager.isSignedIn(function(token) {
       var signedIn = !!token && !!NOTIFICATION_CARDS_URL;
-      tasks.debugSetStepName(
-          'onStateChange-get-googleGeolocationAccessEnabledPref');
-      instrumented.
-          preferencesPrivate.
-          googleGeolocationAccessEnabled.
-          get({}, function(prefValue) {
-            var geolocationEnabled = !!prefValue.value;
+      instrumented.metricsPrivate.getFieldTrial(
+          'GoogleNow',
+          function(response) {
+            // '' means we were enabled via the flags, but
+            // the experiment is not running.
+            console.log('Experiment Status: ' + response);
+            var enableExperiment = (response.substring(0, 6) == 'Enable') ||

[vadimt, 2013/08/09 22:13:39]

You don't need this. Simply write:
var enableBackground = response != 'EnableWithoutBackground';

(Or wait till I land variation params API and use it; your choice)

[robliao, 2013/08/09 22:16:47]

We want to enable the experiment even with EnableWithoutBackground.
Your proposal would cause that status to not be in the experiment.
On 2013/08/09 22:13:39, vadimt wrote:

+                                   (response == '');
+            var enableBackground = enableExperiment &&
+                                   (response != 'EnableWithoutBackground');
             tasks.debugSetStepName(
-              'onStateChange-get-userRespondedToToast');
-            instrumented.storage.local.get(
-                'userRespondedToToast',
-                function(items) {
-                  var userRespondedToToast = !!items.userRespondedToToast;
-                  updateRunningState(
-                      signedIn,
-                      geolocationEnabled,
-                      userRespondedToToast,
-                      callback);
+                'onStateChange-get-googleGeolocationAccessEnabledPref');
+            instrumented.
+                preferencesPrivate.
+                googleGeolocationAccessEnabled.
+                get({}, function(prefValue) {
+                  var geolocationEnabled = !!prefValue.value;
+                  tasks.debugSetStepName(
+                    'onStateChange-get-userRespondedToToast');
+                  instrumented.storage.local.get(
+                      'userRespondedToToast',
+                      function(items) {
+                        var userRespondedToToast = !!items.userRespondedToToast;
+                        updateRunningState(
+                            signedIn,
+                            geolocationEnabled,
+                            userRespondedToToast,
+                            enableExperiment,
+                            enableBackground,
+                            callback);
+                      });
                 });
           });
     });