Sync: Turn on full history sync by default.

Full history sync (and history delete directives) were off by default, but turned on in M27 via an experiment. This meant that history from signed-in devices was not visible on chrome://history until the browser was restarted. This CL make full history sync enabled by default, but retains a command-line switch and about:flags option to disable it. It also removes the option to disable delete directives only, since that had the same effect as disabling full history sync. TBR=brettw@chromium.org BUG=233098 TEST=Start up Chrome in a fresh profile, and sign in. Go to chrome://history and ensure that it says "Showing history from your signed-in devices" at the top of the history page. Review URL: https://chromiumcodereview.appspot.com/14344002 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@196116 0039d316-1c4b-4281-b951-d872f2087c98
dednal · Apr 24, 2013 · 80eed2d · 80eed2d
1 parent babd5a9
commit 80eed2d
Show file tree

Hide file tree

Showing 14 changed files with 33 additions and 58 deletions.
diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
@@ -7041,11 +7041,11 @@ Keep your key file in a safe place. You will need it to create new versions of y
           Enable experimental Bluetooth features.
         </message>
       </if>
-      <message name="IDS_FLAGS_FULL_HISTORY_SYNC_NAME" desc="Name of the flag to enable full history sync.">
-        Enable full history sync.
+      <message name="IDS_FLAGS_FULL_HISTORY_SYNC_NAME" desc="Name of the flag to disable full history sync.">
+        Disable full history sync
       </message>
-      <message name="IDS_FLAGS_FULL_HISTORY_SYNC_DESCRIPTION" desc="Description for the flag to enable full history sync.">
-        If tab sync is enabled, this feature allows you to see and delete history entries from your synced devices at chrome://history.
+      <message name="IDS_FLAGS_FULL_HISTORY_SYNC_DESCRIPTION" desc="Description for the flag to disable full history sync.">
+        Allows you to see and delete history entries from your signed-in devices at chrome://history.
       </message>
       <message name="IDS_FLAGS_DISABLE_ACCELERATED_VIDEO_DECODE_NAME" desc="Name of the flag to disable accelerated video decode where available.">
         Disable hardware-accelerated video decode.

