Enhanced bookmarks: added support for sync based experiment

BUG=321393

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

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

9 files changed, 65 insertions, 27 deletions

diff --git a/chrome/browser/bookmarks/enhanced_bookmarks_features.cc b/chrome/browser/bookmarks/enhanced_bookmarks_features.cc
index 777ac07..6b2bc9e 100644
--- a/chrome/browser/bookmarks/enhanced_bookmarks_features.cc
+++ b/chrome/browser/bookmarks/enhanced_bookmarks_features.cc
@@ -24,11 +24,8 @@ bool IsBookmarksExtensionHash(const std::string& sha1_hex) {
 
 };  // namespace
 
-bool OptInIntoBookmarksExperimentIfHasExtension(
+bool IsBookmarksExtensionInstalled(
     const extensions::ExtensionIdSet& extension_ids) {
-  if (base::FieldTrialList::FindFullName(kFieldTrialName) != "Default")
-    return false;
-
   // Compare installed extension ids with ones we expect.
   for (extensions::ExtensionIdSet::const_iterator iter = extension_ids.begin();
        iter != extension_ids.end(); ++iter) {
@@ -36,21 +33,24 @@ bool OptInIntoBookmarksExperimentIfHasExtension(
     DCHECK_EQ(id_hash.length(), base::kSHA1Length);
     std::string hash = base::HexEncode(id_hash.c_str(), id_hash.length());
 
-    if (IsBookmarksExtensionHash(hash)) {
-      // Enable features bookmarks depends on and opt-in user into Finch group
-      // for reporting.
-      CommandLine* command_line = CommandLine::ForCurrentProcess();
-      command_line->AppendSwitch(switches::kManualEnhancedBookmarks);
-      command_line->AppendSwitch(switches::kEnableSyncArticles);
-      command_line->AppendSwitch(switches::kEnableDomDistiller);
+    if (IsBookmarksExtensionHash(hash))
       return true;
-    }
   }
   return false;
 }
 
+bool OptInIntoBookmarksExperiment() {
+  if (base::FieldTrialList::FindFullName(kFieldTrialName) != "Default")
+    return false;
+
+  // Opt-in user into Finch group.
+  CommandLine* command_line = CommandLine::ForCurrentProcess();
+  command_line->AppendSwitch(switches::kManualEnhancedBookmarks);
+  return true;
+}
+
 bool IsEnhancedBookmarksExperimentEnabled() {
-  std::string ext_id = GetEnhancedBookmarksExtensionId();
+  std::string ext_id = GetEnhancedBookmarksExtensionIdFromFinch();
   extensions::FeatureProvider* feature_provider =
       extensions::FeatureProvider::GetPermissionFeatures();
   extensions::Feature* feature = feature_provider->GetFeature("metricsPrivate");
@@ -81,6 +81,6 @@ bool IsEnableSyncArticlesSet() {
   return false;
 }
 
-std::string GetEnhancedBookmarksExtensionId() {
+std::string GetEnhancedBookmarksExtensionIdFromFinch() {
   return chrome_variations::GetVariationParamValue(kFieldTrialName, "id");
 }
diff --git a/chrome/browser/bookmarks/enhanced_bookmarks_features.h b/chrome/browser/bookmarks/enhanced_bookmarks_features.h
index e7e4440..3a178d3 100644
--- a/chrome/browser/bookmarks/enhanced_bookmarks_features.h
+++ b/chrome/browser/bookmarks/enhanced_bookmarks_features.h
@@ -9,12 +9,14 @@
 
 #include "extensions/common/extension.h"
 
-// If user not in Finch experiment then check if extension was installed
-// manually and then opt-in user into experiment.
-// Returns true if user was opt-in.
-bool OptInIntoBookmarksExperimentIfHasExtension(
+// Returns true if Enhanced bookmarks extension is installed
+bool IsBookmarksExtensionInstalled(
     const extensions::ExtensionIdSet& extension_ids);
 
+// If user not in Finch experiment then opt-in user into experiment.
+// Returns true if user was opt-in.
+bool OptInIntoBookmarksExperiment();
+
 // Returns true if enhanced bookmarks experiment is enabled.
 bool IsEnhancedBookmarksExperimentEnabled();
 
@@ -25,6 +27,6 @@ bool IsEnableDomDistillerSet();
 bool IsEnableSyncArticlesSet();
 
 // Get extension id from Finch EnhancedBookmarks group parameters.
-std::string GetEnhancedBookmarksExtensionId();
+std::string GetEnhancedBookmarksExtensionIdFromFinch();
 
 #endif  // CHROME_BROWSER_BOOKMARKS_ENHANCED_BOOKMARKS_FEATURES_H_
diff --git a/chrome/browser/extensions/external_component_loader.cc b/chrome/browser/extensions/external_component_loader.cc
index 579a38b..50b0a3d 100644
--- a/chrome/browser/extensions/external_component_loader.cc
+++ b/chrome/browser/extensions/external_component_loader.cc
@@ -40,11 +40,21 @@ void ExternalComponentLoader::StartLoading() {
 
   if (CommandLine::ForCurrentProcess()->
           GetSwitchValueASCII(switches::kEnableEnhancedBookmarks) != "0") {
-    std::string ext_id = GetEnhancedBookmarksExtensionId();
+    std::string ext_id;
+    if (profile_->GetPrefs()->GetBoolean(
+            prefs::kEnhancedBookmarksExperimentEnabled)) {
+      ext_id =
+          profile_->GetPrefs()->GetString(prefs::kEnhancedBookmarksExtensionId);
+    } else {
+      ext_id = GetEnhancedBookmarksExtensionIdFromFinch();
+    }
     if (!ext_id.empty()) {
       prefs_->SetString(ext_id + ".external_update_url",
                         extension_urls::GetWebstoreUpdateUrl().spec());
     }
+  } else {
+    profile_->GetPrefs()->ClearPref(prefs::kEnhancedBookmarksExperimentEnabled);
+    profile_->GetPrefs()->ClearPref(prefs::kEnhancedBookmarksExtensionId);
   }
   LoadFinished();
 }
diff --git a/chrome/browser/extensions/external_component_loader.h b/chrome/browser/extensions/external_component_loader.h
index 02bb459..0312d5b 100644
--- a/chrome/browser/extensions/external_component_loader.h
+++ b/chrome/browser/extensions/external_component_loader.h
@@ -23,8 +23,6 @@ class ExternalComponentLoader : public ExternalLoader {
  public:
   explicit ExternalComponentLoader(Profile* profile);
 
-  static bool IsEnhancedBookmarksExperimentEnabled();
-
  protected:
   virtual void StartLoading() OVERRIDE;
 
diff --git a/chrome/browser/sync/profile_sync_components_factory_impl.cc b/chrome/browser/sync/profile_sync_components_factory_impl.cc
index 6d17cf4..dcb9d88 100644
--- a/chrome/browser/sync/profile_sync_components_factory_impl.cc
+++ b/chrome/browser/sync/profile_sync_components_factory_impl.cc
@@ -226,12 +226,14 @@ void ProfileSyncComponentsFactoryImpl::RegisterCommonDataTypes(
     pss->RegisterDataTypeController(
         new PasswordDataTypeController(this, profile_, pss));
   }
-  // Article sync is disabled by default.  Register only if explicitly enabled.
   ExtensionService* extension_service = extension_system_->extension_service();
   if (extension_service) {
-    OptInIntoBookmarksExperimentIfHasExtension(
-        extension_service->extensions()->GetIDs());
+    if (IsBookmarksExtensionInstalled(
+            extension_service->extensions()->GetIDs())) {
+      OptInIntoBookmarksExperiment();
+    }
   }
+  // Article sync is disabled by default.  Register only if explicitly enabled.
   if (IsEnableSyncArticlesSet()) {
     pss->RegisterDataTypeController(
         new UIDataTypeController(
diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc
index c6c8095..ad4b070 100644
--- a/chrome/browser/sync/profile_sync_service.cc
+++ b/chrome/browser/sync/profile_sync_service.cc
@@ -1077,6 +1077,8 @@ void ProfileSyncService::OnExperimentsChanged(
   if (current_experiments_.Matches(experiments))
     return;
 
+  current_experiments_ = experiments;
+
   // Handle preference-backed experiments first.
   if (experiments.gcm_channel_state != syncer::Experiments::UNSET) {
     profile()->GetPrefs()->SetBoolean(prefs::kGCMChannelEnabled,
@@ -1085,6 +1087,13 @@ void ProfileSyncService::OnExperimentsChanged(
     gcm::GCMProfileServiceFactory::GetForProfile(profile());
   }
 
+  if (experiments.enhanced_bookmarks_enabled) {
+    profile_->GetPrefs()->SetBoolean(prefs::kEnhancedBookmarksExperimentEnabled,
+                                     true);
+    profile_->GetPrefs()->SetString(prefs::kEnhancedBookmarksExtensionId,
+                                    experiments.enhanced_bookmarks_ext_id);
+  }
+
   // 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
   // setup. As a result, the migrator won't exist yet. This is fine because for
@@ -1129,8 +1138,6 @@ void ProfileSyncService::OnExperimentsChanged(
       OnMigrationNeededForTypes(to_register);
     }
   }
-
-  current_experiments_ = experiments;
 }
 
 void ProfileSyncService::UpdateAuthErrorState(const AuthError& error) {
diff --git a/chrome/browser/sync/sync_prefs.cc b/chrome/browser/sync/sync_prefs.cc
index eabbf00..7eb1c35 100644
--- a/chrome/browser/sync/sync_prefs.cc
+++ b/chrome/browser/sync/sync_prefs.cc
@@ -74,6 +74,17 @@ void SyncPrefs::RegisterProfilePrefs(
   RegisterDataTypePreferredPref(registry, syncer::BOOKMARKS, true);
   user_types.Remove(syncer::BOOKMARKS);
 
+  // These two prefs are set from sync experiment to enable enhanced bookmarks.
+  registry->RegisterBooleanPref(
+      prefs::kEnhancedBookmarksExperimentEnabled,
+      false,
+      user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
+
+  registry->RegisterStringPref(
+      prefs::kEnhancedBookmarksExtensionId,
+      std::string(),
+      user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
+
   // All types are set to off by default, which forces a configuration to
   // explicitly enable them. GetPreferredTypes() will ensure that any new
   // implicit types are enabled when their pref group is, or via
diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc
index 65a6fb2..2510f74 100644
--- a/chrome/common/pref_names.cc
+++ b/chrome/common/pref_names.cc
@@ -101,6 +101,12 @@ const char kRestoreStartupURLsMigrationTime[] =
 // data in the profile folder on disk but only in memory.
 const char kForceEphemeralProfiles[] = "profile.ephemeral_mode";
 
+// Set to true when enhanced bookmarks experiment is enabled via Chrome sync.
+const char kEnhancedBookmarksExperimentEnabled[] = "enhanced_bookmarks_enabled";
+
+// Enhanced bookmarks extension id passed via Chrome sync.
+const char kEnhancedBookmarksExtensionId[] = "enhanced_bookmarks_extension_id";
+
 // The application locale.
 // For OS_CHROMEOS we maintain kApplicationLocale property in both local state
 // and user's profile.  Global property determines locale of login screen,
diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h
index 739d6d9..9ec8b06 100644
--- a/chrome/common/pref_names.h
+++ b/chrome/common/pref_names.h
@@ -35,6 +35,8 @@ extern const char kURLsToRestoreOnStartup[];
 extern const char kURLsToRestoreOnStartupOld[];
 extern const char kRestoreStartupURLsMigrationTime[];
 extern const char kForceEphemeralProfiles[];
+extern const char kEnhancedBookmarksExperimentEnabled[];
+extern const char kEnhancedBookmarksExtensionId[];
 
 // For OS_CHROMEOS we maintain kApplicationLocale property in both local state
 // and user's profile.  Global property determines locale of login screen,