Add Finch Checks to the State Machine

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 691 matching lines @@
 function setShouldPollCards(shouldPollCardsRequest, onSuccess) {
   tasks.debugSetStepName(
         'setShouldRun-shouldRun-updateCardsAttemptsIsRunning');
   updateCardsAttempts.isRunning(function(currentValue) {
     if (shouldPollCardsRequest != currentValue) {
       console.log('Action Taken setShouldPollCards=' + shouldPollCardsRequest);
       if (shouldPollCardsRequest)
         startPollingCards();
       else
         stopPollingCards();
+    } else {
+      console.log(
+          'Action Ignored setShouldPollCards=' + shouldPollCardsRequest);
     }
     onSuccess();
   });
 }
 
 /**
  * Shows or hides the toast.
  * @param {boolean} visibleRequest true to show the toast and
  *     false to hide the toast.
  * @param {function} onSuccess Called on completion.
  */
 function setToastVisible(visibleRequest, onSuccess) {
   tasks.debugSetStepName(
       'setToastVisible-shouldSetToastVisible-getAllNotifications');
   chrome.notifications.getAll(function(notifications) {
     // TODO(vadimt): Figure out what to do when notifications are disabled for
     // our extension.
     notifications = notifications || {};
 
     if (visibleRequest != !!notifications[WELCOME_TOAST_NOTIFICATION_ID]) {
       console.log('Action Taken setToastVisible=' + visibleRequest);
       if (visibleRequest)
         showWelcomeToast();
       else
         hideWelcomeToast();
+    } else {
+      console.log('Action Ignored setToastVisible=' + visibleRequest);
     }
 
     onSuccess();
   });
 }
 
 /**
+ * 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} onSuccess Called on completion.
+ */
+function setBackgroundEnable(backgroundEnable, onSuccess) {

[vadimt, 2013/08/02 21:54:43]

onSuccess -> callback

[robliao, 2013/08/03 01:21:18]

Kept this way to remain consistent with the above. We can change this in another
CL.
On 2013/08/02 21:54:43, vadimt wrote:

+  chrome.permissions.contains({permissions: ['background']},

[vadimt, 2013/08/02 21:54:43]

Please instrument new APIs.

[robliao, 2013/08/03 01:21:18]

Done.

+      function(hasPermission) {
+        if (backgroundEnable != hasPermission) {
+          console.log('Action Taken setBackgroundEnable=' + backgroundEnable);
+          if (backgroundEnable)
+            chrome.permissions.request(
+                {permissions: ['background']},
+                function() {
+                  onSuccess();
+                });
+          else
+            chrome.permissions.remove(
+                {permissions: ['background']},
+                function() {
+                  onSuccess();
+                });
+        } else {
+          console.log('Action Ignored setBackgroundEnable=' + backgroundEnable);
+          onSuccess();
+        }
+      });
+}
+
+/**
  * 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.
  * @param {boolean} geolocationEnabled true if
  *     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.

[vadimt, 2013/08/02 21:54:43]

Actually, we need a flag that conditionally turns on background mode while we
are polling. So, we may have 2 states: polling & background, polling &
!background.

[robliao, 2013/08/03 01:21:18]

We will want to update the doc:
S & L => poll + background permission; TS = true; no showing toast

Do we want to only set this on responding to the toast?
On 2013/08/02 21:54:43, vadimt wrote:

[vadimt, 2013/08/03 02:07:53]

The ultimate goal is to implement as described in the doc.
However, we want to first run the experiment to make sure that background mode
is safe. Then, presumably, we'll switch all users to background mode.
So, I see this conditional background mode as a temporary technical detail that
doesn't need to be documented. However, I've added this as a note in the doc.

[robliao, 2013/08/09 21:46:44]

Clarified with
Modes without Background:
EnableWithoutBackground

Modes with Background
EnableWithBackground
EnabledViaFlag
''

On 2013/08/03 02:07:53, vadimt wrote:

  * @param {function()} callback Call this function on completion.
  */
 function updateRunningState(
     signedIn,
     geolocationEnabled,
     userRespondedToToast,
+    enableExperiment,
     callback) {

[vadimt, 2013/08/02 21:54:43]

Are you going to get rid of callback and debugSetStepName in a separate CL?
Sorry if I already asked this.

[robliao, 2013/08/03 01:21:18]

That's the plan. I didn't know if you were doing this or I was. I can set this
up.
On 2013/08/02 21:54:43, vadimt wrote:

[vadimt, 2013/08/03 02:07:53]

Please do this only for your task. I'll take care of others.

[robliao, 2013/08/09 21:46:44]

Done. There was no debugSetStepName added in.
On 2013/08/03 02:07:53, vadimt wrote:

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

OK :), but please plan a separate CL where you'll get rid of debugSetStepName
and callback ONLY IN STATE_CHANGED_TASK_NAME functions.

 
   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,
         // the user has implicitly responded to the toast.
         // We do not want to show it again.
         storage.set({userRespondedToToast: true});
       }
 
       shouldPollCards = true;
