Restored an option to enable enhanced bookmarks experiment from Finch

chrome/browser/bookmarks/enhanced_bookmarks_features.cc

 // Copyright 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.
 
 #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"
 
 namespace {
 
 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) {
   ListPrefUpdate update(local_state, prefs::kEnabledLabsExperiments);
   base::ListValue* experiments_list = update.Get();
   size_t index;
   if (bookmarks_experiment_state == kNoBookmarksExperiment) {
     experiments_list->Remove(
         base::StringValue(switches::kManualEnhancedBookmarks), &index);
     experiments_list->Remove(
@@ skipping 12 matching lines @@
   }
 }
 
 bool IsEnhancedBookmarksExperimentEnabled() {
   CommandLine* command_line = CommandLine::ForCurrentProcess();
   if (command_line->HasSwitch(switches::kManualEnhancedBookmarks) ||
       command_line->HasSwitch(switches::kManualEnhancedBookmarksOptout)) {
     return true;
   }
 
+  return IsEnhancedBookmarksExperimentEnabledFromFinch();
+}
+
+bool IsEnhancedBookmarksExperimentEnabledFromFinch() {
   std::string ext_id = GetEnhancedBookmarksExtensionIdFromFinch();
   extensions::FeatureProvider* feature_provider =
       extensions::FeatureProvider::GetPermissionFeatures();
   extensions::Feature* feature = feature_provider->GetFeature("metricsPrivate");
   return feature && feature->IsIdInWhitelist(ext_id);
 }
 
 bool IsEnableDomDistillerSet() {
   if (CommandLine::ForCurrentProcess()->
       HasSwitch(switches::kEnableDomDistiller)) {
@@ skipping 14 matching lines @@
   if (chrome_variations::GetVariationParamValue(
           kFieldTrialName, "enable-sync-articles") == "1")
     return true;
 
   return false;
 }
 
 std::string GetEnhancedBookmarksExtensionIdFromFinch() {
   return chrome_variations::GetVariationParamValue(kFieldTrialName, "id");
 }