[Sync] Fix test UserEvent creation.

Due to the recent rework of how to decide if events should be recorded
or not, the test events created through sync-internals was not updated.
This fixes them by making two changes.

1. event_case() is now correctly set so that the service can identify
the type of event.

2. navigation_id's presence is not set if the text field is not filled
in, allowing it to bypass the requirement for history sync to be
enabled.

Bug: 792730
Change-Id: Icabf100da886cbd28efc8db2ce3e6e8cb4a50af1
Reviewed-on: https://chromium-review.googlesource.com/812975
Commit-Queue: Sky Malice <skym@chromium.org>
Reviewed-by: Patrick Noland <pnoland@google.com>
Cr-Commit-Position: refs/heads/master@{#522298}

chrome/browser/ui/webui/sync_internals_message_handler.cc

@@ -54,6 +54,15 @@ int64_t StringAtIndexToInt64(const base::ListValue* list, int index) {
   return 0;
 }
 
+// Returns whether the there is any value at the given |index|.
+bool HasSomethingAtIndex(const base::ListValue* list, int index) {
+  std::string str;
+  if (list->GetString(index, &str)) {
+    return !str.empty();
+  }
+  return false;
+}
+
 }  //  namespace
 
 SyncInternalsMessageHandler::SyncInternalsMessageHandler()
@@ -229,8 +238,18 @@ void SyncInternalsMessageHandler::HandleWriteUserEvent(
       browser_sync::UserEventServiceFactory::GetForProfile(profile);
 
   sync_pb::UserEventSpecifics event_specifics;
+  // Even though there's nothing to set inside the test event object, it needs
+  // to be created so that later logic can discern our event type.
+  event_specifics.mutable_test_event();
+
+  // |event_time_usec| is required.
   event_specifics.set_event_time_usec(StringAtIndexToInt64(args, 0));
-  event_specifics.set_navigation_id(StringAtIndexToInt64(args, 1));
+
+  // |navigation_id| is optional, treat empty string and 0 differently.
+  if (HasSomethingAtIndex(args, 1)) {
+    event_specifics.set_navigation_id(StringAtIndexToInt64(args, 1));
+  }
+
   user_event_service->RecordUserEvent(event_specifics);
 }
 

chrome/browser/ui/webui/sync_internals_message_handler_unittest.cc

@@ -27,6 +27,7 @@
 using base::DictionaryValue;
 using base::ListValue;
 using base::Value;
+using sync_pb::UserEventSpecifics;
 using syncer::FakeUserEventService;
 using syncer::SyncService;
 using syncer::SyncServiceObserver;
@@ -337,8 +338,9 @@ TEST_F(SyncInternalsMessageHandlerTest, WriteUserEvent) {
   handler()->HandleWriteUserEvent(&args);
 
   ASSERT_EQ(1u, fake_user_event_service()->GetRecordedUserEvents().size());
-  const sync_pb::UserEventSpecifics& event =
+  const UserEventSpecifics& event =
       *fake_user_event_service()->GetRecordedUserEvents().begin();
+  EXPECT_EQ(UserEventSpecifics::kTestEvent, event.event_case());
   EXPECT_EQ(1000000000000000000, event.event_time_usec());
   EXPECT_EQ(-1, event.navigation_id());
 }
@@ -346,13 +348,48 @@ TEST_F(SyncInternalsMessageHandlerTest, WriteUserEvent) {
 TEST_F(SyncInternalsMessageHandlerTest, WriteUserEventBadParse) {
   ListValue args;
   args.AppendString("123abc");
+  args.AppendString("abcdefghijklmnopqrstuvwxyz");
+  handler()->HandleWriteUserEvent(&args);
+
+  ASSERT_EQ(1u, fake_user_event_service()->GetRecordedUserEvents().size());
+  const UserEventSpecifics& event =
+      *fake_user_event_service()->GetRecordedUserEvents().begin();
+  EXPECT_EQ(UserEventSpecifics::kTestEvent, event.event_case());
+  EXPECT_EQ(0, event.event_time_usec());
+  EXPECT_EQ(0, event.navigation_id());
+}
+
+TEST_F(SyncInternalsMessageHandlerTest, WriteUserEventBlank) {
+  ListValue args;
   args.AppendString("");
+  args.AppendString("");
+  handler()->HandleWriteUserEvent(&args);
+
+  ASSERT_EQ(1u, fake_user_event_service()->GetRecordedUserEvents().size());
+  const UserEventSpecifics& event =
+      *fake_user_event_service()->GetRecordedUserEvents().begin();
+  EXPECT_EQ(UserEventSpecifics::kTestEvent, event.event_case());
+  EXPECT_TRUE(event.has_event_time_usec());
+  EXPECT_EQ(0, event.event_time_usec());
+  // Should not have a navigation_id because that means something different to
+  // the UserEvents logic.
+  EXPECT_FALSE(event.has_navigation_id());
+}
+
+TEST_F(SyncInternalsMessageHandlerTest, WriteUserEventZero) {
+  ListValue args;
+  args.AppendString("0");
+  args.AppendString("0");
   handler()->HandleWriteUserEvent(&args);
 
   ASSERT_EQ(1u, fake_user_event_service()->GetRecordedUserEvents().size());
-  const sync_pb::UserEventSpecifics& event =
+  const UserEventSpecifics& event =
       *fake_user_event_service()->GetRecordedUserEvents().begin();
+  EXPECT_EQ(UserEventSpecifics::kTestEvent, event.event_case());
+  EXPECT_TRUE(event.has_event_time_usec());
   EXPECT_EQ(0, event.event_time_usec());
+  // Should have a navigation_id, even though the value is 0.
+  EXPECT_TRUE(event.has_navigation_id());
   EXPECT_EQ(0, event.navigation_id());
 }