+      shouldSetBackground = true;
     } else {
       if (userRespondedToToast) {
         recordEvent(GoogleNowEvent.USER_SUPPRESSED);
       } else {
         shouldSetToastVisible = true;
       }
     }
   } else {
     recordEvent(GoogleNowEvent.STOPPED);
   }
 
   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);
+    });
   });
 }
 
 /**
  * Coordinates the behavior of Google Now for Chrome depending on
  * Chrome and extension state.
  */
 function onStateChange() {
   tasks.add(STATE_CHANGED_TASK_NAME, function(callback) {
     tasks.debugSetStepName('onStateChange-isSignedIn');
     authenticationManager.isSignedIn(function(token) {
       var signedIn = !!token && !!NOTIFICATION_CARDS_URL;
-      tasks.debugSetStepName(
-          'onStateChange-get-googleGeolocationAccessEnabledPref');
-      googleGeolocationAccessEnabledPref.get({}, function(prefValue) {
-        var geolocationEnabled = !!prefValue.value;
+      chrome.metricsPrivate.getFieldTrial('GoogleNow', function(response) {

[vadimt, 2013/08/02 21:54:43]

How did you test this?

[robliao, 2013/08/03 01:21:18]

Tested with my personal finch server.
On 2013/08/02 21:54:43, vadimt wrote:

+        // '' means we were enabled via the flags but not the experiment.

[vadimt, 2013/08/02 21:54:43]

More precisely:
// '' means we were enabled via the flags, but the experiment is not running.

Explanation:
If experiment is running, turning on the flag will force EnableWithFlag group,
otherwise, the value will be ''.

ALSO:
Please modify C++ code that looks at the experiment code per that email.

[robliao, 2013/08/03 01:21:18]

Done.

+        var enableExperiment = (response == 'EnableWithBackground' ||
+                               (response == 'EnableWithFlag') ||
+                               (response == ''));
         tasks.debugSetStepName(
-          'onStateChange-get-userRespondedToToast');
-        storage.get('userRespondedToToast', function(items) {
-          var userRespondedToToast = !!items.userRespondedToToast;
-          updateRunningState(
-              signedIn,
-              geolocationEnabled,
-              userRespondedToToast,
-              callback);
+            'onStateChange-get-googleGeolocationAccessEnabledPref');
+        googleGeolocationAccessEnabledPref.get({}, function(prefValue) {
+          var geolocationEnabled = !!prefValue.value;
+          tasks.debugSetStepName(
+            'onStateChange-get-userRespondedToToast');
+          storage.get('userRespondedToToast', function(items) {
+            var userRespondedToToast = !!items.userRespondedToToast;
+            updateRunningState(
+                signedIn,
+                geolocationEnabled,
+                userRespondedToToast,
+                enableExperiment,
+                callback);
+          });
         });
       });
     });
   });
 }
 
 /**
  * Displays a toast to the user asking if they want to opt in to receiving
  * Google Now cards.
  */
@@ skipping 74 matching lines @@
 
 chrome.location.onLocationUpdate.addListener(function(position) {
   recordEvent(GoogleNowEvent.LOCATION_UPDATE);
   updateNotificationsCards(position);
 });
 
 chrome.omnibox.onInputEntered.addListener(function(text) {
   localStorage['server_url'] = NOTIFICATION_CARDS_URL = text;
   initialize();
 });