Add more sync tests than span browser restarts

They verify the very basic requirement of progress markers being
peristed and loaded upon restart, which prevents clients to fetch all
data from scratch from the server.

This also surfaces a bug with USS bookmarks that should be addressed in
a follow-up patch.

Bug: 856696
Change-Id: I924249579f2d5790f25be6809b03f50d5c421ffc
Reviewed-on: https://chromium-review.googlesource.com/c/1347366
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Mohamed Amir Yosef <mamir@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610385}

chrome/browser/sync/test/integration/single_client_bookmarks_sync_test.cc

@@ -6,9 +6,11 @@
 
 #include "base/macros.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/metrics/histogram_tester.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/sync/test/integration/bookmarks_helper.h"
 #include "chrome/browser/sync/test/integration/feature_toggler.h"
+#include "chrome/browser/sync/test/integration/profile_sync_service_harness.h"
 #include "chrome/browser/sync/test/integration/single_client_status_change_checker.h"
 #include "chrome/browser/sync/test/integration/sync_test.h"
 #include "chrome/browser/sync/test/integration/updated_progress_marker_checker.h"
@@ -536,6 +538,55 @@ IN_PROC_BROWSER_TEST_P(SingleClientBookmarksSyncTest, E2E_ONLY(SanitySetup)) {
   ASSERT_TRUE(SetupSync()) <<  "SetupSync() failed.";
 }
 
+IN_PROC_BROWSER_TEST_P(SingleClientBookmarksSyncTest,
+                       PRE_PersistProgressMarkerOnRestart) {
+  const std::string title = "Seattle Sounders FC";
+  fake_server::EntityBuilderFactory entity_builder_factory;
+  fake_server::BookmarkEntityBuilder bookmark_builder =
+      entity_builder_factory.NewBookmarkEntityBuilder(title);
+  fake_server_->InjectEntity(bookmark_builder.BuildFolder());
+
+  base::HistogramTester histogram_tester;
+  ASSERT_TRUE(SetupSync()) << "SetupSync() failed.";
+  ASSERT_EQ(1, GetBookmarkBarNode(kSingleProfileIndex)->child_count());
+
+  EXPECT_NE(
+      0, histogram_tester.GetBucketCount("Sync.ModelTypeEntityChange3.BOOKMARK",
+                                         /*REMOTE_INITIAL_UPDATE=*/5));
+}
+
+IN_PROC_BROWSER_TEST_P(SingleClientBookmarksSyncTest,
+                       PersistProgressMarkerOnRestart) {
+  const std::string title = "Seattle Sounders FC";
+  fake_server::EntityBuilderFactory entity_builder_factory;
+  fake_server::BookmarkEntityBuilder bookmark_builder =
+      entity_builder_factory.NewBookmarkEntityBuilder(title);
+  fake_server_->InjectEntity(bookmark_builder.BuildFolder());
+
+  base::HistogramTester histogram_tester;
+  ASSERT_TRUE(SetupClients()) << "SetupClients() failed.";
+  ASSERT_EQ(1, GetBookmarkBarNode(kSingleProfileIndex)->child_count());
+
+#if defined(CHROMEOS)
+  // identity::SetRefreshTokenForPrimaryAccount() is needed on ChromeOS in order
+  // to get a non-empty refresh token on startup.
+  GetClient(0)->SignInPrimaryAccount();
+#endif  // defined(CHROMEOS)
+  ASSERT_TRUE(GetClient(kSingleProfileIndex)->AwaitEngineInitialization());
+
+  // After restart, the last sync cycle snapshot should be empty.
+  // Once a sync request happened (e.g. by a poll), that snapshot is populated.
+  // We use the following checker to simply wait for an non-empty snapshot.
+  EXPECT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait());
+
+  // TODO(mamir): The expectation below should pass but doesn't with USS.
+  // Investigate the cause and fix the underlying issue.
+  // EXPECT_EQ(
+  //    0,
+  //    histogram_tester.GetBucketCount("Sync.ModelTypeEntityChange3.BOOKMARK",
+  //                                       /*REMOTE_INITIAL_UPDATE=*/5));
+}
+
 INSTANTIATE_TEST_CASE_P(USS,
                         SingleClientBookmarksSyncTest,
                         ::testing::Values(false, true));

