Consolidated enhanced bookmarks experiment logic in one place.

BUG=

Review URL: https://codereview.chromium.org/291153012

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@272730 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/bookmarks/enhanced_bookmarks_features.cc

@@ -55,9 +55,11 @@ bool GetBookmarksExperimentExtensionID(const PrefService* user_prefs,
   return false;
 }
 
-void UpdateBookmarksExperimentState(PrefService* user_prefs,
-                                    PrefService* local_state,
-                                    bool user_signed_in) {
+void UpdateBookmarksExperimentState(
+    PrefService* user_prefs,
+    PrefService* local_state,
+    bool user_signed_in,
+    BookmarksExperimentState experiment_enabled_from_sync) {
  PrefService* flags_storage = local_state;
 #if defined(OS_CHROMEOS)
   // Chrome OS is using user prefs for flags storage.
@@ -80,16 +82,8 @@ void UpdateBookmarksExperimentState(PrefService* user_prefs,
 
   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 (IsEnhancedBookmarksExperimentEnabledFromFinch() && !user_signed_in) {
     if (opt_out) {
       // Experiment enabled but user opted out.
       bookmarks_experiment_new_state = kBookmarksExperimentOptOutFromFinch;
@@ -97,6 +91,18 @@ void UpdateBookmarksExperimentState(PrefService* user_prefs,
       // Experiment enabled.
       bookmarks_experiment_new_state = kBookmarksExperimentEnabledFromFinch;
     }
+  } else if (experiment_enabled_from_sync == kBookmarksExperimentEnabled) {
+    // Experiment enabled from Chrome sync.
+    if (opt_out) {
+      // Experiment enabled but user opted out.
+      bookmarks_experiment_new_state = kBookmarksExperimentEnabledUserOptOut;
+    } else {
+      // Experiment enabled.
+      bookmarks_experiment_new_state = kBookmarksExperimentEnabled;
+    }
+  } else if (experiment_enabled_from_sync == kNoBookmarksExperiment) {
+    // Experiment is not enabled from Chrome sync.
+    bookmarks_experiment_new_state = kNoBookmarksExperiment;
   } else if (bookmarks_experiment_state_before == kBookmarksExperimentEnabled) {
     if (opt_out) {
       // Experiment enabled but user opted out.
@@ -120,9 +126,8 @@ void UpdateBookmarksExperimentState(PrefService* user_prefs,
   user_prefs->SetInteger(
       sync_driver::prefs::kEnhancedBookmarksExperimentEnabled,
       bookmarks_experiment_new_state);
-  if (bookmarks_experiment_state_before != bookmarks_experiment_new_state)
-    ForceFinchBookmarkExperimentIfNeeded(flags_storage,
-                                         bookmarks_experiment_new_state);
+  ForceFinchBookmarkExperimentIfNeeded(flags_storage,
+                                       bookmarks_experiment_new_state);
 }
 
 void ForceFinchBookmarkExperimentIfNeeded(

chrome/browser/bookmarks/enhanced_bookmarks_features.h

@@ -21,6 +21,7 @@ enum BookmarksExperimentState {
   kBookmarksExperimentEnabledFromFinch,
   kBookmarksExperimentOptOutFromFinch,
   kBookmarksExperimentEnabledFromFinchUserSignedIn,
+  kBookmarksExperimentEnabledFromSyncUnknown,
   kBookmarksExperimentEnumSize
 };
 
@@ -31,9 +32,11 @@ bool GetBookmarksExperimentExtensionID(const PrefService* user_prefs,
 
 // Updates bookmark experiment state based on information from Chrome sync
 // and Finch experiments.
-void UpdateBookmarksExperimentState(PrefService* user_prefs,
-                                    PrefService* local_state,
-                                    bool user_signed_in);
+void UpdateBookmarksExperimentState(
+    PrefService* user_prefs,
+    PrefService* local_state,
+    bool user_signed_in,
+    BookmarksExperimentState experiment_enabled_from_sync);
 
 // Sets flag to opt-in user into Finch experiment.
 void ForceFinchBookmarkExperimentIfNeeded(

chrome/browser/extensions/external_component_loader.cc

@@ -42,7 +42,8 @@ void ExternalComponentLoader::StartLoading() {
 
   UpdateBookmarksExperimentState(profile_->GetPrefs(),
                                  g_browser_process->local_state(),
-                                 IsUserSignedin(profile_));
+                                 IsUserSignedin(profile_),
+                                 kBookmarksExperimentEnabledFromSyncUnknown);
   std::string ext_id;
   if (GetBookmarksExperimentExtensionID(profile_->GetPrefs(), &ext_id) &&
       !ext_id.empty()) {

chrome/browser/sync/profile_sync_service.cc

@@ -1037,44 +1037,18 @@ void ProfileSyncService::OnExperimentsChanged(
   profile()->GetPrefs()->SetBoolean(prefs::kInvalidationServiceUseGCMChannel,
                                     experiments.gcm_invalidations_enabled);
 
-  int bookmarks_experiment_state_before = profile_->GetPrefs()->GetInteger(
-      sync_driver::prefs::kEnhancedBookmarksExperimentEnabled);
-  // kEnhancedBookmarksExperiment flag could have values "", "1" and "0".
-  // "" and "1" means experiment is enabled.
-  if ((CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
-           switches::kEnhancedBookmarksExperiment) != "0")) {
-    profile_->GetPrefs()->SetInteger(
-        sync_driver::prefs::kEnhancedBookmarksExperimentEnabled,
-        experiments.enhanced_bookmarks_enabled ? kBookmarksExperimentEnabled
-                                               : kNoBookmarksExperiment);
+  if (experiments.enhanced_bookmarks_enabled) {
     profile_->GetPrefs()->SetString(
         sync_driver::prefs::kEnhancedBookmarksExtensionId,
         experiments.enhanced_bookmarks_ext_id);
   } else {
-    // User opt-out from chrome://flags
-    if (experiments.enhanced_bookmarks_enabled) {
-      profile_->GetPrefs()->SetInteger(
-          sync_driver::prefs::kEnhancedBookmarksExperimentEnabled,
-          kBookmarksExperimentEnabledUserOptOut);
-      // Keep extension id up-to-date in case will opt-in later.
-      profile_->GetPrefs()->SetString(
-          sync_driver::prefs::kEnhancedBookmarksExtensionId,
-          experiments.enhanced_bookmarks_ext_id);
-    } else {
-      profile_->GetPrefs()->ClearPref(
-          sync_driver::prefs::kEnhancedBookmarksExperimentEnabled);
-      profile_->GetPrefs()->ClearPref(
-          sync_driver::prefs::kEnhancedBookmarksExtensionId);
-    }
-  }
-  BookmarksExperimentState bookmarks_experiment_state =
-      static_cast<BookmarksExperimentState>(profile_->GetPrefs()->GetInteger(
-          sync_driver::prefs::kEnhancedBookmarksExperimentEnabled));
-  // If bookmark experiment state was changed update about flags experiment.
-  if (bookmarks_experiment_state_before != bookmarks_experiment_state) {
-    ForceFinchBookmarkExperimentIfNeeded(g_browser_process->local_state(),
-                                         bookmarks_experiment_state);
+    profile_->GetPrefs()->ClearPref(
+        sync_driver::prefs::kEnhancedBookmarksExtensionId);
   }
+  UpdateBookmarksExperimentState(
+      profile_->GetPrefs(), g_browser_process->local_state(), true,
+      experiments.enhanced_bookmarks_enabled ? kBookmarksExperimentEnabled :
+                                               kNoBookmarksExperiment);
 
   // If this is a first time sync for a client, this will be called before
   // OnBackendInitialized() to ensure the new datatypes are available at sync