[Sync] Add an error integration test for USS.

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

Index: chrome/browser/sync/test/integration/two_client_uss_sync_test.cc
diff --git a/chrome/browser/sync/test/integration/two_client_uss_sync_test.cc b/chrome/browser/sync/test/integration/two_client_uss_sync_test.cc
index b9df9538c0d7a1787998a90d1c71916cec9d5e10..c159ce0867d18f10c46b625b88f2847bfa718f01 100644
--- a/chrome/browser/sync/test/integration/two_client_uss_sync_test.cc
+++ b/chrome/browser/sync/test/integration/two_client_uss_sync_test.cc
@@ -8,6 +8,7 @@
 #include "chrome/browser/sync/chrome_sync_client.h"
 #include "chrome/browser/sync/profile_sync_service_factory.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/status_change_checker.h"
 #include "chrome/browser/sync/test/integration/sync_integration_test_util.h"
 #include "chrome/browser/sync/test/integration/sync_test.h"
@@ -23,6 +24,7 @@ using syncer_v2::ModelTypeService;
 using syncer_v2::SharedModelTypeProcessor;
 
 const char kKey1[] = "key1";
+const char kKey2[] = "key2";
 const char kValue1[] = "value1";
 const char kValue2[] = "value2";
 const char kValue3[] = "value3";
@@ -181,6 +183,27 @@ class MetadataAbsentChecker : public KeyChecker {
   }
 };
 
+// Wait for PREFERENCES to no longer be running.
+class PrefsNotRunningChecker : public SingleClientStatusChangeChecker {
+ public:
+  explicit PrefsNotRunningChecker(browser_sync::ProfileSyncService* service)
+      : SingleClientStatusChangeChecker(service) {}
+  ~PrefsNotRunningChecker() override {}

[skym, 2016/09/27 20:24:45]

I don't think you need this. I'm going through the other checkers are removing
these. StatusChangeChecker's dtor is not pure.

[maxbogue, 2016/09/28 20:17:08]

Removed them all.

+
+  bool Wait() {
+    SingleClientStatusChangeChecker::Wait();

[skym, 2016/09/27 20:24:44]

This awkwardness should be fixed by my CL.

[maxbogue, 2016/09/28 20:17:09]

Acknowledged.

+    return !TimedOut();
+  }
+
+  bool IsExitConditionSatisfied() override {
+    return !service()->IsDataTypeControllerRunning(syncer::PREFERENCES);
+  }
+
+  std::string GetDebugMessage() const override {
+    return "Waiting for prefs to be not running.";
+  }
+};
+
 class TwoClientUssSyncTest : public SyncTest {
  public:
   TwoClientUssSyncTest() : SyncTest(TWO_CLIENT) {
@@ -301,3 +324,23 @@ IN_PROC_BROWSER_TEST_F(TwoClientUssSyncTest, ConflictResolution) {
   ASSERT_TRUE(DataChecker(model1, kKey1, kValue3).Wait());
   ASSERT_TRUE(DataChecker(model2, kKey1, kValue3).Wait());
 }
+
+IN_PROC_BROWSER_TEST_F(TwoClientUssSyncTest, Error) {
+  ASSERT_TRUE(SetupSync());
+  TestModelTypeService* model1 = GetModelTypeService(0);
+  TestModelTypeService* model2 = GetModelTypeService(1);
+
+  // Add an entity.
+  model1->WriteItem(kKey1, kValue1);
+  ASSERT_TRUE(DataChecker(model2, kKey1, kValue1).Wait());
+
+  // Queue an error in model 2 and trigger a GetUpdates.
+  model2->SetServiceError(syncer::SyncError::DATATYPE_ERROR);
+  model1->WriteItem(kKey1, kValue2);

[skym, 2016/09/27 20:24:45]

What's the point of this? You don't verify that model2 doesn't have
key1->value2.

[maxbogue, 2016/09/28 20:17:09]

To trigger an ApplySyncChanges (GU) in model2.

+
+  // The type should stop syncing but keep tracking metadata.
+  ASSERT_TRUE(PrefsNotRunningChecker(GetSyncService(1)).Wait());

[skym, 2016/09/27 20:24:44]

Seems odd that you error(2) -> write(1) -> block for shutdown(2). Shouldn't you
error(2) -> block for shutdown(2) -> write(1)?

[maxbogue, 2016/09/28 20:17:09]

SetServiceError stores the error that is returned on the next fallible service
operation. The write is to make sure a service operation happens.

+  ASSERT_EQ(1U, model2->db().metadata_count());
+  model2->WriteItem(kKey2, kValue2);
+  ASSERT_EQ(2U, model2->db().metadata_count());
+}