chrome/browser/sync/test/integration/single_client_polling_sync_test.cc

@@ -151,7 +151,7 @@ IN_PROC_BROWSER_TEST_F(SingleClientPollingSyncTest,
   SyncPrefs remote_prefs(GetProfile(0)->GetPrefs());
   ASSERT_FALSE(remote_prefs.GetLastPollTime().is_null());
 
-  // After the start, the last sync cycle snapshot should be empty.
+  // After restart, the last sync cycle snapshot should be empty.
   // Once a sync request happened (e.g. by a poll), that snapshot is populated.
   // We use the following checker to simply wait for an non-empty snapshot.
   EXPECT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait());

chrome/browser/sync/test/integration/single_client_preferences_sync_test.cc

@@ -8,6 +8,7 @@
 #include "base/values.h"
 #include "chrome/browser/sync/test/integration/feature_toggler.h"
 #include "chrome/browser/sync/test/integration/preferences_helper.h"
+#include "chrome/browser/sync/test/integration/profile_sync_service_harness.h"
 #include "chrome/browser/sync/test/integration/sync_test.h"
 #include "chrome/browser/sync/test/integration/updated_progress_marker_checker.h"
 #include "chrome/common/pref_names.h"
@@ -121,6 +122,50 @@ IN_PROC_BROWSER_TEST_P(SingleClientPreferencesSyncTest,
                               /*REMOTE_INITIAL_UPDATE=*/5));
 }
 
+IN_PROC_BROWSER_TEST_P(SingleClientPreferencesSyncTest,
+                       PRE_PersistProgressMarkerOnRestart) {
+  sync_pb::EntitySpecifics specifics;
+  specifics.mutable_preference()->set_name("testing.my-test-preference");
+  fake_server_->InjectEntity(
+      syncer::PersistentUniqueClientEntity::CreateFromEntitySpecifics(
+          specifics.preference().name(), specifics, /*creation_time=*/0,
+          /*last_modified_time=*/0));
+
+  base::HistogramTester histogram_tester;
+  ASSERT_TRUE(SetupSync());
+  EXPECT_EQ(1, histogram_tester.GetBucketCount(
+                   "Sync.ModelTypeEntityChange3.PREFERENCE",
+                   /*REMOTE_INITIAL_UPDATE=*/5));
+}
+
+IN_PROC_BROWSER_TEST_P(SingleClientPreferencesSyncTest,
+                       PersistProgressMarkerOnRestart) {
+  sync_pb::EntitySpecifics specifics;
+  specifics.mutable_preference()->set_name("testing.my-test-preference");
+  fake_server_->InjectEntity(
+      syncer::PersistentUniqueClientEntity::CreateFromEntitySpecifics(
+          specifics.preference().name(), specifics, /*creation_time=*/0,
+          /*last_modified_time=*/0));
+
+  base::HistogramTester histogram_tester;
+  ASSERT_TRUE(SetupClients()) << "SetupClients() failed.";
+#if defined(CHROMEOS)
+  // identity::SetRefreshTokenForPrimaryAccount() is needed on ChromeOS in order
+  // to get a non-empty refresh token on startup.
+  GetClient(0)->SignInPrimaryAccount();
+#endif  // defined(CHROMEOS)
+  ASSERT_TRUE(GetClient(0)->AwaitEngineInitialization());
+
+  // After restart, the last sync cycle snapshot should be empty.
+  // Once a sync request happened (e.g. by a poll), that snapshot is populated.
+  // We use the following checker to simply wait for an non-empty snapshot.
+  EXPECT_TRUE(UpdatedProgressMarkerChecker(GetSyncService(0)).Wait());
+
+  EXPECT_EQ(0, histogram_tester.GetBucketCount(
+                   "Sync.ModelTypeEntityChange3.PREFERENCE",
+                   /*REMOTE_INITIAL_UPDATE=*/5));
+}
+
 INSTANTIATE_TEST_CASE_P(USS,
                         SingleClientPreferencesSyncTest,
                         ::testing::Values(false, true));