diff --git a/chrome/browser/about_flags.cc b/chrome/browser/about_flags.cc
@@ -1276,11 +1276,11 @@ const Experiment kExperiments[] = {
   },
 #endif
   {
-    "full-history-sync",
+    "disable-full-history-sync",
     IDS_FLAGS_FULL_HISTORY_SYNC_NAME,
     IDS_FLAGS_FULL_HISTORY_SYNC_DESCRIPTION,
     kOsAll,
-    SINGLE_VALUE_TYPE(switches::kHistoryEnableFullHistorySync)
+    SINGLE_VALUE_TYPE(switches::kHistoryDisableFullHistorySync)
   },
   {
     "enable-usermedia-screen-capture",

diff --git a/chrome/browser/history/web_history_service_factory.cc b/chrome/browser/history/web_history_service_factory.cc
@@ -17,8 +17,8 @@ namespace {
 // Returns true if the user is signed in and full history sync is enabled,
 // and false otherwise.
 bool IsHistorySyncEnabled(Profile* profile) {
-  if (CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kHistoryEnableFullHistorySync)) {
+  if (!CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kHistoryDisableFullHistorySync)) {
     ProfileSyncService* sync =
         ProfileSyncServiceFactory::GetInstance()->GetForProfile(profile);
     return sync &&

diff --git a/chrome/browser/sync/profile_sync_components_factory_impl.cc b/chrome/browser/sync/profile_sync_components_factory_impl.cc
@@ -146,11 +146,9 @@ void ProfileSyncComponentsFactoryImpl::RegisterCommonDataTypes(
         new TypedUrlDataTypeController(this, profile_, pss));
   }
 
-  // Unless it is explicitly disabled, history delete directive sync is
-  // enabled whenever full history sync is enabled.
-  if (command_line_->HasSwitch(switches::kHistoryEnableFullHistorySync) &&
-      !command_line_->HasSwitch(
-          switches::kDisableSyncHistoryDeleteDirectives)) {
+  // Delete directive sync is enabled by default.  Register unless full history
+  // sync is disabled.
+  if (!command_line_->HasSwitch(switches::kHistoryDisableFullHistorySync)) {
     pss->RegisterDataTypeController(
         new UIDataTypeController(
             syncer::HISTORY_DELETE_DIRECTIVES, this, profile_, pss));

diff --git a/chrome/browser/sync/profile_sync_components_factory_impl_unittest.cc b/chrome/browser/sync/profile_sync_components_factory_impl_unittest.cc
@@ -45,6 +45,7 @@ class ProfileSyncComponentsFactoryImplTest : public testing::Test {
 #endif
     datatypes.push_back(syncer::EXTENSIONS);
     datatypes.push_back(syncer::EXTENSION_SETTINGS);
+    datatypes.push_back(syncer::HISTORY_DELETE_DIRECTIVES);
     datatypes.push_back(syncer::PASSWORDS);
     datatypes.push_back(syncer::PREFERENCES);
     datatypes.push_back(syncer::PRIORITY_PREFERENCES);

diff --git a/chrome/browser/sync/profile_sync_service.cc b/chrome/browser/sync/profile_sync_service.cc
@@ -984,12 +984,6 @@ void ProfileSyncService::OnExperimentsChanged(
                                       true);
   }
 
-  if (experiments.full_history_sync) {
-    about_flags::SetExperimentEnabled(g_browser_process->local_state(),
-                                      syncer::kFullHistorySyncFlag,
-                                      true);
-  }
-
   if (experiments.favicon_sync) {
     about_flags::SetExperimentEnabled(g_browser_process->local_state(),
                                       syncer::kFaviconSyncFlag,

diff --git a/chrome/browser/sync/sync_prefs.cc b/chrome/browser/sync/sync_prefs.cc
@@ -411,8 +411,8 @@ void SyncPrefs::RegisterPrefGroups() {
   pref_groups_[syncer::PREFERENCES].Put(syncer::SEARCH_ENGINES);
 
   pref_groups_[syncer::TYPED_URLS].Put(syncer::HISTORY_DELETE_DIRECTIVES);
-  const CommandLine& command_line = *CommandLine::ForCurrentProcess();
-  if (command_line.HasSwitch(switches::kHistoryEnableFullHistorySync)) {
+  if (!CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kHistoryDisableFullHistorySync)) {
     pref_groups_[syncer::TYPED_URLS].Put(syncer::SESSIONS);
     pref_groups_[syncer::TYPED_URLS].Put(syncer::FAVICON_IMAGES);
     pref_groups_[syncer::TYPED_URLS].Put(syncer::FAVICON_TRACKING);

diff --git a/chrome/browser/sync/sync_prefs_unittest.cc b/chrome/browser/sync/sync_prefs_unittest.cc
@@ -24,8 +24,6 @@ using ::testing::StrictMock;
 class SyncPrefsTest : public testing::Test {
  protected:
   virtual void SetUp() OVERRIDE {
-    CommandLine::ForCurrentProcess()->AppendSwitch(
-        switches::kHistoryEnableFullHistorySync);
     SyncPrefs::RegisterUserPrefs(pref_service_.registry());
   }
 

diff --git a/chrome/browser/sync/test/integration/enable_disable_test.cc b/chrome/browser/sync/test/integration/enable_disable_test.cc
@@ -111,10 +111,12 @@ IN_PROC_BROWSER_TEST_F(EnableDisableSingleClientTest, DisableOneAtATime) {
     ASSERT_TRUE(GetClient(0)->DisableSyncForDatatype(it.Get()));
 
     // AUTOFILL_PROFILE is lumped together with AUTOFILL.
-    // SESSIONS is lumped together with PROXY_TABS and
-    // HISTORY_DELETE_DIRECTIVES.
+    // SESSIONS is lumped together with PROXY_TABS and TYPED_URLS.
+    // HISTORY_DELETE_DIRECTIVES is lumped together with TYPED_URLS.
     // PRIORITY_PREFERENCES is lumped together with PREFERENCES.
-    if (it.Get() == syncer::AUTOFILL_PROFILE || it.Get() == syncer::SESSIONS ||
+    if (it.Get() == syncer::AUTOFILL_PROFILE ||
+        it.Get() == syncer::SESSIONS ||
+        it.Get() == syncer::HISTORY_DELETE_DIRECTIVES ||
         it.Get() == syncer::PRIORITY_PREFERENCES) {
       continue;
     }
@@ -125,14 +127,19 @@ IN_PROC_BROWSER_TEST_F(EnableDisableSingleClientTest, DisableOneAtATime) {
     ASSERT_FALSE(DoesTopLevelNodeExist(user_share, it.Get()))
         << syncer::ModelTypeToString(it.Get());
 
-    // AUTOFILL_PROFILE is lumped together with AUTOFILL.
     if (it.Get() == syncer::AUTOFILL) {
+      // AUTOFILL_PROFILE is lumped together with AUTOFILL.
+      ASSERT_FALSE(DoesTopLevelNodeExist(user_share, syncer::AUTOFILL_PROFILE));
+    } else if (it.Get() == syncer::TYPED_URLS) {
       ASSERT_FALSE(DoesTopLevelNodeExist(user_share,
-                                         syncer::AUTOFILL_PROFILE));
-    } else if (it.Get() == syncer::HISTORY_DELETE_DIRECTIVES ||
-               it.Get() == syncer::PROXY_TABS) {
-      ASSERT_FALSE(DoesTopLevelNodeExist(user_share,
-                                         syncer::SESSIONS));
+                                         syncer::HISTORY_DELETE_DIRECTIVES));
+      // SESSIONS should be enabled only if PROXY_TABS is.
+      ASSERT_EQ(GetClient(0)->IsTypePreferred(syncer::PROXY_TABS),
+                DoesTopLevelNodeExist(user_share, syncer::SESSIONS));
+    } else if (it.Get() == syncer::PROXY_TABS) {
+      // SESSIONS should be enabled only if TYPED_URLS is.
+      ASSERT_EQ(GetClient(0)->IsTypePreferred(syncer::TYPED_URLS),
+                DoesTopLevelNodeExist(user_share, syncer::SESSIONS));
     } else if (it.Get() == syncer::PREFERENCES) {
       ASSERT_FALSE(DoesTopLevelNodeExist(user_share,
                                          syncer::PRIORITY_PREFERENCES));

diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
@@ -396,10 +396,6 @@ const char kDisableSyncExtensionSettings[]  = "disable-sync-extension-settings";
 // Disables syncing of extensions.
 const char kDisableSyncExtensions[]         = "disable-sync-extensions";
 
-// Disables syncing of history delete directives.
-const char kDisableSyncHistoryDeleteDirectives[] =
-    "disable-sync-history-delete-directives";
-
 // Disables syncing browser passwords.
 const char kDisableSyncPasswords[]          = "disable-sync-passwords";
 
@@ -757,9 +753,6 @@ const char kHideIcons[]                     = "hide-icons";
 // Disables full history sync.
 const char kHistoryDisableFullHistorySync[] = "disable-full-history-sync";
 
-// Enables full history sync (not just typed URLs) for signed-in users.
-const char kHistoryEnableFullHistorySync[]  = "enable-full-history-sync";
-
 // Enables grouping websites by domain and filtering them by period.
 const char kHistoryEnableGroupByDomain[]    = "enable-grouped-history";
 

diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
@@ -118,7 +118,6 @@ extern const char kDisableSyncBookmarks[];
 extern const char kDisableSyncDictionary[];
 extern const char kDisableSyncExtensionSettings[];
 extern const char kDisableSyncExtensions[];
-extern const char kDisableSyncHistoryDeleteDirectives[];
 extern const char kDisableSyncPasswords[];
 extern const char kDisableSyncPreferences[];
 extern const char kDisableSyncPriorityPreferences[];
@@ -212,7 +211,6 @@ extern const char kHelp[];
 extern const char kHelpShort[];
 extern const char kHideIcons[];
 extern const char kHistoryDisableFullHistorySync[];
-extern const char kHistoryEnableFullHistorySync[];
 extern const char kHistoryEnableGroupByDomain[];
 extern const char kHistoryWebHistoryUrl[];
 extern const char kHomePage[];

diff --git a/chrome/test/base/testing_profile.cc b/chrome/test/base/testing_profile.cc
@@ -35,6 +35,7 @@
 #include "chrome/browser/history/shortcuts_backend.h"
 #include "chrome/browser/history/shortcuts_backend_factory.h"
 #include "chrome/browser/history/top_sites.h"
+#include "chrome/browser/history/web_history_service_factory.h"
 #include "chrome/browser/net/proxy_service_factory.h"
 #include "chrome/browser/notifications/desktop_notification_service.h"
 #include "chrome/browser/notifications/desktop_notification_service_factory.h"
@@ -346,6 +347,8 @@ void TestingProfile::CreateHistoryService(bool delete_file, bool no_db) {
                              no_db)) {
     HistoryServiceFactory::GetInstance()->SetTestingFactoryAndUse(this, NULL);
   }
+  // Disable WebHistoryService by default, since it makes network requests.
+  WebHistoryServiceFactory::GetInstance()->SetTestingFactory(this, NULL);
 }
 
 void TestingProfile::DestroyHistoryService() {

diff --git a/sync/internal_api/public/util/experiments.h b/sync/internal_api/public/util/experiments.h
@@ -12,23 +12,19 @@ namespace syncer {
 const char kKeystoreEncryptionTag[] = "keystore_encryption";
 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() : keystore_encryption(false),
                   autofill_culling(false),
-                  full_history_sync(false),
                   favicon_sync(false),
                   favicon_sync_limit(200) {}
 
   bool Matches(const Experiments& rhs) {
     return (keystore_encryption == rhs.keystore_encryption &&
             autofill_culling == rhs.autofill_culling &&
-            full_history_sync == rhs.full_history_sync &&
             favicon_sync == rhs.favicon_sync &&
             favicon_sync_limit == rhs.favicon_sync_limit);
   }
@@ -39,9 +35,6 @@ struct Experiments {
   // Enable deletion of expired autofill entries (if autofill sync is enabled).
   bool autofill_culling;
 
-  // 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;
 

diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc
@@ -1366,16 +1366,6 @@ bool SyncManagerImpl::ReceivedExperiment(Experiments* experiments) {
     found_experiment = true;
   }
 
-  ReadNode full_history_sync_node(&trans);
-  if (full_history_sync_node.InitByClientTagLookup(
-          syncer::EXPERIMENTS,
-          syncer::kFullHistorySyncTag) == BaseNode::INIT_OK &&
-      full_history_sync_node.GetExperimentsSpecifics().
-          history_delete_directives().enabled()) {
-    experiments->full_history_sync = true;
-    found_experiment = true;
-  }
-
   ReadNode favicon_sync_node(&trans);
   if (favicon_sync_node.InitByClientTagLookup(
           syncer::EXPERIMENTS,