summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--chrome/browser/resources/sync_internals/chrome_sync.js3
-rw-r--r--chrome/browser/resources/sync_internals/sync_node_browser.js41
-rw-r--r--chrome/browser/sync/engine/syncapi.cc89
-rw-r--r--chrome/browser/sync/engine/syncapi.h28
-rw-r--r--chrome/browser/sync/engine/syncapi_unittest.cc177
5 files changed, 263 insertions, 75 deletions
diff --git a/chrome/browser/resources/sync_internals/chrome_sync.js b/chrome/browser/resources/sync_internals/chrome_sync.js
index de1e292..e0eb4cf 100644
--- a/chrome/browser/resources/sync_internals/chrome_sync.js
+++ b/chrome/browser/resources/sync_internals/chrome_sync.js
@@ -140,7 +140,8 @@ var syncFunctions = [
// Node lookup functions.
'getRootNode',
- 'getNodeById',
+ 'getNodesById',
+ 'getChildNodeIds',
'findNodesContainingString'
];
diff --git a/chrome/browser/resources/sync_internals/sync_node_browser.js b/chrome/browser/resources/sync_internals/sync_node_browser.js
index 4939337..3ce4469 100644
--- a/chrome/browser/resources/sync_internals/sync_node_browser.js
+++ b/chrome/browser/resources/sync_internals/sync_node_browser.js
@@ -18,6 +18,23 @@ cr.define('chrome.sync', function() {
// hide all these details.
/**
+ * Returns an object which measures elapsed time.
+ */
+ var makeTimer = function() {
+ var start = new Date();
+
+ return {
+ /**
+ * @return {number} The number of seconds since the timer was
+ * created.
+ */
+ get elapsedSeconds() {
+ return ((new Date()).getTime() - start.getTime()) / 1000.0;
+ }
+ };
+ };
+
+ /**
* Gets all children of the given node and passes it to the given
* callback.
* @param {Object} nodeInfo The info for the node whose children we
@@ -26,16 +43,19 @@ cr.define('chrome.sync', function() {
* children.
*/
function getSyncNodeChildren(nodeInfo, callback) {
- var children = [];
- function processChildInfo(childNodeInfo) {
- if (!childNodeInfo) {
+ var timer = makeTimer();
+ chrome.sync.getChildNodeIds(nodeInfo.id, function(childNodeIds) {
+ console.debug('getChildNodeIds took ' +
+ timer.elapsedSeconds + 's to retrieve ' +
+ childNodeIds.length + ' ids');
+ timer = makeTimer();
+ chrome.sync.getNodesById(childNodeIds, function(children) {
+ console.debug('getNodesById took ' +
+ timer.elapsedSeconds + 's to retrieve ' +
+ children.length + ' nodes');
callback(children);
- return;
- }
- children.push(childNodeInfo);
- chrome.sync.getNodeById(childNodeInfo.successorId, processChildInfo);
- };
- chrome.sync.getNodeById(nodeInfo.firstChildId, processChildInfo);
+ });
+ });
}
/**
@@ -59,10 +79,13 @@ cr.define('chrome.sync', function() {
treeItem.addEventListener('expand', function(event) {
if (!treeItem.triggeredLoad_) {
getSyncNodeChildren(nodeInfo, function(children) {
+ var timer = makeTimer();
for (var i = 0; i < children.length; ++i) {
var childTreeItem = makeNodeTreeItem(children[i]);
treeItem.add(childTreeItem);
}
+ console.debug('adding ' + children.length + ' children took ' +
+ timer.elapsedSeconds + 's');
});
treeItem.triggeredLoad_ = true;
}
diff --git a/chrome/browser/sync/engine/syncapi.cc b/chrome/browser/sync/engine/syncapi.cc
index 8c0eec1..ca11c34 100644
--- a/chrome/browser/sync/engine/syncapi.cc
+++ b/chrome/browser/sync/engine/syncapi.cc
@@ -1548,7 +1548,10 @@ class SyncManager::SyncInternal
void OnIPAddressChangedImpl();
// Functions called by ProcessMessage().
- browser_sync::JsArgList ProcessGetNodeByIdMessage(
+ browser_sync::JsArgList ProcessGetNodesByIdMessage(
+ const browser_sync::JsArgList& args);
+
+ browser_sync::JsArgList ProcessGetChildNodeIdsMessage(
const browser_sync::JsArgList& args);
browser_sync::JsArgList ProcessFindNodesContainingString(
@@ -2719,13 +2722,20 @@ void SyncManager::SyncInternal::ProcessMessage(
return_args.Append(root.ToValue());
parent_router_->RouteJsMessageReply(
name, browser_sync::JsArgList(&return_args), sender);
- } else if (name == "getNodeById") {
+ } else if (name == "getNodesById") {
+ if (!parent_router_) {
+ LogNoRouter(name, args);
+ return;
+ }
+ parent_router_->RouteJsMessageReply(
+ name, ProcessGetNodesByIdMessage(args), sender);
+ } else if (name == "getChildNodeIds") {
if (!parent_router_) {
LogNoRouter(name, args);
return;
}
parent_router_->RouteJsMessageReply(
- name, ProcessGetNodeByIdMessage(args), sender);
+ name, ProcessGetChildNodeIdsMessage(args), sender);
} else if (name == "findNodesContainingString") {
if (!parent_router_) {
LogNoRouter(name, args);
@@ -2758,29 +2768,70 @@ void SyncManager::SyncInternal::RouteJsMessageReply(
parent_router_->RouteJsMessageReply(name, args, target);
}
-browser_sync::JsArgList SyncManager::SyncInternal::ProcessGetNodeByIdMessage(
- const browser_sync::JsArgList& args) {
- ListValue null_return_args_list;
- null_return_args_list.Append(Value::CreateNullValue());
- browser_sync::JsArgList null_return_args(&null_return_args_list);
+namespace {
+
+bool GetId(const ListValue& ids, int i, int64* id) {
std::string id_str;
- if (!args.Get().GetString(0, &id_str)) {
- return null_return_args;
+ if (!ids.GetString(i, &id_str)) {
+ return false;
}
- int64 id;
- if (!base::StringToInt64(id_str, &id)) {
- return null_return_args;
+ if (!base::StringToInt64(id_str, id)) {
+ return false;
}
- if (id == kInvalidId) {
- return null_return_args;
+ if (*id == kInvalidId) {
+ return false;
}
+ return true;
+}
+
+} // namespace
+
+browser_sync::JsArgList SyncManager::SyncInternal::ProcessGetNodesByIdMessage(
+ const browser_sync::JsArgList& args) {
+ ListValue return_args;
+ ListValue* nodes = new ListValue();
+ return_args.Append(nodes);
+ ListValue* id_list = NULL;
ReadTransaction trans(GetUserShare());
- ReadNode node(&trans);
- if (!node.InitByIdLookup(id)) {
- return null_return_args;
+ if (args.Get().GetList(0, &id_list)) {
+ for (size_t i = 0; i < id_list->GetSize(); ++i) {
+ int64 id = kInvalidId;
+ if (!GetId(*id_list, i, &id)) {
+ continue;
+ }
+ ReadNode node(&trans);
+ if (!node.InitByIdLookup(id)) {
+ continue;
+ }
+ nodes->Append(node.ToValue());
+ }
}
+ return browser_sync::JsArgList(&return_args);
+}
+
+browser_sync::JsArgList SyncManager::SyncInternal::
+ ProcessGetChildNodeIdsMessage(
+ const browser_sync::JsArgList& args) {
ListValue return_args;
- return_args.Append(node.ToValue());
+ ListValue* child_ids = new ListValue();
+ return_args.Append(child_ids);
+ int64 id = kInvalidId;
+ if (GetId(args.Get(), 0, &id)) {
+ ReadTransaction trans(GetUserShare());
+ ReadNode node(&trans);
+ if (node.InitByIdLookup(id)) {
+ int64 child_id = node.GetFirstChildId();
+ while (child_id != kInvalidId) {
+ ReadNode child_node(&trans);
+ if (!child_node.InitByIdLookup(child_id)) {
+ break;
+ }
+ child_ids->Append(Value::CreateStringValue(
+ base::Int64ToString(child_id)));
+ child_id = child_node.GetSuccessorId();
+ }
+ }
+ }
return browser_sync::JsArgList(&return_args);
}
diff --git a/chrome/browser/sync/engine/syncapi.h b/chrome/browser/sync/engine/syncapi.h
index f51c574..c454c40 100644
--- a/chrome/browser/sync/engine/syncapi.h
+++ b/chrome/browser/sync/engine/syncapi.h
@@ -979,22 +979,24 @@ class SyncManager {
// The following messages are processed by the returned backend:
//
// getNotificationState():
- // If there is a parent router, sends the
- // onGetNotificationStateFinished(boolean notificationsEnabled)
- // event to |sender| via the parent router with whether or not
- // notifications are enabled.
+ // callback(boolean notificationsEnabled):
+ // notificationsEnabled: whether or not notifications are
+ // enabled.
//
// getRootNode():
- // If there is a parent router, sends the
- // onGetRootNodeFinished(dictionary nodeInfo) event to |sender|
- // via the parent router with information on the root node.
+ // callback(dictionary nodeInfo):
+ // nodeInfo: Information on the root node.
//
- // getNodeById(string id):
- // If there is a parent router, sends the
- // onGetNodeByIdFinished(dictionary nodeInfo) event to |sender|
- // via the parent router with information on the node with the
- // given id (metahandle), if the id is valid and a node with that
- // id exists. Otherwise, calls onGetNodeByIdFinished(null).
+ // getNodesById(array idList):
+ // idList: A list of IDs as strings.
+ // callback(array nodeList):
+ // nodeList: Information on each node for each valid id in
+ // idList. Not guaranteed to be in any order.
+ //
+ // getChildNodeIds(string id):
+ // id: The id of the node for which to return the child node ids.
+ // callback(array idList):
+ // idList: The child node IDs of the node with the given id.
//
// All other messages are dropped.
browser_sync::JsBackend* GetJsBackend();
diff --git a/chrome/browser/sync/engine/syncapi_unittest.cc b/chrome/browser/sync/engine/syncapi_unittest.cc
index ba18eb6..229947b 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();
}