Initial clear server data impl

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

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/sync/test/integration/sync_test.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <limits>
@@ skipping 593 matching lines @@
 
 bool SyncTest::SetupSync() {
   // Create sync profiles and clients if they haven't already been created.
   if (profiles_.empty()) {
     if (!SetupClients()) {
       LOG(FATAL) << "SetupClients() failed.";
       return false;
     }
   }
 
+  int clientIndex = 0;

[pavely, 2017/03/08 21:15:14]

SyncTest::SetupSync contains sequence of calls to other helper functions with
small amount of code around each call (SetupClients, GetClient()->SetupSync.

Does it make sense to extract this block of code into separate helper function
to maintain readability of SetupSync?

[wylieb, 2017/03/09 18:42:26]

Good point. The readability is bad here. I factored out the code that
setup/cleared data into a method. I kept the index and increment around, but I
think it's easily readable. 

+  // If we're using external servers, clear server data so the account starts
+  // with a clean slate.
+  if (UsingExternalServers()) {
+    // Setup the first client so the sync engine is initialized, which is
+    // required to clear server data.
+    DVLOG(1) << "Setting up first client for clear.";
+    if (!GetClient(clientIndex)->SetupSyncForClear()) {
+      LOG(FATAL) << "SetupSync() failed.";
+      return false;
+    }
+
+    DVLOG(1) << "Done setting up first client for clear.";
+    if (!ClearServerData(GetClient(clientIndex++))) {
+      LOG(FATAL) << "ClearServerData failed.";
+      return false;
+    }
+  }
+
   // Sync each of the profiles.
-  for (int i = 0; i < num_clients_; ++i) {
-    if (!GetClient(i)->SetupSync()) {
+  for (; clientIndex < num_clients_; clientIndex++) {
+    DVLOG(1) << "Setting up " << clientIndex << " client";
+    if (!GetClient(clientIndex)->SetupSync()) {
       LOG(FATAL) << "SetupSync() failed.";
       return false;
     }
   }
 
   // Because clients may modify sync data as part of startup (for example local
   // session-releated data is rewritten), we need to ensure all startup-based
   // changes have propagated between the clients.
   //
   // Tests that don't use self-notifications can't await quiescense.  They'll
@@ skipping 505 matching lines @@
 
 void SyncTest::TriggerSyncForModelTypes(int index,
                                         syncer::ModelTypeSet model_types) {
   GetSyncService(index)->TriggerRefresh(model_types);
 }
 
 void SyncTest::SetPreexistingPreferencesFileContents(
     const std::string& contents) {
   preexisting_preferences_file_contents_ = contents;
 }
+
+bool SyncTest::ClearServerData(ProfileSyncServiceHarness* harness) {
+  // At this point our birthday is good.
+  base::RunLoop run_loop;
+  harness->service()->ClearServerDataForTest(run_loop.QuitClosure());
+  run_loop.Run();
+
+  // Our birthday is bad here so restart sync to get the new birthday from the

[pavely, 2017/03/08 21:15:14]

"Our birthday is bad" => "Our birthday is invalidated on the server"

+  // server.
+  return harness->RestartSyncService();
+}