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.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=196116

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

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

15 files changed, 39 insertions, 61 deletions

diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index 274f935..d18b545 100644
--- 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
index 5a7f492..826f55b 100644
--- 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.cc b/chrome/browser/history/web_history_service.cc
index 5f9418e..45a531d 100644
--- a/chrome/browser/history/web_history_service.cc
+++ b/chrome/browser/history/web_history_service.cc
@@ -102,8 +102,10 @@ class RequestImpl : public WebHistoryService::Request,
     }
     url_fetcher_->GetResponseAsString(&response_body_);
     url_fetcher_.reset();
-    callback_.Run(this, true);
     is_pending_ = false;
+    callback_.Run(this, true);
+    // It is valid for the callback to delete |this|, so do not access any
+    // members below here.
   }
 
   // OAuth2TokenService::Consumer interface.
@@ -124,9 +126,10 @@ class RequestImpl : public WebHistoryService::Request,
       const OAuth2TokenService::Request* request,
       const GoogleServiceAuthError& error) OVERRIDE {
     token_request_.reset();
-    LOG(WARNING) << "Failed to get OAuth token: " << error.ToString();
-    callback_.Run(this, false);
     is_pending_ = false;
+    callback_.Run(this, false);
+    // It is valid for the callback to delete |this|, so do not access any
+    // members below here.
   }
 
   // Helper for creating a new URLFetcher for the API request.
diff --git a/chrome/browser/history/web_history_service_factory.cc b/chrome/browser/history/web_history_service_factory.cc
index af51d1a..66a3b33 100644
--- 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
index 9049b76..6cf9b9b 100644
--- 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
index bcfd326..5965179 100644
--- 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
index 14312e58..7808897 100644
--- 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
index b779881..9a2d798 100644
--- 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
index c6f1b66..1b78842 100644
--- 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
index 244550e..475b43c 100644
--- 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
index b2894aa..89842e0 100644
--- 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
index 35f8095..9f441de 100644
--- 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
index 92e1286..54c3276 100644
--- 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
index a3b874b..c4b7dd5 100644
--- a/sync/internal_api/public/util/experiments.h
+++ b/sync/internal_api/public/util/experiments.h
@@ -12,8 +12,6 @@ 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";
 
@@ -21,14 +19,12 @@ const char kFaviconSyncFlag[] = "enable-sync-favicons";
 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
index 0f807bc..fcccff6 100644
--- 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,