[Sync] Added integ tests to verify UserEvents retry on TRANSIENT_ERROR.

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

Index: chrome/browser/sync/test/integration/single_client_user_events_sync_test.cc
diff --git a/chrome/browser/sync/test/integration/single_client_user_events_sync_test.cc b/chrome/browser/sync/test/integration/single_client_user_events_sync_test.cc
index 8a7564ce2da64a5042e8f0e9e91a64769ea03130..d5f69296ccb2f3fc980f2f0db841d91089b5710b 100644
--- a/chrome/browser/sync/test/integration/single_client_user_events_sync_test.cc
+++ b/chrome/browser/sync/test/integration/single_client_user_events_sync_test.cc
@@ -11,12 +11,44 @@
 #include "components/sync/protocol/user_event_specifics.pb.h"
 #include "components/sync/user_events/user_event_service.h"
 
+using base::Time;
+using base::TimeDelta;
 using fake_server::FakeServer;
 using sync_pb::UserEventSpecifics;
 using sync_pb::SyncEntity;
+using sync_pb::CommitResponse;
 
 namespace {
 
+UserEventSpecifics CreateEvent(int minutes_ago) {
+  UserEventSpecifics specifics;
+  specifics.set_event_time_usec(
+      (Time::Now() - TimeDelta::FromMinutes(minutes_ago)).ToInternalValue());
+  specifics.mutable_test_event();
+  return specifics;
+}
+
+CommitResponse::ResponseType BounceType(
+    CommitResponse::ResponseType type,
+    const fake_server::FakeServerEntity& entity) {
+  return type;
+}
+
+CommitResponse::ResponseType TransientErrorFirst(
+    bool* first,
+    UserEventSpecifics* retry_specifics,
+    const fake_server::FakeServerEntity& entity) {
+  if (*first) {
+    *first = false;
+    SyncEntity sync_entity;
+    entity.SerializeAsProto(&sync_entity);
+    *retry_specifics = sync_entity.specifics().user_event();
+    return CommitResponse::TRANSIENT_ERROR;
+  } else {
+    return CommitResponse::SUCCESS;
+  }
+}
+
 class UserEventEqualityChecker : public SingleClientStatusChangeChecker {
  public:
   UserEventEqualityChecker(browser_sync::ProfileSyncService* service,
@@ -24,7 +56,8 @@ class UserEventEqualityChecker : public SingleClientStatusChangeChecker {
                            std::vector<UserEventSpecifics> expected_specifics)
       : SingleClientStatusChangeChecker(service), fake_server_(fake_server) {
     for (const UserEventSpecifics& specifics : expected_specifics) {
-      expected_specifics_[specifics.event_time_usec()] = specifics;
+      expected_specifics_.insert(std::pair<int64_t, UserEventSpecifics>(
+          specifics.event_time_usec(), specifics));
     }
   }
 
@@ -40,17 +73,24 @@ class UserEventEqualityChecker : public SingleClientStatusChangeChecker {
       return false;
     }
 
+    // Because we have a multimap, we cannot just check counts and equality of
+    // items, but we need to make sure 2 of A and 1 of B is not the same as 1 of
+    // A and 2 of B. So to make this easy, copy the multimap and remove items.
+    std::multimap<int64_t, UserEventSpecifics> copied_expected_(
+        expected_specifics_.begin(), expected_specifics_.end());
     for (const SyncEntity& entity : entities) {
       UserEventSpecifics server_specifics = entity.specifics().user_event();
-      auto iter = expected_specifics_.find(server_specifics.event_time_usec());
+      auto iter = copied_expected_.find(server_specifics.event_time_usec());
       // We don't expect to encounter id matching events with different values,
       // this isn't going to recover so fail the test case now.
-      CHECK(expected_specifics_.end() != iter);
+      CHECK(copied_expected_.end() != iter);
       // TODO(skym): This may need to change if we start updating navigation_id
       // based on what sessions data is committed, and end up committing the
       // same event multiple times.
       EXPECT_EQ(iter->second.navigation_id(), server_specifics.navigation_id());
       EXPECT_EQ(iter->second.event_case(), server_specifics.event_case());
+
+      copied_expected_.erase(iter);
     }
 
     return true;
@@ -62,7 +102,7 @@ class UserEventEqualityChecker : public SingleClientStatusChangeChecker {
 
  private:
   FakeServer* fake_server_;
-  std::map<int64_t, UserEventSpecifics> expected_specifics_;
+  std::multimap<int64_t, UserEventSpecifics> expected_specifics_;
 };
 
 class SingleClientUserEventsSyncTest : public SyncTest {
@@ -82,12 +122,67 @@ IN_PROC_BROWSER_TEST_F(SingleClientUserEventsSyncTest, Sanity) {
       GetFakeServer()->GetSyncEntitiesByModelType(syncer::USER_EVENTS).size());
   syncer::UserEventService* event_service =
       browser_sync::UserEventServiceFactory::GetForProfile(GetProfile(0));
-  UserEventSpecifics specifics1;
-  specifics1.set_event_time_usec(base::Time::Now().ToInternalValue());
-  specifics1.mutable_test_event();
+  UserEventSpecifics specifics = CreateEvent(0);
+  event_service->RecordUserEvent(specifics);
+  UserEventEqualityChecker(GetSyncService(0), GetFakeServer(), {specifics})
+      .Wait();
+}
+
+IN_PROC_BROWSER_TEST_F(SingleClientUserEventsSyncTest, RetrySequential) {
+  ASSERT_TRUE(SetupSync());
+  UserEventSpecifics specifics1 = CreateEvent(1);
+  UserEventSpecifics specifics2 = CreateEvent(2);
+  syncer::UserEventService* event_service =
+      browser_sync::UserEventServiceFactory::GetForProfile(GetProfile(0));
+
+  GetFakeServer()->OverrideResponseType(
+      base::Bind(&BounceType, CommitResponse::TRANSIENT_ERROR));
   event_service->RecordUserEvent(specifics1);
+
+  // This will block until we hit a TRANSIENT_ERROR, at which point we will
+  // regain control and can switch back to SUCCESS.
   UserEventEqualityChecker(GetSyncService(0), GetFakeServer(), {specifics1})
       .Wait();
+  GetFakeServer()->OverrideResponseType(
+      base::Bind(&BounceType, CommitResponse::SUCCESS));
+  // Because the fake server records commits even on failure, we are able to
+  // verify that the commit for this event reached the server twice.
+  UserEventEqualityChecker(GetSyncService(0), GetFakeServer(),
+                           {specifics1, specifics1})
+      .Wait();
+
+  // Only record |specifics2| after |specifics1| was successful to avoid race
+  // conditions.
+  event_service->RecordUserEvent(specifics2);
+  UserEventEqualityChecker(GetSyncService(0), GetFakeServer(),
+                           {specifics1, specifics1, specifics2})
+      .Wait();
+}
+
+IN_PROC_BROWSER_TEST_F(SingleClientUserEventsSyncTest, RetryParallel) {
+  ASSERT_TRUE(SetupSync());
+  bool first = true;
+  UserEventSpecifics specifics1 = CreateEvent(1);
+  UserEventSpecifics specifics2 = CreateEvent(2);
+  UserEventSpecifics retry_specifics;
+
+  syncer::UserEventService* event_service =
+      browser_sync::UserEventServiceFactory::GetForProfile(GetProfile(0));
+
+  // We're not really sure if |specifics1| or |specifics2| is going to see the
+  // error, so record the one that does into |retry_specifics| and use it in
+  // expectations.
+  GetFakeServer()->OverrideResponseType(
+      base::Bind(&TransientErrorFirst, &first, &retry_specifics));
+
+  event_service->RecordUserEvent(specifics2);
+  event_service->RecordUserEvent(specifics1);
+  UserEventEqualityChecker(GetSyncService(0), GetFakeServer(),
+                           {specifics1, specifics2})
+      .Wait();
+  UserEventEqualityChecker(GetSyncService(0), GetFakeServer(),
+                           {specifics1, specifics2, retry_specifics})
+      .Wait();
 }
 
 }  // namespace