diff options
author | rlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-01-16 20:59:01 +0000 |
---|---|---|
committer | rlarocque@chromium.org <rlarocque@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-01-16 20:59:01 +0000 |
commit | 34da86a6d837a137631c4d9c98d0d8e6709edb01 (patch) | |
tree | 24789fd85d009a4712cea24abf936745a066babb | |
parent | b4f5dae6842567d21ca5eedca1f1aedcb5387a60 (diff) | |
download | chromium_src-34da86a6d837a137631c4d9c98d0d8e6709edb01.zip chromium_src-34da86a6d837a137631c4d9c98d0d8e6709edb01.tar.gz chromium_src-34da86a6d837a137631c4d9c98d0d8e6709edb01.tar.bz2 |
sync: Remove some WebUI debug functions
This CL removes a number of functions from the sync debugging framework
in order to prepare for some upcoming changes to sync debugging (due to
328606).
This CL removes the following functions:
- GetRootNodeDetails
- GetNodeSummaries
- GetNodeDetails
- GetChildNodeIds
It also adds the function 'getListOfKnownTypes' to help replace some old
functionality required by the 'data' tab and to fix issue 329013.
Rather than having that tab fetch a list of children of the root node,
it now asks for a list of all known data types. This is a better
solution, since it does not require sync to be fully initialized in
order to properly populate the set of checkboxes.
The most significant user of the removed functionality is the node
browser. Its javascript code has been modified in order to transition
it to relying on the getAllNodes function. Rather than fetching node
data on demand by a series of asynchronous callbacks, the tab now
fetches a list of all known sync nodes at once. This has the advantage
of providing a more consistent (though more stale) snapshot. This CL
adds a refresh button to the node browser tab in order to give users
some control over its staleness.
This change had some minor side-effects. The node browser now relies on
items' string-based IDs rather than their metahandles when determining
the nodes' hierarchy. We've also had to add a 'positionIndex' field to
the output of getAllNodes to make it easier for the node browser to sort
its entries. In part to make it easier to fetch this field, most of the
code associated with getAllNodes has been moved into the
syncable::Directory.
In addition to all these changes, this CL adds some webui tests to help
prevent regressions in about:sync functionality. For now, they only
cover the node browser. More tests will be added in future CLs.
BUG=110517,329013,328606
Review URL: https://codereview.chromium.org/134443004
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@245313 0039d316-1c4b-4281-b951-d872f2087c98
24 files changed, 438 insertions, 796 deletions
diff --git a/build/ios/grit_whitelist.txt b/build/ios/grit_whitelist.txt index 1ef7f17..4e43c24 100644 --- a/build/ios/grit_whitelist.txt +++ b/build/ios/grit_whitelist.txt @@ -53,7 +53,6 @@ IDR_SYNC_INTERNALS_DATA_JS IDR_SYNC_INTERNALS_EVENTS_JS IDR_SYNC_INTERNALS_INDEX_HTML IDR_SYNC_INTERNALS_INDEX_JS -IDR_SYNC_INTERNALS_NODE_BROWSER_JS IDR_SYNC_INTERNALS_NOTIFICATIONS_JS IDR_SYNC_INTERNALS_SEARCH_JS IDR_SYNC_INTERNALS_SYNC_LOG_JS diff --git a/chrome/browser/resources/sync_internals/chrome_sync.js b/chrome/browser/resources/sync_internals/chrome_sync.js index 659f78b..fbbada0 100644 --- a/chrome/browser/resources/sync_internals/chrome_sync.js +++ b/chrome/browser/resources/sync_internals/chrome_sync.js @@ -146,15 +146,13 @@ var syncFunctions = [ 'getNotificationState', 'getNotificationInfo', + // Get a static list of available data types. + 'getListOfTypes', + // Client server communication logging functions. 'getClientServerTraffic', - // Node lookup functions. See chrome/browser/sync/engine/syncapi.h - // for docs. - 'getRootNodeDetails', - 'getNodeSummariesById', - 'getNodeDetailsById', - 'getChildNodeIds', + // Get an array containing a JSON representations of all known sync nodes. 'getAllNodes', ]; diff --git a/chrome/browser/resources/sync_internals/data.js b/chrome/browser/resources/sync_internals/data.js index 471759e..0f5c088 100644 --- a/chrome/browser/resources/sync_internals/data.js +++ b/chrome/browser/resources/sync_internals/data.js @@ -164,19 +164,10 @@ function createTypesCheckboxes(types) { }); } -function populateDatatypes(childNodeSummaries) { - var types = childNodeSummaries.map(function(n) { - return n.type; - }); - types = types.sort(); - createTypesCheckboxes(types); -} - document.addEventListener('DOMContentLoaded', function() { - chrome.sync.getRootNodeDetails(function(rootNode) { - chrome.sync.getChildNodeIds(rootNode.id, function(childNodeIds) { - chrome.sync.getNodeSummariesById(childNodeIds, populateDatatypes); - }); + chrome.sync.getListOfTypes(function(types) { + types.sort(); + createTypesCheckboxes(types); }); }); diff --git a/chrome/browser/resources/sync_internals/node_browser.html b/chrome/browser/resources/sync_internals/node_browser.html index b1d9b6a..3b323b0 100644 --- a/chrome/browser/resources/sync_internals/node_browser.html +++ b/chrome/browser/resources/sync_internals/node_browser.html @@ -7,58 +7,52 @@ item detail on the lower right. --> as its container (style.height=100% doesn't work). Also fix behavior when tree is too tall (currently it makes you scroll the entire page). --> - <div id="sync-node-tree-container"> - <tree id="sync-node-tree" icon-visibility="parent"></tree> + <div id="sync-node-browser-refresher"> + <button id="node-browser-refresh-button">Refresh</button> + <div id="node-refresh-status"> + Last refresh time: <span id="node-browser-refresh-time">Never</span> + </div> </div> - <div id="sync-node-splitter"></div> <div id="sync-node-browser-container"> - <table id="node-browser"> - <tr> - <td>ID</td> - <td jscontent="id"></td> - </tr> - <tr> - <td>Modification Time</td> - <td jscontent="modificationTime"></td> - </tr> - <tr> - <td>Parent</td> - <td jsContent="parentId"></td> - </tr> - <tr> - <td>Is Folder</td> - <td jscontent="isFolder"></td> - </tr> - <tr> - <td>Title</td> - <td jscontent="title"></td> - </tr> - <tr> - <td>Type</td> - <td jscontent="type"></td> - </tr> - <tr> - <td>External ID</td> - <td jscontent="externalId"></td> - </tr> - <tr> - <td>Predecessor</td> - <td jscontent="predecessorId"></td> - </tr> - <tr> - <td>Successor</td> - <td jscontent="successorId"></td> - </tr> - <tr> - <td>First Child</td> - <td jscontent="firstChildId"></td> - </tr> - <tr> - <td>Entry</td> - <td><pre jscontent="entry"></pre></td> - </tr> - </table> + <div id="sync-node-tree-container"> + </div> + <div id="sync-node-splitter"></div> + <div id="node-details"> + <table> + <tr> + <td>Title</td> + <td jscontent="NON_UNIQUE_NAME"></td> + </tr> + <tr> + <td>ID</td> + <td jscontent="ID"></td> + </tr> + <tr> + <td>Modification Time</td> + <td jscontent="MTIME"></td> + </tr> + <tr> + <td>Parent</td> + <td jscontent="PARENT_ID"></td> + </tr> + <tr> + <td>Is Folder</td> + <td jscontent="IS_DIR"></td> + </tr> + <tr> + <td>Type</td> + <td jscontent="serverModelType"></td> + </tr> + <tr> + <td>External ID</td> + <td jscontent="LOCAL_EXTERNAL_ID"></td> + </tr> + <tr jsdisplay="$this.hasOwnProperty('positionIndex')"> + <td>Position Index</td> + <td jscontent="positionIndex"></td> + </tr> + </table> + <pre jscontent="JSON.stringify($this, null, 2)"></pre></td> + </div> </div> </div> - -<script src="chrome://sync-internals/node_browser.js"></script> diff --git a/chrome/browser/resources/sync_internals/node_browser.js b/chrome/browser/resources/sync_internals/node_browser.js deleted file mode 100644 index 03f561b..0000000 --- a/chrome/browser/resources/sync_internals/node_browser.js +++ /dev/null @@ -1,6 +0,0 @@ -// Copyright (c) 2011 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -chrome.sync.decorateSyncNodeBrowser('sync-node-tree'); -cr.ui.decorate('#sync-node-splitter', cr.ui.Splitter); diff --git a/chrome/browser/resources/sync_internals/sync_node_browser.css b/chrome/browser/resources/sync_internals/sync_node_browser.css index 33bec66..c669647 100644 --- a/chrome/browser/resources/sync_internals/sync_node_browser.css +++ b/chrome/browser/resources/sync_internals/sync_node_browser.css @@ -2,10 +2,16 @@ * Use of this source code is governed by a BSD-style license that can be * found in the LICENSE file. */ -#sync-node-main { +#sync-node-browser-refresher { + border-bottom: 1px rgb(160,160,160) solid; +} + +#sync-node-browser-refresher > * { + display: inline-block; +} + +#sync-node-browser-container { display: -webkit-box; - /* Should be > #sync-page's min-height. */ - /* TODO(akalin): Find a less hacky way to do this. */ height: 750px; } @@ -17,7 +23,6 @@ max-width: 50%; min-width: 50px; overflow: auto; - padding: 5px; width: 200px; } @@ -42,20 +47,22 @@ </if> } -#sync-node-browser-container { +#sync-node-details-container { -webkit-box-flex: 1; height: 100%; overflow: auto; + visibility: hidden; /* Element is invisible until first refresh. */ } -#node-browser { +#node-details { width: 100%; } -#node-browser td { +#node-details td { vertical-align: top; + white-space: nowrap; } -#node-browser tr:nth-child(odd) { +#node-details tr:nth-child(odd) { background: rgb(239, 243, 255); } diff --git a/chrome/browser/resources/sync_internals/sync_node_browser.js b/chrome/browser/resources/sync_internals/sync_node_browser.js index 8d1ff3d..d20b864 100644 --- a/chrome/browser/resources/sync_internals/sync_node_browser.js +++ b/chrome/browser/resources/sync_internals/sync_node_browser.js @@ -6,52 +6,83 @@ // require: cr/ui.js // require: cr/ui/tree.js -cr.define('chrome.sync', function() { +(function() { /** - * Gets all children of the given node and passes it to the given - * callback. - * @param {string} id The id whose children we want. - * @param {function(Array.<!Object>)} callback The callback to call - * with the list of children summaries. + * A helper function to determine if a node is the root of its type. + * + * @param {!Object} node The node to check. */ - function getSyncNodeChildrenSummaries(id, callback) { - var timer = chrome.sync.makeTimer(); - chrome.sync.getChildNodeIds(id, function(childNodeIds) { - console.debug('getChildNodeIds took ' + - timer.elapsedSeconds + 's to retrieve ' + - childNodeIds.length + ' ids'); - timer = chrome.sync.makeTimer(); - chrome.sync.getNodeSummariesById( - childNodeIds, function(childrenSummaries) { - console.debug('getNodeSummariesById took ' + - timer.elapsedSeconds + 's to retrieve summaries for ' + - childrenSummaries.length + ' nodes'); - callback(childrenSummaries); - }); - }); + var isTypeRootNode = function(node) { + return node.PARENT_ID == 'r' && node.UNIQUE_SERVER_TAG != ''; + } + + /** + * A helper function to determine if a node is a child of the given parent. + * + * @param {string} parentId The ID of the parent. + * @param {!Object} node The node to check. + */ + var isChildOf = function(parentId, node) { + return node.PARENT_ID == parentId; + } + + /** + * A helper function to sort sync nodes. + * + * Sorts by position index if possible, falls back to sorting by name, and + * finally sorting by METAHANDLE. + * + * If this proves to be slow and expensive, we should experiment with moving + * this functionality to C++ instead. + */ + var nodeComparator = function(nodeA, nodeB) { + if (nodeA.hasOwnProperty('positionIndex') && + nodeB.hasOwnProperty('positionIndex')) { + return nodeA.positionIndex - nodeB.positionIndex; + } else if (nodeA.NON_UNIQUE_NAME != nodeB.NON_UNIQUE_NAME) { + return nodeA.NON_UNIQUE_NAME.localeCompare(nodeB.NON_UNIQUE_NAME); + } else { + return nodeA.METAHANDLE - nodeB.METAHANDLE; + } + } + + /** + * Updates the node detail view with the details for the given node. + * @param {!Object} node The struct representing the node we want to display. + */ + function updateNodeDetailView(node) { + var nodeDetailsView = $('node-details'); + nodeDetailsView.hidden = false; + jstProcess(new JsEvalContext(node.entry_), nodeDetailsView); + } + + /** + * Updates the 'Last refresh time' display. + * @param {string} The text to display. + */ + function setLastRefreshTime(str) { + $('node-browser-refresh-time').textContent = str; } /** * Creates a new sync node tree item. - * @param {{id: string, title: string, isFolder: boolean}} - * nodeSummary The nodeSummary object for the node (as returned - * by chrome.sync.getNodeSummariesById()). + * * @constructor + * @param {!Object} node The nodeDetails object for the node as returned by + * chrome.sync.getAllNodes(). * @extends {cr.ui.TreeItem} */ - var SyncNodeTreeItem = function(nodeSummary) { - var treeItem = new cr.ui.TreeItem({ - id_: nodeSummary.id - }); + var SyncNodeTreeItem = function(node) { + var treeItem = new cr.ui.TreeItem(); treeItem.__proto__ = SyncNodeTreeItem.prototype; - treeItem.label = nodeSummary.title; - if (nodeSummary.isFolder) { + treeItem.entry_ = node; + treeItem.label = node.NON_UNIQUE_NAME; + if (node.IS_DIR) { treeItem.mayHaveChildren_ = true; - // Load children asynchronously on expand. - // TODO(akalin): Add a throbber while loading? - treeItem.triggeredLoad_ = false; + // Load children on expand. + treeItem.expanded_ = false; treeItem.addEventListener('expand', treeItem.handleExpand_.bind(treeItem)); } else { @@ -64,55 +95,31 @@ cr.define('chrome.sync', function() { __proto__: cr.ui.TreeItem.prototype, /** - * Retrieves the details for this node. - * @param {function(Object)} callback The callback that will be - * called with the node details, or null if it could not be - * retrieved. + * Finds the children of this node and appends them to the tree. */ - getDetails: function(callback) { - chrome.sync.getNodeDetailsById([this.id_], function(nodeDetails) { - callback(nodeDetails[0] || null); - }); - }, - handleExpand_: function(event) { - if (!this.triggeredLoad_) { - getSyncNodeChildrenSummaries(this.id_, this.addChildNodes_.bind(this)); - this.triggeredLoad_ = true; - } - }, + var treeItem = this; - /** - * Adds children from the list of children summaries. - * @param {Array.<{id: string, title: string, isFolder: boolean}>} - * childrenSummaries The list of children summaries with which - * to create the child nodes. - */ - addChildNodes_: function(childrenSummaries) { - var timer = chrome.sync.makeTimer(); - for (var i = 0; i < childrenSummaries.length; ++i) { - var childTreeItem = new SyncNodeTreeItem(childrenSummaries[i]); - this.add(childTreeItem); + if (treeItem.expanded_) { + return; } - console.debug('adding ' + childrenSummaries.length + - ' children took ' + timer.elapsedSeconds + 's'); - } - }; + treeItem.expanded_ = true; - /** - * Updates the node detail view with the details for the given node. - * @param {!Object} nodeDetails The details for the node we want - * to display. - */ - function updateNodeDetailView(nodeDetails) { - var nodeBrowser = document.getElementById('node-browser'); - // TODO(akalin): Write a nicer detail viewer. - nodeDetails.entry = JSON.stringify(nodeDetails.entry, null, 2); - jstProcess(new JsEvalContext(nodeDetails), nodeBrowser); - } + var children = treeItem.tree.allNodes.filter( + isChildOf.bind(undefined, treeItem.entry_.ID)); + children.sort(nodeComparator); + + children.forEach(function(node) { + treeItem.add(new SyncNodeTreeItem(node)); + }); + }, + }; /** - * Creates a new sync node tree. + * Creates a new sync node tree. Technically, it's a forest since it each + * type has its own root node for its own tree, but it still looks and acts + * mostly like a tree. + * * @param {Object=} opt_propertyBag Optional properties. * @constructor * @extends {cr.ui.Tree} @@ -125,57 +132,78 @@ cr.define('chrome.sync', function() { decorate: function() { cr.ui.Tree.prototype.decorate.call(this); this.addEventListener('change', this.handleChange_.bind(this)); - chrome.sync.getRootNodeDetails(this.makeRoot_.bind(this)); + this.allNodes = []; }, - /** - * Creates the root of the tree. - * @param {{id: string, title: string, isFolder: boolean}} - * rootNodeSummary The summary info for the root node. - */ - makeRoot_: function(rootNodeSummary) { - // The root node usually doesn't have a title. - rootNodeSummary.title = rootNodeSummary.title || 'Root'; - var rootTreeItem = new SyncNodeTreeItem(rootNodeSummary); - this.add(rootTreeItem); + populate: function(nodes) { + var tree = this; + + // We store the full set of nodes in the SyncNodeTree object. + tree.allNodes = nodes; + + var roots = tree.allNodes.filter(isTypeRootNode); + roots.sort(nodeComparator); + + roots.forEach(function(typeRoot) { + tree.add(new SyncNodeTreeItem(typeRoot)); + }); }, handleChange_: function(event) { if (this.selectedItem) { - this.selectedItem.getDetails(updateNodeDetailView); + updateNodeDetailView(this.selectedItem); } } }; - function decorateSyncNodeBrowser(syncNodeBrowser) { - cr.ui.decorate(syncNodeBrowser, SyncNodeTree); + /** + * Clears any existing UI state. Useful prior to a refresh. + */ + function clear() { + var treeContainer = $('sync-node-tree-container'); + while (treeContainer.firstChild) { + treeContainer.removeChild(treeContainer.firstChild); + } + + var nodeDetailsView = $('node-details'); + nodeDetailsView.hidden = true; } - // This is needed because JsTemplate (which is needed by - // updateNodeDetailView) is loaded at the end of the file after - // everything else. - // - // TODO(akalin): Remove dependency on JsTemplate and get rid of - // this. - var domLoaded = false; - var pendingSyncNodeBrowsers = []; - function decorateSyncNodeBrowserAfterDOMLoad(id) { - var e = document.getElementById(id); - if (domLoaded) { - decorateSyncNodeBrowser(e); - } else { - pendingSyncNodeBrowsers.push(e); - } + /** + * Fetch the latest set of nodes and refresh the UI. + */ + function refresh() { + $('node-browser-refresh-button').disabled = true; + + clear(); + setLastRefreshTime('In progress since ' + (new Date()).toLocaleString()); + + chrome.sync.getAllNodes(function(nodes) { + var treeContainer = $('sync-node-tree-container'); + var tree = document.createElement('tree'); + tree.setAttribute('id', 'sync-node-tree'); + tree.setAttribute('icon-visibility', 'parent'); + treeContainer.appendChild(tree); + + cr.ui.decorate(tree, SyncNodeTree); + tree.populate(nodes); + + setLastRefreshTime((new Date()).toLocaleString()); + $('node-browser-refresh-button').disabled = false; + }); } - document.addEventListener('DOMContentLoaded', function() { - for (var i = 0; i < pendingSyncNodeBrowsers.length; ++i) { - decorateSyncNodeBrowser(pendingSyncNodeBrowsers[i]); - } - domLoaded = true; + document.addEventListener('DOMContentLoaded', function(e) { + $('node-browser-refresh-button').addEventListener('click', refresh); + cr.ui.decorate('#sync-node-splitter', cr.ui.Splitter); + + // Automatically trigger a refresh the first time this tab is selected. + $('sync-browser-tab').addEventListener('selectedChange', function f(e) { + if (this.selected) { + $('sync-browser-tab').removeEventListener('selectedChange', f); + refresh(); + } + }); }); - return { - decorateSyncNodeBrowser: decorateSyncNodeBrowserAfterDOMLoad - }; -}); +})(); diff --git a/chrome/browser/resources/sync_internals_resources.grd b/chrome/browser/resources/sync_internals_resources.grd index 3c8f32e..71fe6ab 100644 --- a/chrome/browser/resources/sync_internals_resources.grd +++ b/chrome/browser/resources/sync_internals_resources.grd @@ -22,7 +22,6 @@ <include name="IDR_SYNC_INTERNALS_EVENTS_JS" file="sync_internals/events.js" type="BINDATA" /> <include name="IDR_SYNC_INTERNALS_NOTIFICATIONS_JS" file="sync_internals/notifications.js" type="BINDATA" /> <include name="IDR_SYNC_INTERNALS_SEARCH_JS" file="sync_internals/search.js" type="BINDATA" /> - <include name="IDR_SYNC_INTERNALS_NODE_BROWSER_JS" file="sync_internals/node_browser.js" type="BINDATA" /> <include name="IDR_SYNC_INTERNALS_TRAFFIC_JS" file="sync_internals/traffic.js" type="BINDATA" /> </includes> </release> diff --git a/chrome/browser/ui/webui/sync_internals_browsertest.js b/chrome/browser/ui/webui/sync_internals_browsertest.js index 5a261bf..733bac5 100644 --- a/chrome/browser/ui/webui/sync_internals_browsertest.js +++ b/chrome/browser/ui/webui/sync_internals_browsertest.js @@ -20,7 +20,9 @@ SyncInternalsWebUITest.prototype = { /** @override */ preLoad: function() { - // TODO(zea): mock out the the sync info to fake an active syncer. + this.makeAndRegisterMockHandler([ + 'getAllNodes', + ]); }, /** @@ -48,6 +50,114 @@ SyncInternalsWebUITest.prototype = { } }; +/** Constant hard-coded value to return from mock getAllNodes */ +var HARD_CODED_ALL_NODES = [ +{ + "BASE_SERVER_SPECIFICS": {}, + "BASE_VERSION": "1388699799780000", + "CTIME": "Wednesday, December 31, 1969 4:00:00 PM", + "ID": "sZ:ADqtAZw5kjSwSkukraMoMX6z0OlFXENzhA+1HZNcO6LbATQrkVenHJS5" + + "AgICYfj8/6KcvwlCw3FIvcRFtOEP3zSP5YJ1VH53/Q==", + "IS_DEL": false, + "IS_DIR": true, + "IS_UNAPPLIED_UPDATE": false, + "IS_UNSYNCED": false, + "LOCAL_EXTERNAL_ID": "0", + "META_HANDLE": "376", + "MTIME": "Wednesday, December 31, 1969 4:00:00 PM", + "NON_UNIQUE_NAME": "Typed URLs", + "PARENT_ID": "r", + "SERVER_CTIME": "Wednesday, December 31, 1969 4:00:00 PM", + "SERVER_IS_DEL": false, + "SERVER_IS_DIR": true, + "SERVER_MTIME": "Wednesday, December 31, 1969 4:00:00 PM", + "SERVER_NON_UNIQUE_NAME": "Typed URLs", + "SERVER_PARENT_ID": "r", + "SERVER_SPECIFICS": { + "typed_url": { + "visit_transitions": [], + "visits": [] + } + }, + "SERVER_UNIQUE_POSITION": "INVALID[]", + "SERVER_VERSION": "1388699799780000", + "SPECIFICS": { + "typed_url": { + "visit_transitions": [], + "visits": [] + } + }, + "SYNCING": false, + "TRANSACTION_VERSION": "1", + "UNIQUE_BOOKMARK_TAG": "", + "UNIQUE_CLIENT_TAG": "", + "UNIQUE_POSITION": "INVALID[]", + "UNIQUE_SERVER_TAG": "google_chrome_typed_urls", + "isDirty": false, + "serverModelType": "Typed URLs" +}, +{ + "BASE_SERVER_SPECIFICS": {}, + "BASE_VERSION": "1372291923970334", + "CTIME": "Wednesday, June 26, 2013 5:12:03 PM", + "ID": "sZ:ADqtAZyz70DhOIusPT1v2XCd/8YT8Fy43WlqdRyH6UwoBAqMkX5Pnkl/sW9A" + + "+AVrmzAPWFTrRBf0AWD57HyN4GcYXwSR9q4lYA==", + "IS_DEL": false, + "IS_DIR": false, + "IS_UNAPPLIED_UPDATE": false, + "IS_UNSYNCED": false, + "LOCAL_EXTERNAL_ID": "0", + "META_HANDLE": "3011", + "MTIME": "Wednesday, June 26, 2013 5:12:03 PM", + "NON_UNIQUE_NAME": "http://chrome.com/", + "PARENT_ID": "sZ:ADqtAZw5kjSwSkukraMoMX6z0OlFXENzhA+1HZNcO6LbATQrkVen" + + "HJS5AgICYfj8/6KcvwlCw3FIvcRFtOEP3zSP5YJ1VH53/Q==", + "SERVER_CTIME": "Wednesday, June 26, 2013 5:12:03 PM", + "SERVER_IS_DEL": false, + "SERVER_IS_DIR": false, + "SERVER_MTIME": "Wednesday, June 26, 2013 5:12:03 PM", + "SERVER_NON_UNIQUE_NAME": "http://chrome.com/", + "SERVER_PARENT_ID": "sZ:ADqtAZw5kjSwSkukraMoMX6z0OlFXENzhA+1HZNcO6LbAT" + + "QrkVenHJS5AgICYfj8/6KcvwlCw3FIvcRFtOEP3zSP5YJ1VH53/Q==", + "SERVER_SPECIFICS": { + "typed_url": { + "hidden": false, + "title": "Chrome", + "url": "http://chrome.com/", + "visit_transitions": [ + "268435457" + ], + "visits": [ + "13016765523677321" + ] + } + }, + "SERVER_UNIQUE_POSITION": "INVALID[]", + "SERVER_VERSION": "1372291923970334", + "SPECIFICS": { + "typed_url": { + "hidden": false, + "title": "Chrome", + "url": "http://chrome.com/", + "visit_transitions": [ + "268435457" + ], + "visits": [ + "13016765523677321" + ] + } + }, + "SYNCING": false, + "TRANSACTION_VERSION": "1", + "UNIQUE_BOOKMARK_TAG": "", + "UNIQUE_CLIENT_TAG": "J28uWKpXPuQwR3SJKbuLqzYGOcM=", + "UNIQUE_POSITION": "INVALID[]", + "UNIQUE_SERVER_TAG": "", + "isDirty": false, + "serverModelType": "Typed URLs" +} +]; + TEST_F('SyncInternalsWebUITest', 'Uninitialized', function() { assertNotEquals(null, chrome.sync.aboutInfo); expectTrue(this.hasInDetails(true, 'Username', '')); @@ -77,3 +187,57 @@ TEST_F('SyncInternalsWebUITest', 'SearchTabDoesntChangeOnItemSelect', $('sync-results-list').getListItemByIndex(0).selected = true; expectTrue($('sync-search-tab').selected); }); + +TEST_F('SyncInternalsWebUITest', 'NodeBrowserTest', function() { + this.mockHandler.expects(once()).getAllNodes([]).will( + callFunction(function() { + chrome.sync.getAllNodes.handleReply(HARD_CODED_ALL_NODES); + })); + + // Hit the refresh button. + $('node-browser-refresh-button').click(); + + // Check that the refresh time was updated. + expectNotEquals($('node-browser-refresh-time').textContent, 'Never'); + + // Verify some hard-coded assumptions. These depend on the vaue of the + // hard-coded nodes, specified elsewhere in this file. + + // Start with the tree itself. + var tree = $('sync-node-tree'); + assertEquals(1, tree.items.length); + + // Check the type root and expand it. + var typeRoot = tree.items[0]; + expectFalse(typeRoot.expanded); + typeRoot.expanded = true; + assertEquals(1, typeRoot.items.length); + + // An actual sync node. The child of the type root. + var leaf = typeRoot.items[0]; + + // Verify that selecting it affects the details view. + expectTrue($('node-details').hasAttribute('hidden')); + leaf.selected = true; + expectFalse($('node-details').hasAttribute('hidden')); +}); + +TEST_F('SyncInternalsWebUITest', 'NodeBrowserRefreshOnTabSelect', function() { + this.mockHandler.expects(once()).getAllNodes([]).will( + callFunction(function() { + chrome.sync.getAllNodes.handleReply(HARD_CODED_ALL_NODES); + })); + + // Should start with non-refreshed node browser. + expectEquals($('node-browser-refresh-time').textContent, 'Never'); + + // Selecting the tab will refresh it. + $('sync-browser-tab').selected = true; + expectNotEquals($('node-browser-refresh-time').textContent, 'Never'); + + // Re-selecting the tab shouldn't re-refresh. + $('node-browser-refresh-time').textContent = 'TestCanary'; + $('sync-browser-tab').selected = false; + $('sync-browser-tab').selected = true; + expectEquals($('node-browser-refresh-time').textContent, 'TestCanary'); +}); diff --git a/chrome/browser/ui/webui/sync_internals_message_handler.cc b/chrome/browser/ui/webui/sync_internals_message_handler.cc index 88d7e29..1f03b59 100644 --- a/chrome/browser/ui/webui/sync_internals_message_handler.cc +++ b/chrome/browser/ui/webui/sync_internals_message_handler.cc @@ -19,6 +19,7 @@ using syncer::JsArgList; using syncer::JsEventDetails; using syncer::JsReplyHandler; +using syncer::ModelTypeSet; using syncer::WeakHandle; SyncInternalsMessageHandler::SyncInternalsMessageHandler() @@ -44,18 +45,19 @@ void SyncInternalsMessageHandler::RegisterMessages() { base::Bind(&SyncInternalsMessageHandler::OnGetAboutInfo, base::Unretained(this))); + web_ui()->RegisterMessageCallback( + "getListOfTypes", + base::Bind(&SyncInternalsMessageHandler::OnGetListOfTypes, + base::Unretained(this))); + RegisterJsControllerCallback("getNotificationState"); RegisterJsControllerCallback("getNotificationInfo"); - RegisterJsControllerCallback("getRootNodeDetails"); - RegisterJsControllerCallback("getNodeSummariesById"); - RegisterJsControllerCallback("getNodeDetailsById"); RegisterJsControllerCallback("getAllNodes"); - RegisterJsControllerCallback("getChildNodeIds"); RegisterJsControllerCallback("getClientServerTraffic"); } void SyncInternalsMessageHandler::OnGetAboutInfo(const base::ListValue* args) { - // TODO(rlarocque): We should DCHECK(!args) here. + // TODO(rlarocque): We should DCHECK(!args) here. See crbug.com/334431. scoped_ptr<base::DictionaryValue> value = sync_ui_util::ConstructAboutInformation(GetProfileSyncService()); web_ui()->CallJavascriptFunction( @@ -63,6 +65,20 @@ void SyncInternalsMessageHandler::OnGetAboutInfo(const base::ListValue* args) { *value); } +void SyncInternalsMessageHandler::OnGetListOfTypes( + const base::ListValue* args) { + // TODO(rlarocque): We should DCHECK(!args) here. See crbug.com/334431. + base::ListValue type_list; + ModelTypeSet protocol_types = syncer::ProtocolTypes(); + for (ModelTypeSet::Iterator it = protocol_types.First(); + it.Good(); it.Inc()) { + type_list.Append(new base::StringValue(ModelTypeToString(it.Get()))); + } + web_ui()->CallJavascriptFunction( + "chrome.sync.getListOfTypes.handleReply", + type_list); +} + void SyncInternalsMessageHandler::HandleJsReply( const std::string& name, const JsArgList& args) { DVLOG(1) << "Handling reply for " << name << " message" diff --git a/chrome/browser/ui/webui/sync_internals_message_handler.h b/chrome/browser/ui/webui/sync_internals_message_handler.h index 6fd9714..5b652d5 100644 --- a/chrome/browser/ui/webui/sync_internals_message_handler.h +++ b/chrome/browser/ui/webui/sync_internals_message_handler.h @@ -31,6 +31,7 @@ class SyncInternalsMessageHandler void ForwardToJsController(const std::string& name, const base::ListValue*); void OnGetAboutInfo(const base::ListValue*); + void OnGetListOfTypes(const base::ListValue*); // syncer::JsEventHandler implementation. virtual void HandleJsEvent( diff --git a/chrome/browser/ui/webui/sync_internals_ui.cc b/chrome/browser/ui/webui/sync_internals_ui.cc index eda485b..baf4061 100644 --- a/chrome/browser/ui/webui/sync_internals_ui.cc +++ b/chrome/browser/ui/webui/sync_internals_ui.cc @@ -34,8 +34,6 @@ content::WebUIDataSource* CreateSyncInternalsHTMLSource() { source->AddResourcePath("notifications.js", IDR_SYNC_INTERNALS_NOTIFICATIONS_JS); source->AddResourcePath("search.js", IDR_SYNC_INTERNALS_SEARCH_JS); - source->AddResourcePath("node_browser.js", - IDR_SYNC_INTERNALS_NODE_BROWSER_JS); source->AddResourcePath("traffic.js", IDR_SYNC_INTERNALS_TRAFFIC_JS); source->SetDefaultResource(IDR_SYNC_INTERNALS_INDEX_HTML); return source; diff --git a/sync/internal_api/base_node.cc b/sync/internal_api/base_node.cc index 3f8f2f0..2d0cdd7 100644 --- a/sync/internal_api/base_node.cc +++ b/sync/internal_api/base_node.cc @@ -8,7 +8,6 @@ #include "base/strings/string_number_conversions.h" #include "base/strings/utf_string_conversions.h" -#include "base/values.h" #include "sync/internal_api/public/base_transaction.h" #include "sync/internal_api/syncapi_internal.h" #include "sync/protocol/app_specifics.pb.h" @@ -227,36 +226,8 @@ int BaseNode::GetPositionIndex() const { return GetEntry()->GetPositionIndex(); } -base::DictionaryValue* BaseNode::GetSummaryAsValue() const { - base::DictionaryValue* node_info = new base::DictionaryValue(); - node_info->SetString("id", base::Int64ToString(GetId())); - node_info->SetBoolean("isFolder", GetIsFolder()); - node_info->SetString("title", GetTitle()); - node_info->Set("type", ModelTypeToValue(GetModelType())); - return node_info; -} - -base::DictionaryValue* BaseNode::GetDetailsAsValue() const { - base::DictionaryValue* node_info = GetSummaryAsValue(); - node_info->SetString( - "modificationTime", GetTimeDebugString(GetModificationTime())); - node_info->SetString("parentId", base::Int64ToString(GetParentId())); - // Specifics are already in the Entry value, so no need to duplicate - // it here. - node_info->SetString("externalId", base::Int64ToString(GetExternalId())); - if (GetEntry()->ShouldMaintainPosition() && - !GetEntry()->GetIsDel()) { - node_info->SetString("successorId", base::Int64ToString(GetSuccessorId())); - node_info->SetString( - "predecessorId", base::Int64ToString(GetPredecessorId())); - } - if (GetEntry()->GetIsDir()) { - node_info->SetString( - "firstChildId", base::Int64ToString(GetFirstChildId())); - } - node_info->Set( - "entry", GetEntry()->ToValue(GetTransaction()->GetCryptographer())); - return node_info; +base::DictionaryValue* BaseNode::ToValue() const { + return GetEntry()->ToValue(GetTransaction()->GetCryptographer()); } int64 BaseNode::GetExternalId() const { diff --git a/sync/internal_api/public/base_node.h b/sync/internal_api/public/base_node.h index 4f285dc..04e48d9 100644 --- a/sync/internal_api/public/base_node.h +++ b/sync/internal_api/public/base_node.h @@ -213,13 +213,8 @@ class SYNC_EXPORT BaseNode { virtual const syncable::Entry* GetEntry() const = 0; virtual const BaseTransaction* GetTransaction() const = 0; - // Dumps a summary of node info into a DictionaryValue and returns it. - // Transfers ownership of the DictionaryValue to the caller. - base::DictionaryValue* GetSummaryAsValue() const; - - // Dumps all node details into a DictionaryValue and returns it. - // Transfers ownership of the DictionaryValue to the caller. - base::DictionaryValue* GetDetailsAsValue() const; + // Returns a base::DictionaryValue serialization of this node. + base::DictionaryValue* ToValue() const; protected: BaseNode(); diff --git a/sync/internal_api/public/sync_manager.h b/sync/internal_api/public/sync_manager.h index fdf41b6..de8ee79 100644 --- a/sync/internal_api/public/sync_manager.h +++ b/sync/internal_api/public/sync_manager.h @@ -213,47 +213,6 @@ class SYNC_EXPORT SyncManager : public syncer::InvalidationHandler { */ // function getNotificationState(callback); - /** - * Gets details about the root node. - * - * @param {function(!Object)} callback Called with details about the - * root node. - */ - // TODO(akalin): Change this to getRootNodeId or eliminate it - // entirely. - // function getRootNodeDetails(callback); - - /** - * Gets summary information for a list of ids. - * - * @param {Array.<string>} idList List of 64-bit ids in decimal - * string form. - * @param {Array.<{id: string, title: string, isFolder: boolean}>} - * callback Called with summaries for the nodes in idList that - * exist. - */ - // function getNodeSummariesById(idList, callback); - - /** - * Gets detailed information for a list of ids. - * - * @param {Array.<string>} idList List of 64-bit ids in decimal - * string form. - * @param {Array.<!Object>} callback Called with detailed - * information for the nodes in idList that exist. - */ - // function getNodeDetailsById(idList, callback); - - /** - * Gets child ids for a given id. - * - * @param {string} id 64-bit id in decimal string form of the parent - * node. - * @param {Array.<string>} callback Called with the (possibly empty) - * list of child ids. - */ - // function getChildNodeIds(id); - virtual void OnInitializationComplete( const WeakHandle<JsBackend>& js_backend, const WeakHandle<DataTypeDebugInfoListener>& debug_info_listener, diff --git a/sync/internal_api/sync_manager_impl.cc b/sync/internal_api/sync_manager_impl.cc index 12fd991..9b55e83 100644 --- a/sync/internal_api/sync_manager_impl.cc +++ b/sync/internal_api/sync_manager_impl.cc @@ -191,21 +191,9 @@ SyncManagerImpl::SyncManagerImpl(const std::string& name) "getNotificationInfo", &SyncManagerImpl::GetNotificationInfo); BindJsMessageHandler( - "getRootNodeDetails", - &SyncManagerImpl::GetRootNodeDetails); - BindJsMessageHandler( - "getNodeSummariesById", - &SyncManagerImpl::GetNodeSummariesById); - BindJsMessageHandler( - "getNodeDetailsById", - &SyncManagerImpl::GetNodeDetailsById); - BindJsMessageHandler( "getAllNodes", &SyncManagerImpl::GetAllNodes); BindJsMessageHandler( - "getChildNodeIds", - &SyncManagerImpl::GetChildNodeIds); - BindJsMessageHandler( "getClientServerTraffic", &SyncManagerImpl::GetClientServerTraffic); } @@ -1041,16 +1029,6 @@ JsArgList SyncManagerImpl::GetNotificationInfo( return JsArgList(&return_args); } -JsArgList SyncManagerImpl::GetRootNodeDetails( - const JsArgList& args) { - ReadTransaction trans(FROM_HERE, GetUserShare()); - ReadNode root(&trans); - root.InitByRootLookup(); - base::ListValue return_args; - return_args.Append(root.GetDetailsAsValue()); - return JsArgList(&return_args); -} - JsArgList SyncManagerImpl::GetClientServerTraffic( const JsArgList& args) { base::ListValue return_args; @@ -1060,90 +1038,12 @@ JsArgList SyncManagerImpl::GetClientServerTraffic( return JsArgList(&return_args); } -namespace { - -int64 GetId(const base::ListValue& ids, int i) { - std::string id_str; - if (!ids.GetString(i, &id_str)) { - return kInvalidId; - } - int64 id = kInvalidId; - if (!base::StringToInt64(id_str, &id)) { - return kInvalidId; - } - return id; -} - -JsArgList GetNodeInfoById( - const JsArgList& args, - UserShare* user_share, - base::DictionaryValue* (BaseNode::*info_getter)() const) { - CHECK(info_getter); - base::ListValue return_args; - base::ListValue* node_summaries = new base::ListValue(); - return_args.Append(node_summaries); - const base::ListValue* id_list = NULL; - ReadTransaction trans(FROM_HERE, user_share); - if (args.Get().GetList(0, &id_list)) { - CHECK(id_list); - for (size_t i = 0; i < id_list->GetSize(); ++i) { - int64 id = GetId(*id_list, i); - if (id == kInvalidId) { - continue; - } - ReadNode node(&trans); - if (node.InitByIdLookup(id) != BaseNode::INIT_OK) { - continue; - } - node_summaries->Append((node.*info_getter)()); - } - } - return JsArgList(&return_args); -} - -} // namespace - -JsArgList SyncManagerImpl::GetNodeSummariesById(const JsArgList& args) { - return GetNodeInfoById(args, GetUserShare(), &BaseNode::GetSummaryAsValue); -} - -JsArgList SyncManagerImpl::GetNodeDetailsById(const JsArgList& args) { - return GetNodeInfoById(args, GetUserShare(), &BaseNode::GetDetailsAsValue); -} - JsArgList SyncManagerImpl::GetAllNodes(const JsArgList& args) { - base::ListValue return_args; - base::ListValue* result = new base::ListValue(); - return_args.Append(result); - ReadTransaction trans(FROM_HERE, GetUserShare()); - std::vector<const syncable::EntryKernel*> entry_kernels; - trans.GetDirectory()->GetAllEntryKernels(trans.GetWrappedTrans(), - &entry_kernels); - - for (std::vector<const syncable::EntryKernel*>::const_iterator it = - entry_kernels.begin(); it != entry_kernels.end(); ++it) { - result->Append((*it)->ToValue(trans.GetCryptographer())); - } - - return JsArgList(&return_args); -} - -JsArgList SyncManagerImpl::GetChildNodeIds(const JsArgList& args) { base::ListValue return_args; - base::ListValue* child_ids = new base::ListValue(); - return_args.Append(child_ids); - int64 id = GetId(args.Get(), 0); - if (id != kInvalidId) { - ReadTransaction trans(FROM_HERE, GetUserShare()); - syncable::Directory::Metahandles child_handles; - trans.GetDirectory()->GetChildHandlesByHandle(trans.GetWrappedTrans(), - id, &child_handles); - for (syncable::Directory::Metahandles::const_iterator it = - child_handles.begin(); it != child_handles.end(); ++it) { - child_ids->Append(new base::StringValue(base::Int64ToString(*it))); - } - } + scoped_ptr<base::ListValue> nodes( + trans.GetDirectory()->GetAllNodeDetails(trans.GetWrappedTrans())); + return_args.Append(nodes.release()); return JsArgList(&return_args); } diff --git a/sync/internal_api/sync_manager_impl.h b/sync/internal_api/sync_manager_impl.h index ed758bc..2a71344 100644 --- a/sync/internal_api/sync_manager_impl.h +++ b/sync/internal_api/sync_manager_impl.h @@ -274,7 +274,6 @@ class SYNC_EXPORT_PRIVATE SyncManagerImpl : // JS message handlers. JsArgList GetNotificationState(const JsArgList& args); JsArgList GetNotificationInfo(const JsArgList& args); - JsArgList GetRootNodeDetails(const JsArgList& args); JsArgList GetAllNodes(const JsArgList& args); JsArgList GetNodeSummariesById(const JsArgList& args); JsArgList GetNodeDetailsById(const JsArgList& args); diff --git a/sync/internal_api/sync_manager_impl_unittest.cc b/sync/internal_api/sync_manager_impl_unittest.cc index 57e2a5b..f97b62d 100644 --- a/sync/internal_api/sync_manager_impl_unittest.cc +++ b/sync/internal_api/sync_manager_impl_unittest.cc @@ -97,24 +97,6 @@ using syncable::kEncryptedString; namespace { -void ExpectInt64Value(int64 expected_value, - const base::DictionaryValue& value, - const std::string& key) { - std::string int64_str; - EXPECT_TRUE(value.GetString(key, &int64_str)); - int64 val = 0; - EXPECT_TRUE(base::StringToInt64(int64_str, &val)); - EXPECT_EQ(expected_value, val); -} - -void ExpectTimeValue(const base::Time& expected_value, - const base::DictionaryValue& value, - const std::string& key) { - std::string time_str; - EXPECT_TRUE(value.GetString(key, &time_str)); - EXPECT_EQ(GetTimeDebugString(expected_value), time_str); -} - // Makes a non-folder child of the root node. Returns the id of the // newly-created node. int64 MakeNode(UserShare* share, @@ -526,91 +508,6 @@ TEST_F(SyncApiTest, BaseNodeSetSpecificsPreservesUnknownFields) { EXPECT_FALSE(node.GetEntitySpecifics().unknown_fields().empty()); } -namespace { - -void CheckNodeValue(const BaseNode& node, const base::DictionaryValue& value, - bool is_detailed) { - size_t expected_field_count = 4; - - ExpectInt64Value(node.GetId(), value, "id"); - { - bool is_folder = false; - EXPECT_TRUE(value.GetBoolean("isFolder", &is_folder)); - EXPECT_EQ(node.GetIsFolder(), is_folder); - } - ExpectDictStringValue(node.GetTitle(), value, "title"); - - ModelType expected_model_type = node.GetModelType(); - std::string type_str; - EXPECT_TRUE(value.GetString("type", &type_str)); - if (expected_model_type >= FIRST_REAL_MODEL_TYPE) { - ModelType model_type = ModelTypeFromString(type_str); - EXPECT_EQ(expected_model_type, model_type); - } else if (expected_model_type == TOP_LEVEL_FOLDER) { - EXPECT_EQ("Top-level folder", type_str); - } else if (expected_model_type == UNSPECIFIED) { - EXPECT_EQ("Unspecified", type_str); - } else { - ADD_FAILURE(); - } - - if (is_detailed) { - { - scoped_ptr<base::DictionaryValue> expected_entry( - node.GetEntry()->ToValue(NULL)); - const base::Value* entry = NULL; - EXPECT_TRUE(value.Get("entry", &entry)); - EXPECT_TRUE(base::Value::Equals(entry, expected_entry.get())); - } - - ExpectInt64Value(node.GetParentId(), value, "parentId"); - ExpectTimeValue(node.GetModificationTime(), value, "modificationTime"); - ExpectInt64Value(node.GetExternalId(), value, "externalId"); - expected_field_count += 4; - - if (value.HasKey("predecessorId")) { - ExpectInt64Value(node.GetPredecessorId(), value, "predecessorId"); - expected_field_count++; - } - if (value.HasKey("successorId")) { - ExpectInt64Value(node.GetSuccessorId(), value, "successorId"); - expected_field_count++; - } - if (value.HasKey("firstChildId")) { - ExpectInt64Value(node.GetFirstChildId(), value, "firstChildId"); - expected_field_count++; - } - } - - EXPECT_EQ(expected_field_count, value.size()); -} - -} // namespace - -TEST_F(SyncApiTest, BaseNodeGetSummaryAsValue) { - ReadTransaction trans(FROM_HERE, test_user_share_.user_share()); - ReadNode node(&trans); - node.InitByRootLookup(); - scoped_ptr<base::DictionaryValue> details(node.GetSummaryAsValue()); - if (details) { - CheckNodeValue(node, *details, false); - } else { - ADD_FAILURE(); - } -} - -TEST_F(SyncApiTest, BaseNodeGetDetailsAsValue) { - ReadTransaction trans(FROM_HERE, test_user_share_.user_share()); - ReadNode node(&trans); - node.InitByRootLookup(); - scoped_ptr<base::DictionaryValue> details(node.GetDetailsAsValue()); - if (details) { - CheckNodeValue(node, *details, true); - } else { - ADD_FAILURE(); - } -} - TEST_F(SyncApiTest, EmptyTags) { WriteTransaction trans(FROM_HERE, test_user_share_.user_share()); ReadNode root_node(&trans); @@ -877,6 +774,12 @@ class SyncManagerTest : public testing::Test, (*out)[PRIORITY_PREFERENCES] = GROUP_PASSIVE; } + ModelTypeSet GetEnabledTypes() { + ModelSafeRoutingInfo routing_info; + GetModelSafeRoutingInfo(&routing_info); + return GetRoutingInfoTypes(routing_info); + } + virtual void OnChangesApplied( ModelType model_type, int64 model_version, @@ -1039,247 +942,13 @@ TEST_F(SyncManagerTest, ProcessJsMessage) { StrictMock<MockJsReplyHandler> reply_handler; - base::ListValue disabled_args; - disabled_args.Append(new base::StringValue("TRANSIENT_INVALIDATION_ERROR")); - EXPECT_CALL(reply_handler, - HandleJsReply("getNotificationState", - HasArgsAsList(disabled_args))); + HandleJsReply("getNotificationInfo", _)); // This message should be dropped. SendJsMessage("unknownMessage", kNoArgs, reply_handler.AsWeakHandle()); - SendJsMessage("getNotificationState", kNoArgs, reply_handler.AsWeakHandle()); -} - -TEST_F(SyncManagerTest, ProcessJsMessageGetRootNodeDetails) { - const JsArgList kNoArgs; - - StrictMock<MockJsReplyHandler> reply_handler; - - JsArgList return_args; - - EXPECT_CALL(reply_handler, - HandleJsReply("getRootNodeDetails", _)) - .WillOnce(SaveArg<1>(&return_args)); - - SendJsMessage("getRootNodeDetails", kNoArgs, reply_handler.AsWeakHandle()); - - EXPECT_EQ(1u, return_args.Get().GetSize()); - const base::DictionaryValue* node_info = NULL; - EXPECT_TRUE(return_args.Get().GetDictionary(0, &node_info)); - if (node_info) { - ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare()); - ReadNode node(&trans); - node.InitByRootLookup(); - CheckNodeValue(node, *node_info, true); - } else { - ADD_FAILURE(); - } -} - -void CheckGetNodesByIdReturnArgs(SyncManager* sync_manager, - const JsArgList& return_args, - int64 id, - bool is_detailed) { - EXPECT_EQ(1u, return_args.Get().GetSize()); - const base::ListValue* nodes = NULL; - ASSERT_TRUE(return_args.Get().GetList(0, &nodes)); - ASSERT_TRUE(nodes); - EXPECT_EQ(1u, nodes->GetSize()); - const base::DictionaryValue* node_info = NULL; - EXPECT_TRUE(nodes->GetDictionary(0, &node_info)); - ASSERT_TRUE(node_info); - ReadTransaction trans(FROM_HERE, sync_manager->GetUserShare()); - ReadNode node(&trans); - EXPECT_EQ(BaseNode::INIT_OK, node.InitByIdLookup(id)); - CheckNodeValue(node, *node_info, is_detailed); -} - -class SyncManagerGetNodesByIdTest : public SyncManagerTest { - protected: - virtual ~SyncManagerGetNodesByIdTest() {} - - void RunGetNodesByIdTest(const char* message_name, bool is_detailed) { - int64 root_id = kInvalidId; - { - ReadTransaction trans(FROM_HERE, sync_manager_.GetUserShare()); - ReadNode root_node(&trans); - root_node.InitByRootLookup(); - root_id = root_node.GetId(); - } - - int64 child_id = - MakeNode(sync_manager_.GetUserShare(), BOOKMARKS, "testtag"); - - StrictMock<MockJsReplyHandler> reply_handler; - - JsArgList return_args; - - const int64 ids[] = { root_id, child_id }; - - EXPECT_CALL(reply_handler, - HandleJsReply(message_name, _)) - .Times(arraysize(ids)).WillRepeatedly(SaveArg<1>(&return_args)); - - for (size_t i = 0; i < arraysize(ids); ++i) { - base::ListValue args; - base::ListValue* id_values = new base::ListValue(); - args.Append(id_values); - id_values->Append(new base::StringValue(base::Int64ToString(ids[i]))); - SendJsMessage(message_name, - JsArgList(&args), reply_handler.AsWeakHandle()); - - CheckGetNodesByIdReturnArgs(&sync_manager_, return_args, - ids[i], is_detailed); - } - } - - void RunGetNodesByIdFailureTest(const char* message_name) { - StrictMock<MockJsReplyHandler> reply_handler; - - base::ListValue empty_list_args; - empty_list_args.Append(new base::ListValue()); - - EXPECT_CALL(reply_handler, - HandleJsReply(message_name, - HasArgsAsList(empty_list_args))) - .Times(6); - - { - base::ListValue args; - SendJsMessage(message_name, - JsArgList(&args), reply_handler.AsWeakHandle()); - } - - { - base::ListValue args; - args.Append(new base::ListValue()); - SendJsMessage(message_name, - JsArgList(&args), reply_handler.AsWeakHandle()); - } - - { - base::ListValue args; - base::ListValue* ids = new base::ListValue(); - args.Append(ids); - ids->Append(new base::StringValue(std::string())); - SendJsMessage( - message_name, JsArgList(&args), reply_handler.AsWeakHandle()); - } - - { - base::ListValue args; - base::ListValue* ids = new base::ListValue(); - args.Append(ids); - ids->Append(new base::StringValue("nonsense")); - SendJsMessage(message_name, - JsArgList(&args), reply_handler.AsWeakHandle()); - } - - { - base::ListValue args; - base::ListValue* ids = new base::ListValue(); - args.Append(ids); - ids->Append(new base::StringValue("0")); - SendJsMessage(message_name, - JsArgList(&args), reply_handler.AsWeakHandle()); - } - - { - base::ListValue args; - base::ListValue* ids = new base::ListValue(); - args.Append(ids); - ids->Append(new base::StringValue("9999")); - SendJsMessage(message_name, - JsArgList(&args), reply_handler.AsWeakHandle()); - } - } -}; - -TEST_F(SyncManagerGetNodesByIdTest, GetNodeSummariesById) { - RunGetNodesByIdTest("getNodeSummariesById", false); -} - -TEST_F(SyncManagerGetNodesByIdTest, GetNodeDetailsById) { - RunGetNodesByIdTest("getNodeDetailsById", true); -} - -TEST_F(SyncManagerGetNodesByIdTest, GetNodeSummariesByIdFailure) { - RunGetNodesByIdFailureTest("getNodeSummariesById"); -} - -TEST_F(SyncManagerGetNodesByIdTest, GetNodeDetailsByIdFailure) { - RunGetNodesByIdFailureTest("getNodeDetailsById"); -} - -TEST_F(SyncManagerTest, GetChildNodeIds) { - StrictMock<MockJsReplyHandler> reply_handler; - - JsArgList return_args; - - EXPECT_CALL(reply_handler, - HandleJsReply("getChildNodeIds", _)) - .Times(1).WillRepeatedly(SaveArg<1>(&return_args)); - - { - base::ListValue args; - args.Append(new base::StringValue("1")); - SendJsMessage("getChildNodeIds", - JsArgList(&args), reply_handler.AsWeakHandle()); - } - - EXPECT_EQ(1u, return_args.Get().GetSize()); - const base::ListValue* nodes = NULL; - ASSERT_TRUE(return_args.Get().GetList(0, &nodes)); - ASSERT_TRUE(nodes); - EXPECT_EQ(9u, nodes->GetSize()); -} - -TEST_F(SyncManagerTest, GetChildNodeIdsFailure) { - StrictMock<MockJsReplyHandler> reply_handler; - - base::ListValue empty_list_args; - empty_list_args.Append(new base::ListValue()); - - EXPECT_CALL(reply_handler, - HandleJsReply("getChildNodeIds", - HasArgsAsList(empty_list_args))) - .Times(5); - - { - base::ListValue args; - SendJsMessage("getChildNodeIds", - JsArgList(&args), reply_handler.AsWeakHandle()); - } - - { - base::ListValue args; - args.Append(new base::StringValue(std::string())); - SendJsMessage( - "getChildNodeIds", JsArgList(&args), reply_handler.AsWeakHandle()); - } - - { - base::ListValue args; - args.Append(new base::StringValue("nonsense")); - SendJsMessage("getChildNodeIds", - JsArgList(&args), reply_handler.AsWeakHandle()); - } - - { - base::ListValue args; - args.Append(new base::StringValue("0")); - SendJsMessage("getChildNodeIds", - JsArgList(&args), reply_handler.AsWeakHandle()); - } - - { - base::ListValue args; - args.Append(new base::StringValue("9999")); - SendJsMessage("getChildNodeIds", - JsArgList(&args), reply_handler.AsWeakHandle()); - } + SendJsMessage("getNotificationInfo", kNoArgs, reply_handler.AsWeakHandle()); } TEST_F(SyncManagerTest, GetAllNodesTest) { diff --git a/sync/js/js_test_util.cc b/sync/js/js_test_util.cc index 331efcc..7d40289 100644 --- a/sync/js/js_test_util.cc +++ b/sync/js/js_test_util.cc @@ -87,12 +87,6 @@ class HasDetailsMatcher return ::testing::MakeMatcher(new HasArgsMatcher(expected_args)); } -::testing::Matcher<const JsArgList&> HasArgsAsList( - const base::ListValue& expected_args) { - scoped_ptr<base::ListValue> expected_args_copy(expected_args.DeepCopy()); - return HasArgs(JsArgList(expected_args_copy.get())); -} - ::testing::Matcher<const JsEventDetails&> HasDetails( const JsEventDetails& expected_details) { return ::testing::MakeMatcher(new HasDetailsMatcher(expected_details)); diff --git a/sync/js/js_test_util.h b/sync/js/js_test_util.h index 9cf91fb..8f98248 100644 --- a/sync/js/js_test_util.h +++ b/sync/js/js_test_util.h @@ -35,10 +35,6 @@ void PrintTo(const JsEventDetails& details, ::std::ostream* os); // EXPECT_CALL(mock, HandleJsReply("foo", HasArgs(expected_args))); ::testing::Matcher<const JsArgList&> HasArgs(const JsArgList& expected_args); -// Like HasArgs() but takes a ListValue instead. -::testing::Matcher<const JsArgList&> HasArgsAsList( - const base::ListValue& expected_args); - // A gmock matcher for JsEventDetails. Use like: // // EXPECT_CALL(mock, HandleJsEvent("foo", HasArgs(expected_details))); diff --git a/sync/syncable/directory.cc b/sync/syncable/directory.cc index 33b7e15..d83b1f6 100644 --- a/sync/syncable/directory.cc +++ b/sync/syncable/directory.cc @@ -271,24 +271,6 @@ bool Directory::GetChildHandlesById( return true; } -bool Directory::GetChildHandlesByHandle( - BaseTransaction* trans, int64 handle, - Directory::Metahandles* result) { - if (!SyncAssert(this == trans->directory(), FROM_HERE, - "Directories don't match", trans)) - return false; - - result->clear(); - - ScopedKernelLock lock(this); - EntryKernel* kernel = GetEntryByHandle(handle, &lock); - if (!kernel) - return true; - - AppendChildHandles(lock, kernel->ref(ID), result); - return true; -} - int Directory::GetTotalNodeCount( BaseTransaction* trans, EntryKernel* kernel) const { @@ -887,16 +869,6 @@ void Directory::GetAllMetaHandles(BaseTransaction* trans, } } -void Directory::GetAllEntryKernels(BaseTransaction* trans, - std::vector<const EntryKernel*>* result) { - result->clear(); - ScopedKernelLock lock(this); - for (MetahandlesMap::iterator i = kernel_->metahandles_map.begin(); - i != kernel_->metahandles_map.end(); ++i) { - result->push_back(i->second); - } -} - void Directory::GetUnsyncedMetaHandles(BaseTransaction* trans, Metahandles* result) { result->clear(); @@ -947,6 +919,30 @@ void Directory::CollectMetaHandleCounts( } } +scoped_ptr<base::ListValue> Directory::GetAllNodeDetails( + BaseTransaction* trans) { + scoped_ptr<base::ListValue> nodes(new base::ListValue()); + + ScopedKernelLock lock(this); + for (MetahandlesMap::iterator it = kernel_->metahandles_map.begin(); + it != kernel_->metahandles_map.end(); ++it) { + EntryKernel* kernel = it->second; + scoped_ptr<base::DictionaryValue> node( + kernel->ToValue(GetCryptographer(trans))); + + // Add the position index if appropriate. This must be done here (and not + // in EntryKernel) because the EntryKernel does not have access to its + // siblings. + if (kernel->ShouldMaintainPosition() && !kernel->ref(IS_DEL)) { + node->SetInteger("positionIndex", GetPositionIndex(trans, kernel)); + } + + nodes->Append(node.release()); + } + + return nodes.Pass(); +} + bool Directory::CheckInvariantsOnTransactionClose( syncable::BaseTransaction* trans, const MetahandleSet& modified_handles) { diff --git a/sync/syncable/directory.h b/sync/syncable/directory.h index 0206dbb..c5959b9 100644 --- a/sync/syncable/directory.h +++ b/sync/syncable/directory.h @@ -14,6 +14,7 @@ #include "base/containers/hash_tables.h" #include "base/file_util.h" #include "base/gtest_prod_util.h" +#include "base/values.h" #include "sync/base/sync_export.h" #include "sync/internal_api/public/util/report_unrecoverable_error_function.h" #include "sync/internal_api/public/util/weak_handle.h" @@ -250,12 +251,6 @@ class SYNC_EXPORT Directory { bool GetChildHandlesById(BaseTransaction*, const Id& parent_id, Metahandles* result); - // Returns the child meta handles (even those for deleted/unlinked - // nodes) for given meta handle. Clears |result| if there are no - // children. - bool GetChildHandlesByHandle(BaseTransaction*, int64 handle, - Metahandles* result); - // Counts all items under the given node, including the node itself. int GetTotalNodeCount(BaseTransaction*, EntryKernel* kernel_) const; @@ -302,10 +297,6 @@ class SYNC_EXPORT Directory { // WARNING: THIS METHOD PERFORMS SYNCHRONOUS I/O VIA SQLITE. bool SaveChanges(); - // Fill in |result| with all entry kernels. - void GetAllEntryKernels(BaseTransaction* trans, - std::vector<const EntryKernel*>* result); - // Returns the number of entities with the unsynced bit set. int64 unsynced_entity_count() const; @@ -331,6 +322,8 @@ class SYNC_EXPORT Directory { void CollectMetaHandleCounts(std::vector<int>* num_entries_by_type, std::vector<int>* num_to_delete_entries_by_type); + scoped_ptr<base::ListValue> GetAllNodeDetails(BaseTransaction* trans); + // Sets the level of invariant checking performed after transactions. void SetInvariantCheckLevel(InvariantCheckLevel check_level); diff --git a/sync/syncable/syncable_unittest.cc b/sync/syncable/syncable_unittest.cc index cfcc2db..8f261c7 100644 --- a/sync/syncable/syncable_unittest.cc +++ b/sync/syncable/syncable_unittest.cc @@ -117,14 +117,6 @@ TEST_F(SyncableGeneralTest, General) { ASSERT_EQ(OPENED, dir.Open( "SimpleTest", &delegate_, NullTransactionObserver())); - int64 root_metahandle; - { - ReadTransaction rtrans(FROM_HERE, &dir); - Entry e(&rtrans, GET_BY_ID, rtrans.root_id()); - ASSERT_TRUE(e.good()); - root_metahandle = e.GetMetahandle(); - } - int64 written_metahandle; const Id id = TestIdFactory::FromNumber(99); std::string name = "Jeff"; @@ -137,9 +129,6 @@ TEST_F(SyncableGeneralTest, General) { Directory::Metahandles child_handles; dir.GetChildHandlesById(&rtrans, rtrans.root_id(), &child_handles); EXPECT_TRUE(child_handles.empty()); - - dir.GetChildHandlesByHandle(&rtrans, root_metahandle, &child_handles); - EXPECT_TRUE(child_handles.empty()); } // Test creating a new meta entry. @@ -167,14 +156,6 @@ TEST_F(SyncableGeneralTest, General) { i != child_handles.end(); ++i) { EXPECT_EQ(*i, written_metahandle); } - - dir.GetChildHandlesByHandle(&rtrans, root_metahandle, &child_handles); - EXPECT_EQ(1u, child_handles.size()); - - for (Directory::Metahandles::iterator i = child_handles.begin(); - i != child_handles.end(); ++i) { - EXPECT_EQ(*i, written_metahandle); - } } // Test writing data to an entity. Also check that GET_BY_HANDLE works. diff --git a/sync/tools/sync_client.cc b/sync/tools/sync_client.cc index a165f29..93b80e0 100644 --- a/sync/tools/sync_client.cc +++ b/sync/tools/sync_client.cc @@ -144,7 +144,7 @@ class LoggingChangeDelegate : public SyncManager::ChangeDelegate { if (it->action != ChangeRecord::ACTION_DELETE) { ReadNode node(trans); CHECK_EQ(node.InitByIdLookup(it->id), BaseNode::INIT_OK); - scoped_ptr<base::DictionaryValue> details(node.GetDetailsAsValue()); + scoped_ptr<base::DictionaryValue> details(node.ToValue()); VLOG(1) << "Details: " << ValueToString(*details); } ++i; |