Restored an option to enable enhanced bookmarks experiment from Finch

chrome/browser/bookmarks/enhanced_bookmarks_features.cc

Index: chrome/browser/bookmarks/enhanced_bookmarks_features.cc
diff --git a/chrome/browser/bookmarks/enhanced_bookmarks_features.cc b/chrome/browser/bookmarks/enhanced_bookmarks_features.cc
index e18f18f7dbe637a817560ef643d0c8dd6d4e1176..60e05f098ab9da7b8847ca76157c007777b980bb 100644
--- a/chrome/browser/bookmarks/enhanced_bookmarks_features.cc
+++ b/chrome/browser/bookmarks/enhanced_bookmarks_features.cc
@@ -5,12 +5,14 @@
 #include "chrome/browser/bookmarks/enhanced_bookmarks_features.h"
 
 #include "base/command_line.h"
+#include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
 #include "base/prefs/scoped_user_pref_update.h"
 #include "base/sha1.h"
 #include "base/strings/string_number_conversions.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
+#include "components/sync_driver/pref_names.h"
 #include "components/variations/variations_associated_data.h"
 #include "extensions/common/features/feature.h"
 #include "extensions/common/features/feature_provider.h"
@@ -21,6 +23,89 @@ const char kFieldTrialName[] = "EnhancedBookmarks";
 
 };  // namespace
 
+bool IsBookmarksExperimentEnabled(PrefService* user_prefs,

[not at google - send to devlin, 2014/05/06 21:35:06]

I mean that there should be no UpdateBookmarksExperimentState, just
IsBookmarksExperimentState, and that takes into account all of these factors
like finch. There shouldn't be any need to write to prefs from here at all, only
read.

[yefimt, 2014/05/06 22:02:39]

It cannot be just IsBookmarksExperimentState, because it does some changes, for
example sets command line flags to force finch experiment. So it will be ide
effecxt for IsXXX function

On 2014/05/06 21:35:06, kalman wrote:

+    std::string* extension_id) {
+  BookmarksExperimentState bookmarks_experiment_state =
+      static_cast<BookmarksExperimentState>(user_prefs->GetInteger(
+          sync_driver::prefs::kEnhancedBookmarksExperimentEnabled));
+  if (bookmarks_experiment_state == kBookmarksExperimentEnabledFromFinch) {
+    *extension_id = GetEnhancedBookmarksExtensionIdFromFinch();
+    return true;
+  } else if (bookmarks_experiment_state == kBookmarksExperimentEnabled) {
+    *extension_id = user_prefs->GetString(
+        sync_driver::prefs::kEnhancedBookmarksExtensionId);
+    return true;
+  }
+
+  return false;
+}
+
+void UpdateBookmarksExperimentState(
+    PrefService* user_prefs, PrefService* local_state, bool user_signed_in) {
+  BookmarksExperimentState bookmarks_experiment_state_before =
+      static_cast<BookmarksExperimentState>(user_prefs->GetInteger(
+          sync_driver::prefs::kEnhancedBookmarksExperimentEnabled));
+  // If user signed out, clear possible previous state.
+  if (!user_signed_in)
+    bookmarks_experiment_state_before = kNoBookmarksExperiment;
+
+  // kEnhancedBookmarksExperiment flag could have values "", "1" and "0".
+  // "0" - user opted out.
+  bool opt_out = CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
+      switches::kEnhancedBookmarksExperiment) == "0";
+
+  BookmarksExperimentState bookmarks_experiment_new_state =
+      kNoBookmarksExperiment;
+  bool enabled_from_finch = false;
+  if (IsEnhancedBookmarksExperimentEnabledFromFinch()) {
+    enabled_from_finch = !user_signed_in;
+    if (user_signed_in) {
+      bookmarks_experiment_new_state =
+          kBookmarksExperimentEnabledFromFinchUserSignedIn;
+    }
+  }
+
+  if (enabled_from_finch) {
+    if (opt_out) {
+      // Experiment enabled but user opted out.
+      bookmarks_experiment_new_state = kBookmarksExperimentOptOutFromFinch;
+    } else {
+      // Experiment enabled.
+      bookmarks_experiment_new_state = kBookmarksExperimentEnabledFromFinch;
+    }
+  } else if (bookmarks_experiment_state_before == kBookmarksExperimentEnabled) {
+    if (opt_out) {
+      // Experiment enabled but user opted out.
+      bookmarks_experiment_new_state = kBookmarksExperimentEnabledUserOptOut;
+    } else {
+      bookmarks_experiment_new_state = kBookmarksExperimentEnabled;
+    }
+  } else if (bookmarks_experiment_state_before ==
+             kBookmarksExperimentEnabledUserOptOut) {
+    if (opt_out) {
+      bookmarks_experiment_new_state = kBookmarksExperimentEnabledUserOptOut;
+    } else {
+      // User opted in again.
+      bookmarks_experiment_new_state = kBookmarksExperimentEnabled;
+    }
+  } else {
+    // Experiment disabled.
+    user_prefs->ClearPref(
+        sync_driver::prefs::kEnhancedBookmarksExperimentEnabled);
+    user_prefs->ClearPref(
+        sync_driver::prefs::kEnhancedBookmarksExtensionId);
+  }
+
+  UMA_HISTOGRAM_ENUMERATION("EnhancedBookmarks.SyncExperimentState",
+                            bookmarks_experiment_new_state,
+                            kBookmarksExperimentEnumSize);
+  user_prefs->SetInteger(
+      sync_driver::prefs::kEnhancedBookmarksExperimentEnabled,
+      bookmarks_experiment_new_state);
+  if (bookmarks_experiment_state_before != bookmarks_experiment_new_state)
+    UpdateBookmarksExperiment(local_state, bookmarks_experiment_new_state);
+}
+
 void UpdateBookmarksExperiment(
     PrefService* local_state,
     BookmarksExperimentState bookmarks_experiment_state) {
@@ -53,6 +138,10 @@ bool IsEnhancedBookmarksExperimentEnabled() {
     return true;
   }
 
+  return IsEnhancedBookmarksExperimentEnabledFromFinch();
+}
+
+bool IsEnhancedBookmarksExperimentEnabledFromFinch() {
   std::string ext_id = GetEnhancedBookmarksExtensionIdFromFinch();
   extensions::FeatureProvider* feature_provider =
       extensions::FeatureProvider::GetPermissionFeatures();