[Sync] Add favicon sync experiment

The old tab sync favicon experiment is deprecated. This patch updates the
strings to the fact that we now have a separate datatype, and hooks this
up to a new sync field.

BUG=154886


Review URL: https://chromiumcodereview.appspot.com/12700016

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

4 files changed, 39 insertions, 22 deletions

diff --git a/sync/internal_api/public/util/experiments.h b/sync/internal_api/public/util/experiments.h
index c13e56a..7194ab3 100644
--- a/sync/internal_api/public/util/experiments.h
+++ b/sync/internal_api/public/util/experiments.h
@@ -14,25 +14,23 @@ const char kKeystoreEncryptionFlag[] = "sync-keystore-encryption";
 const char kAutofillCullingTag[] = "autofill_culling";
 const char kFullHistorySyncTag[] = "history_delete_directives";
 const char kFullHistorySyncFlag[] = "full-history-sync";
+const char kFaviconSyncTag[] = "favicon_sync";
+const char kFaviconSyncFlag[] = "enable-sync-favicons";
 
 // A structure to hold the enable status of experimental sync features.
 struct Experiments {
-  Experiments() : sync_tab_favicons(false),
-                  keystore_encryption(false),
+  Experiments() : keystore_encryption(false),
                   autofill_culling(false),
-                  full_history_sync(false) {}
+                  full_history_sync(false),
+                  favicon_sync(false) {}
 
   bool Matches(const Experiments& rhs) {
-    return (sync_tab_favicons == rhs.sync_tab_favicons &&
-            keystore_encryption == rhs.keystore_encryption &&
+    return (keystore_encryption == rhs.keystore_encryption &&
             autofill_culling == rhs.autofill_culling &&
-            full_history_sync == rhs.full_history_sync);
+            full_history_sync == rhs.full_history_sync &&
+            favicon_sync == rhs.favicon_sync);
   }
 
-  // Enable syncing of favicons within tab sync (only has an effect if tab sync
-  // is already enabled). This takes effect on the next restart.
-  bool sync_tab_favicons;
-
   // Enable keystore encryption logic and the new encryption UI.
   bool keystore_encryption;
 
@@ -41,6 +39,9 @@ struct Experiments {
 
   // Enable full history sync (and history delete directives) for this client.
   bool full_history_sync;
+
+  // Enable the favicons sync datatypes (favicon images and favicon tracking).
+  bool favicon_sync;
 };
 
 }  // namespace syncer
diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc
index 352a57e..bd93a3c 100644
--- a/sync/internal_api/sync_manager_impl.cc
+++ b/sync/internal_api/sync_manager_impl.cc
@@ -1347,10 +1347,6 @@ bool SyncManagerImpl::ReceivedExperiment(Experiments* experiments) {
     return false;
   }
   bool found_experiment = false;
-  if (nigori_node.GetNigoriSpecifics().sync_tab_favicons()) {
-    experiments->sync_tab_favicons = true;
-    found_experiment = true;
-  }
 
   ReadNode keystore_node(&trans);
   if (keystore_node.InitByClientTagLookup(
@@ -1381,6 +1377,15 @@ bool SyncManagerImpl::ReceivedExperiment(Experiments* experiments) {
     found_experiment = true;
   }
 
+  ReadNode favicon_sync_node(&trans);
+  if (favicon_sync_node.InitByClientTagLookup(
+          syncer::EXPERIMENTS,
+          syncer::kFaviconSyncTag) == BaseNode::INIT_OK &&
+      favicon_sync_node.GetExperimentsSpecifics().favicon_sync().enabled()) {
+    experiments->favicon_sync = true;
+    found_experiment = true;
+  }
+
   return found_experiment;
 }
 
diff --git a/sync/protocol/experiments_specifics.proto b/sync/protocol/experiments_specifics.proto
index f40ed96..2779837 100644
--- a/sync/protocol/experiments_specifics.proto
+++ b/sync/protocol/experiments_specifics.proto
@@ -27,6 +27,11 @@ message AutofillCullingFlags {
   optional bool enabled = 1;
 }
 
+// Whether the favicon sync datatypes are enabled.
+message FaviconSyncFlags {
+  optional bool enabled = 1;
+}
+
 // Contains one flag or set of related flags.  Each node of the experiments type
 // will have a unique_client_tag identifying which flags it contains.  By
 // convention, the tag name should match the sub-message name.
@@ -34,4 +39,5 @@ message ExperimentsSpecifics {
   optional KeystoreEncryptionFlags keystore_encryption = 1;
   optional HistoryDeleteDirectives history_delete_directives = 2;
   optional AutofillCullingFlags autofill_culling = 3;
+  optional FaviconSyncFlags favicon_sync = 4;
 }
diff --git a/sync/protocol/proto_value_conversions.cc b/sync/protocol/proto_value_conversions.cc
index 648e1fc..ebb4d95 100644
--- a/sync/protocol/proto_value_conversions.cc
+++ b/sync/protocol/proto_value_conversions.cc
@@ -105,6 +105,15 @@ base::ListValue* MakeRepeatedValue(const F& fields, V* (*converter_fn)(T)) {
                                    std::string >, \
                                base::StringValue>(proto.field(), \
                                             MakeStringValue))
+#define SET_EXPERIMENT_ENABLED_FIELD(field)          \
+  do {                                               \
+    if (proto.has_##field() &&                       \
+        proto.field().has_enabled()) {               \
+      value->Set(#field,                             \
+                 new base::FundamentalValue(         \
+                     proto.field().enabled()));      \
+    }                                                \
+  } while (0)
 
 #define SET_FIELD(field, fn)                         \
   do {                                               \
@@ -208,13 +217,6 @@ base::DictionaryValue* PasswordSpecificsDataToValue(
   return value;
 }
 
-base::DictionaryValue* KeystoreEncryptionFlagsToValue(
-    const sync_pb::KeystoreEncryptionFlags& proto) {
-  base::DictionaryValue* value = new base::DictionaryValue();
-  SET_BOOL(enabled);
-  return value;
-}
-
 base::DictionaryValue* GlobalIdDirectiveToValue(
     const sync_pb::GlobalIdDirective& proto) {
   base::DictionaryValue* value = new base::DictionaryValue();
@@ -339,7 +341,10 @@ base::DictionaryValue* DictionarySpecificsToValue(
 base::DictionaryValue* ExperimentsSpecificsToValue(
     const sync_pb::ExperimentsSpecifics& proto) {
   base::DictionaryValue* value = new base::DictionaryValue();
-  SET(keystore_encryption, KeystoreEncryptionFlagsToValue);
+  SET_EXPERIMENT_ENABLED_FIELD(keystore_encryption);
+  SET_EXPERIMENT_ENABLED_FIELD(history_delete_directives);
+  SET_EXPERIMENT_ENABLED_FIELD(autofill_culling);
+  SET_EXPERIMENT_ENABLED_FIELD(favicon_sync);
   return value;
 }