[Sync] Speed up Javascript node operations

chrome/browser/sync/engine/syncapi_unittest.cc

Index: chrome/browser/sync/engine/syncapi_unittest.cc
diff --git a/chrome/browser/sync/engine/syncapi_unittest.cc b/chrome/browser/sync/engine/syncapi_unittest.cc
index dca4954917b4e6fa2d6e6ecd2cbc6816c4a9aadd..f0681b15119aa36c737f83b5a794eec1bc564bc7 100644
--- a/chrome/browser/sync/engine/syncapi_unittest.cc
+++ b/chrome/browser/sync/engine/syncapi_unittest.cc
@@ -871,23 +871,24 @@ TEST_F(SyncManagerTest, ProcessMessageGetRootNode) {
   js_backend->RemoveParentJsEventRouter();
 }
 
-void CheckGetNodeByIdReturnArgs(const SyncManager& sync_manager,
-                                const JsArgList& return_args,
-                                int64 id) {
+void CheckGetNodesByIdReturnArgs(const SyncManager& sync_manager,
+                                 const JsArgList& return_args,
+                                 int64 id) {
   EXPECT_EQ(1u, return_args.Get().GetSize());
+  ListValue* nodes = NULL;
+  ASSERT_TRUE(return_args.Get().GetList(0, &nodes));
+  ASSERT_TRUE(nodes);
+  EXPECT_EQ(1u, nodes->GetSize());
   DictionaryValue* node_info = NULL;
-  EXPECT_TRUE(return_args.Get().GetDictionary(0, &node_info));
-  if (node_info) {
-    ReadTransaction trans(sync_manager.GetUserShare());
-    ReadNode node(&trans);
-    node.InitByIdLookup(id);
-    CheckNodeValue(node, *node_info);
-  } else {
-    ADD_FAILURE();
-  }
+  EXPECT_TRUE(nodes->GetDictionary(0, &node_info));
+  ASSERT_TRUE(node_info);
+  ReadTransaction trans(sync_manager.GetUserShare());
+  ReadNode node(&trans);
+  node.InitByIdLookup(id);
+  CheckNodeValue(node, *node_info);
 }
 
-TEST_F(SyncManagerTest, ProcessMessageGetNodeById) {
+TEST_F(SyncManagerTest, ProcessMessageGetNodesById) {
   int64 child_id =
       MakeNode(sync_manager_.GetUserShare(), syncable::BOOKMARKS, "testtag");
 
@@ -899,7 +900,7 @@ TEST_F(SyncManagerTest, ProcessMessageGetNodeById) {
   JsArgList return_args;
 
   EXPECT_CALL(event_router,
-              RouteJsMessageReply("getNodeById", _, &event_handler))
+              RouteJsMessageReply("getNodesById", _, &event_handler))
       .Times(2).WillRepeatedly(SaveArg<1>(&return_args));
 
   js_backend->SetParentJsEventRouter(&event_router);
@@ -907,70 +908,180 @@ TEST_F(SyncManagerTest, ProcessMessageGetNodeById) {
   // Should trigger the reply.
   {
     ListValue args;
-    args.Append(Value::CreateStringValue("1"));
-    js_backend->ProcessMessage("getNodeById", JsArgList(&args), &event_handler);
+    ListValue* ids = new ListValue();
+    args.Append(ids);
+    ids->Append(Value::CreateStringValue("1"));
+    js_backend->ProcessMessage("getNodesById",
+                               JsArgList(&args), &event_handler);
   }
 
-  CheckGetNodeByIdReturnArgs(sync_manager_, return_args, 1);
+  CheckGetNodesByIdReturnArgs(sync_manager_, return_args, 1);
 
   // Should trigger another reply.
   {
     ListValue args;
-    args.Append(Value::CreateStringValue(base::Int64ToString(child_id)));
-    js_backend->ProcessMessage("getNodeById", JsArgList(&args), &event_handler);
+    ListValue* ids = new ListValue();
+    args.Append(ids);
+    ids->Append(Value::CreateStringValue(base::Int64ToString(child_id)));
+    js_backend->ProcessMessage("getNodesById",
+                               JsArgList(&args), &event_handler);
   }
 
-  CheckGetNodeByIdReturnArgs(sync_manager_, return_args, child_id);
+  CheckGetNodesByIdReturnArgs(sync_manager_, return_args, child_id);
 
   js_backend->RemoveParentJsEventRouter();
 }
 
-TEST_F(SyncManagerTest, ProcessMessageGetNodeByIdFailure) {
+TEST_F(SyncManagerTest, ProcessMessageGetNodesByIdFailure) {
   browser_sync::JsBackend* js_backend = sync_manager_.GetJsBackend();
 
   StrictMock<MockJsEventHandler> event_handler;
   StrictMock<MockJsEventRouter> event_router;
 
-  ListValue null_args;
-  null_args.Append(Value::CreateNullValue());
+  ListValue empty_list_args;
+  empty_list_args.Append(new ListValue());
 
   EXPECT_CALL(event_router,
-              RouteJsMessageReply("getNodeById", HasArgsAsList(null_args),
+              RouteJsMessageReply("getNodesById",
+                                  HasArgsAsList(empty_list_args),
                                   &event_handler))
-      .Times(5);
+      .Times(6);
 
   js_backend->SetParentJsEventRouter(&event_router);
 
   {
     ListValue args;
-    js_backend->ProcessMessage("getNodeById", JsArgList(&args), &event_handler);
+    js_backend->ProcessMessage("getNodesById",
+                               JsArgList(&args), &event_handler);
   }
 
   {
     ListValue args;
-    args.Append(Value::CreateStringValue(""));
-    js_backend->ProcessMessage("getNodeById", JsArgList(&args), &event_handler);
+    args.Append(new ListValue());
+    js_backend->ProcessMessage("getNodesById",
+                               JsArgList(&args), &event_handler);
   }
 
   {
     ListValue args;
-    args.Append(Value::CreateStringValue("nonsense"));
-    js_backend->ProcessMessage("getNodeById", JsArgList(&args), &event_handler);
+    ListValue* ids = new ListValue();
+    args.Append(ids);
+    ids->Append(Value::CreateStringValue(""));
+    js_backend->ProcessMessage("getNodesById",
+                               JsArgList(&args), &event_handler);
+  }
+
+  {
+    ListValue args;
+    ListValue* ids = new ListValue();
+    args.Append(ids);
+    ids->Append(Value::CreateStringValue("nonsense"));
+    js_backend->ProcessMessage("getNodesById",
+                               JsArgList(&args), &event_handler);
+  }
+
+  {
+    ListValue args;
+    ListValue* ids = new ListValue();
+    args.Append(ids);
+    ids->Append(Value::CreateStringValue("0"));
+    js_backend->ProcessMessage("getNodesById",
+                               JsArgList(&args), &event_handler);
+  }
+
+  {
+    ListValue args;
+    ListValue* ids = new ListValue();
+    args.Append(ids);
+    ids->Append(Value::CreateStringValue("9999"));
+    js_backend->ProcessMessage("getNodesById",
+                               JsArgList(&args), &event_handler);
+  }
+
+  js_backend->RemoveParentJsEventRouter();
+}
+
+TEST_F(SyncManagerTest, ProcessMessageGetChildNodeIds) {
+  browser_sync::JsBackend* js_backend = sync_manager_.GetJsBackend();
+
+  StrictMock<MockJsEventHandler> event_handler;
+  StrictMock<MockJsEventRouter> event_router;
+
+  JsArgList return_args;
+
+  EXPECT_CALL(event_router,
+              RouteJsMessageReply("getChildNodeIds", _, &event_handler))
+      .Times(1).WillRepeatedly(SaveArg<1>(&return_args));
+
+  js_backend->SetParentJsEventRouter(&event_router);
+
+  // Should trigger the reply.
+  {
+    ListValue args;
+    args.Append(Value::CreateStringValue("1"));
+    js_backend->ProcessMessage("getChildNodeIds",
+                               JsArgList(&args), &event_handler);
+  }
+
+  EXPECT_EQ(1u, return_args.Get().GetSize());
+  ListValue* nodes = NULL;
+  ASSERT_TRUE(return_args.Get().GetList(0, &nodes));
+  ASSERT_TRUE(nodes);
+  EXPECT_EQ(5u, nodes->GetSize());
+
+  js_backend->RemoveParentJsEventRouter();
+}
+
+TEST_F(SyncManagerTest, ProcessMessageGetChildNodeIdsFailure) {
+  browser_sync::JsBackend* js_backend = sync_manager_.GetJsBackend();
+
+  StrictMock<MockJsEventHandler> event_handler;
+  StrictMock<MockJsEventRouter> event_router;
+
+  ListValue empty_list_args;
+  empty_list_args.Append(new ListValue());
+
+  EXPECT_CALL(event_router,
+              RouteJsMessageReply("getChildNodeIds",
+                                  HasArgsAsList(empty_list_args),
+                                  &event_handler))
+      .Times(5);
+
+  js_backend->SetParentJsEventRouter(&event_router);
+
+  {
+    ListValue args;
+    js_backend->ProcessMessage("getChildNodeIds",
+                               JsArgList(&args), &event_handler);
+  }
+
+  {
+    ListValue args;
+    args.Append(Value::CreateStringValue(""));
+    js_backend->ProcessMessage("getChildNodeIds",
+                               JsArgList(&args), &event_handler);
   }
 
   {
     ListValue args;
     args.Append(Value::CreateStringValue("nonsense"));
-    js_backend->ProcessMessage("getNodeById", JsArgList(&args), &event_handler);
+    js_backend->ProcessMessage("getChildNodeIds",
+                               JsArgList(&args), &event_handler);
   }
 
   {
     ListValue args;
     args.Append(Value::CreateStringValue("0"));
-    js_backend->ProcessMessage("getNodeById", JsArgList(&args), &event_handler);
+    js_backend->ProcessMessage("getChildNodeIds",
+                               JsArgList(&args), &event_handler);
   }
 
-  // TODO(akalin): Figure out how to test InitByIdLookup() failure.
+  {
+    ListValue args;
+    args.Append(Value::CreateStringValue("9999"));
+    js_backend->ProcessMessage("getChildNodeIds",
+                               JsArgList(&args), &event_handler);
+  }
 
   js_backend->RemoveParentJsEventRouter();
 }