diff --git a/browser/base/content/browser-context.inc b/browser/base/content/browser-context.inc index 77de30fb81c..051415af712 100644 --- a/browser/base/content/browser-context.inc +++ b/browser/base/content/browser-context.inc @@ -293,7 +293,7 @@ accesskey="&keywordfield.accesskey;" oncommand="AddKeywordForSearchField();"/> + oncommand="BrowserSearch.loadSearchFromContext(this.searchTerms);"/> 15) - selectedText = selectedText.substr(0,15) + this.ellipsis; - - // Use the current engine if the search bar is visible, the default - // engine otherwise. - var engineName = ""; - var ss = Cc["@mozilla.org/browser/search-service;1"]. - getService(Ci.nsIBrowserSearchService); - if (isElementVisible(BrowserSearch.searchBar)) - engineName = ss.currentEngine.name; - else - engineName = ss.defaultEngine.name; - - // format "Search for " string to show in menu - var menuLabel = gNavigatorBundle.getFormattedString("contextMenuSearch", - [engineName, - selectedText]); - document.getElementById("context-searchselect").label = menuLabel; - document.getElementById("context-searchselect").accessKey = - gNavigatorBundle.getString("contextMenuSearch.accesskey"); - - return true; - }, - // Returns true if anything is selected. isContentSelection: function() { return !document.commandDispatcher.focusedWindow.getSelection().isCollapsed; @@ -1688,5 +1660,34 @@ nsContextMenu.prototype = { if (this.onImage) return this.mediaURL; return ""; + }, + + // Formats the 'Search for ""' context menu. + formatSearchContextItem: function() { + var menuItem = document.getElementById("context-searchselect"); + var selectedText = this.onLink ? this.linkText() : this.textSelected; + + // Store searchTerms in context menu item so we know what to search onclick + menuItem.searchTerms = selectedText; + + if (selectedText.length > 15) + selectedText = selectedText.substr(0,15) + this.ellipsis; + + // Use the current engine if the search bar is visible, the default + // engine otherwise. + var engineName = ""; + var ss = Cc["@mozilla.org/browser/search-service;1"]. + getService(Ci.nsIBrowserSearchService); + if (isElementVisible(BrowserSearch.searchBar)) + engineName = ss.currentEngine.name; + else + engineName = ss.defaultEngine.name; + + // format "Search for " string to show in menu + var menuLabel = gNavigatorBundle.getFormattedString("contextMenuSearch", + [engineName, + selectedText]); + menuItem.label = menuLabel; + menuItem.accessKey = gNavigatorBundle.getString("contextMenuSearch.accesskey"); } }; diff --git a/browser/base/content/test/general/browser.ini b/browser/base/content/test/general/browser.ini index 35361ae4fdf..9e7c16f8ae3 100644 --- a/browser/base/content/test/general/browser.ini +++ b/browser/base/content/test/general/browser.ini @@ -9,6 +9,7 @@ support-files = browser_bug479408_sample.html browser_bug678392-1.html browser_bug678392-2.html + browser_bug970746.xhtml browser_registerProtocolHandler_notification.html browser_star_hsts.sjs browser_tab_dragdrop2_frame1.xul @@ -202,6 +203,7 @@ skip-if = os == "mac" # Intermittent failures, bug 925225 [browser_bug882977.js] [browser_bug902156.js] [browser_bug906190.js] +[browser_bug970746.js] [browser_canonizeURL.js] [browser_contentAreaClick.js] [browser_contextSearchTabPosition.js] diff --git a/browser/base/content/test/general/browser_bug970746.js b/browser/base/content/test/general/browser_bug970746.js new file mode 100644 index 00000000000..0fdb2e327ae --- /dev/null +++ b/browser/base/content/test/general/browser_bug970746.js @@ -0,0 +1,104 @@ +/* Make sure context menu includes option to search hyperlink text on search engine */ + +function test() { + waitForExplicitFinish(); + + gBrowser.selectedTab = gBrowser.addTab(); + + gBrowser.selectedBrowser.addEventListener("load", function() { + gBrowser.selectedBrowser.removeEventListener("load", arguments.callee, true); + + let doc = gBrowser.contentDocument; + let contentAreaContextMenu = document.getElementById("contentAreaContextMenu"); + let ellipsis = "\u2026"; + + // Tests if the "Search for ''" context menu item is shown for the + // given query string of an element. Tests to make sure label includes the proper search terms. + // + // Options: + // + // id: The id of the element to test. + // isSelected: Flag to enable selection (text hilight) the contents of the element + // shouldBeShown: The display state of the menu item + // expectedLabelContents: The menu item label should contain a portion of this string. + // Will only be tested if shouldBeShown is true. + + let testElement = function(opts) { + let element = doc.getElementById(opts.id); + document.popupNode = element; + + let selection = content.getSelection(); + selection.removeAllRanges(); + + if(opts.isSelected) { + selection.selectAllChildren(element); + } + + let contextMenu = new nsContextMenu(contentAreaContextMenu); + let menuItem = document.getElementById("context-searchselect"); + + is(document.getElementById("context-searchselect").hidden, !opts.shouldBeShown, "search context menu item is shown for '#" + opts.id + "' and selected is '" + opts.isSelected + "'"); + + if(opts.shouldBeShown) { + ok(menuItem.label.contains(opts.expectedLabelContents), "Menu item text '" + menuItem.label + "' contains the correct search terms '" + opts.expectedLabelContents + "'"); + } + } + + testElement({ + id: "link", + isSelected: true, + shouldBeShown: true, + expectedLabelContents: "I'm a link!", + }); + testElement({ + id: "link", + isSelected: false, + shouldBeShown: true, + expectedLabelContents: "I'm a link!", + }); + + testElement({ + id: "longLink", + isSelected: true, + shouldBeShown: true, + expectedLabelContents: "I'm a really lo" + ellipsis, + }); + testElement({ + id: "longLink", + isSelected: false, + shouldBeShown: true, + expectedLabelContents: "I'm a really lo" + ellipsis, + }); + + testElement({ + id: "plainText", + isSelected: true, + shouldBeShown: true, + expectedLabelContents: "Right clicking " + ellipsis, + }); + testElement({ + id: "plainText", + isSelected: false, + shouldBeShown: false, + }); + + testElement({ + id: "mixedContent", + isSelected: true, + shouldBeShown: true, + expectedLabelContents: "I'm some text, " + ellipsis, + }); + testElement({ + id: "mixedContent", + isSelected: false, + shouldBeShown: false, + }); + + // cleanup + document.popupNode = null; + gBrowser.removeCurrentTab(); + finish(); + }, true); + + content.location = "http://mochi.test:8888/browser/browser/base/content/test/general/browser_bug970746.xhtml"; +} \ No newline at end of file diff --git a/browser/base/content/test/general/browser_bug970746.xhtml b/browser/base/content/test/general/browser_bug970746.xhtml new file mode 100644 index 00000000000..ef15f17197f --- /dev/null +++ b/browser/base/content/test/general/browser_bug970746.xhtml @@ -0,0 +1,14 @@ + + + + I'm a link! + I'm a really long link and I should be truncated. + + + Right clicking me when I'm selected should show the menu item. + + + I'm some text, and I'm a link! + + + \ No newline at end of file diff --git a/browser/base/content/test/general/test_contextmenu.html b/browser/base/content/test/general/test_contextmenu.html index ace4a9bf3c4..09eb747de60 100644 --- a/browser/base/content/test/general/test_contextmenu.html +++ b/browser/base/content/test/general/test_contextmenu.html @@ -118,7 +118,8 @@ function runTest(testNum) { "---", null, "context-bookmarklink", true, "context-savelink", true, - "context-copylink", true + "context-copylink", true, + "context-searchselect", true ].concat(inspectItems)); } else { checkContextMenu(["context-openlinkintab", true, @@ -126,7 +127,8 @@ function runTest(testNum) { "---", null, "context-bookmarklink", true, "context-savelink", true, - "context-copylink", true + "context-copylink", true, + "context-searchselect", true ].concat(inspectItems)); } closeContextMenu(); @@ -135,7 +137,9 @@ function runTest(testNum) { case 4: // Context menu for text mailto-link - checkContextMenu(["context-copyemail", true].concat(inspectItems)); + checkContextMenu(["context-copyemail", true, + "context-searchselect", true + ].concat(inspectItems)); closeContextMenu(); openContextMenuFor(img); // Invoke context menu for next test. break; diff --git a/browser/components/distribution.js b/browser/components/distribution.js index 347e5efaf5b..170a382ed0f 100644 --- a/browser/components/distribution.js +++ b/browser/components/distribution.js @@ -167,7 +167,7 @@ DistributionCustomizer.prototype = { , index: index , feedURI: this._makeURI(items[iid]["feedLink"]) , siteURI: this._makeURI(items[iid]["siteLink"]) - }); + }).then(null, Cu.reportError); break; case "bookmark": diff --git a/browser/components/places/content/bookmarkProperties.js b/browser/components/places/content/bookmarkProperties.js index 0b2d25ae375..c6155ac23b5 100644 --- a/browser/components/places/content/bookmarkProperties.js +++ b/browser/components/places/content/bookmarkProperties.js @@ -251,24 +251,20 @@ var BookmarkPropertiesPanel = { case "folder": this._itemType = BOOKMARK_FOLDER; - PlacesUtils.livemarks.getLivemark( - { id: this._itemId }, - (function (aStatus, aLivemark) { - if (Components.isSuccessCode(aStatus)) { - this._itemType = LIVEMARK_CONTAINER; - this._feedURI = aLivemark.feedURI; - this._siteURI = aLivemark.siteURI; - this._fillEditProperties(); + PlacesUtils.livemarks.getLivemark({ id: this._itemId }) + .then(aLivemark => { + this._itemType = LIVEMARK_CONTAINER; + this._feedURI = aLivemark.feedURI; + this._siteURI = aLivemark.siteURI; + this._fillEditProperties(); - let acceptButton = document.documentElement.getButton("accept"); - acceptButton.disabled = !this._inputIsValid(); + let acceptButton = document.documentElement.getButton("accept"); + acceptButton.disabled = !this._inputIsValid(); - let newHeight = window.outerHeight + - this._element("descriptionField").boxObject.height; - window.resizeTo(window.outerWidth, newHeight); - } - }).bind(this) - ); + let newHeight = window.outerHeight + + this._element("descriptionField").boxObject.height; + window.resizeTo(window.outerWidth, newHeight); + }, () => undefined); break; } diff --git a/browser/components/places/content/browserPlacesViews.js b/browser/components/places/content/browserPlacesViews.js index 4b86746a757..e476282bf2f 100644 --- a/browser/components/places/content/browserPlacesViews.js +++ b/browser/components/places/content/browserPlacesViews.js @@ -317,21 +317,17 @@ PlacesViewBase.prototype = { element.setAttribute("hostContainer", "true"); } else if (itemId != -1) { - PlacesUtils.livemarks.getLivemark( - { id: itemId }, - function (aStatus, aLivemark) { - if (Components.isSuccessCode(aStatus)) { - element.setAttribute("livemark", "true"); + PlacesUtils.livemarks.getLivemark({ id: itemId }) + .then(aLivemark => { + element.setAttribute("livemark", "true"); #ifdef XP_MACOSX - // OS X native menubar doesn't track list-style-images since - // it doesn't have a frame (bug 733415). Thus enforce updating. - element.setAttribute("image", ""); - element.removeAttribute("image"); + // OS X native menubar doesn't track list-style-images since + // it doesn't have a frame (bug 733415). Thus enforce updating. + element.setAttribute("image", ""); + element.removeAttribute("image"); #endif - this.controller.cacheLivemarkInfo(aPlacesNode, aLivemark); - } - }.bind(this) - ); + this.controller.cacheLivemarkInfo(aPlacesNode, aLivemark); + }, () => undefined); } let popup = document.createElement("menupopup"); @@ -509,16 +505,12 @@ PlacesViewBase.prototype = { #endif } - PlacesUtils.livemarks.getLivemark( - { id: aPlacesNode.itemId }, - function (aStatus, aLivemark) { - if (Components.isSuccessCode(aStatus)) { - // Controller will use this to build the meta data for the node. - this.controller.cacheLivemarkInfo(aPlacesNode, aLivemark); - this.invalidateContainer(aPlacesNode); - } - }.bind(this) - ); + PlacesUtils.livemarks.getLivemark({ id: aPlacesNode.itemId }) + .then(aLivemark => { + // Controller will use this to build the meta data for the node. + this.controller.cacheLivemarkInfo(aPlacesNode, aLivemark); + this.invalidateContainer(aPlacesNode); + }, () => undefined); } }, @@ -647,26 +639,23 @@ PlacesViewBase.prototype = { return; } - PlacesUtils.livemarks.getLivemark({ id: aPlacesNode.itemId }, - function (aStatus, aLivemark) { - if (Components.isSuccessCode(aStatus)) { - let shouldInvalidate = - !this.controller.hasCachedLivemarkInfo(aPlacesNode); - this.controller.cacheLivemarkInfo(aPlacesNode, aLivemark); - if (aNewState == Ci.nsINavHistoryContainerResultNode.STATE_OPENED) { - aLivemark.registerForUpdates(aPlacesNode, this); - // Prioritize the current livemark. - aLivemark.reload(); - PlacesUtils.livemarks.reloadLivemarks(); - if (shouldInvalidate) - this.invalidateContainer(aPlacesNode); - } - else { - aLivemark.unregisterForUpdates(aPlacesNode); - } + PlacesUtils.livemarks.getLivemark({ id: aPlacesNode.itemId }) + .then(aLivemark => { + let shouldInvalidate = + !this.controller.hasCachedLivemarkInfo(aPlacesNode); + this.controller.cacheLivemarkInfo(aPlacesNode, aLivemark); + if (aNewState == Ci.nsINavHistoryContainerResultNode.STATE_OPENED) { + aLivemark.registerForUpdates(aPlacesNode, this); + // Prioritize the current livemark. + aLivemark.reload(); + PlacesUtils.livemarks.reloadLivemarks(); + if (shouldInvalidate) + this.invalidateContainer(aPlacesNode); } - }.bind(this) - ); + else { + aLivemark.unregisterForUpdates(aPlacesNode); + } + }, () => undefined); } } }, @@ -678,10 +667,10 @@ PlacesViewBase.prototype = { if (aPopup._startMarker.nextSibling == aPopup._endMarker) this._setLivemarkStatusMenuItem(aPopup, Ci.mozILivemark.STATUS_LOADING); - PlacesUtils.livemarks.getLivemark({ id: aPopup._placesNode.itemId }, - function (aStatus, aLivemark) { + PlacesUtils.livemarks.getLivemark({ id: aPopup._placesNode.itemId }) + .then(aLivemark => { let placesNode = aPopup._placesNode; - if (!Components.isSuccessCode(aStatus) || !placesNode.containerOpen) + if (!placesNode.containerOpen) return; if (aLivemark.status != Ci.mozILivemark.STATUS_LOADING) @@ -698,8 +687,7 @@ PlacesViewBase.prototype = { else this._getDOMNodeForPlacesNode(child).removeAttribute("visited"); } - }.bind(this) - ); + }, Components.utils.reportError); }, invalidateContainer: function PVB_invalidateContainer(aPlacesNode) { @@ -1008,15 +996,11 @@ PlacesToolbar.prototype = { button.setAttribute("tagContainer", "true"); } else if (PlacesUtils.nodeIsFolder(aChild)) { - PlacesUtils.livemarks.getLivemark( - { id: aChild.itemId }, - function (aStatus, aLivemark) { - if (Components.isSuccessCode(aStatus)) { - button.setAttribute("livemark", "true"); - this.controller.cacheLivemarkInfo(aChild, aLivemark); - } - }.bind(this) - ); + PlacesUtils.livemarks.getLivemark({ id: aChild.itemId }) + .then(aLivemark => { + button.setAttribute("livemark", "true"); + this.controller.cacheLivemarkInfo(aChild, aLivemark); + }, () => undefined); } let popup = document.createElement("menupopup"); @@ -1268,15 +1252,11 @@ PlacesToolbar.prototype = { if (aAnno == PlacesUtils.LMANNO_FEEDURI) { elt.setAttribute("livemark", true); - PlacesUtils.livemarks.getLivemark( - { id: aPlacesNode.itemId }, - function (aStatus, aLivemark) { - if (Components.isSuccessCode(aStatus)) { - this.controller.cacheLivemarkInfo(aPlacesNode, aLivemark); - this.invalidateContainer(aPlacesNode); - } - }.bind(this) - ); + PlacesUtils.livemarks.getLivemark({ id: aPlacesNode.itemId }) + .then(aLivemark => { + this.controller.cacheLivemarkInfo(aPlacesNode, aLivemark); + this.invalidateContainer(aPlacesNode); + }, Components.utils.reportError); } } else { @@ -1840,15 +1820,11 @@ PlacesPanelMenuView.prototype = { button.setAttribute("tagContainer", "true"); } else if (PlacesUtils.nodeIsFolder(aChild)) { - PlacesUtils.livemarks.getLivemark( - { id: aChild.itemId }, - function (aStatus, aLivemark) { - if (Components.isSuccessCode(aStatus)) { - button.setAttribute("livemark", "true"); - this.controller.cacheLivemarkInfo(aChild, aLivemark); - } - }.bind(this) - ); + PlacesUtils.livemarks.getLivemark({ id: aChild.itemId }) + .then(aLivemark => { + button.setAttribute("livemark", "true"); + this.controller.cacheLivemarkInfo(aChild, aLivemark); + }, () => undefined); } } else if (PlacesUtils.nodeIsURI(aChild)) { @@ -1912,15 +1888,11 @@ PlacesPanelMenuView.prototype = { if (aAnno == PlacesUtils.LMANNO_FEEDURI) { elt.setAttribute("livemark", true); - PlacesUtils.livemarks.getLivemark( - { id: aPlacesNode.itemId }, - function (aStatus, aLivemark) { - if (Components.isSuccessCode(aStatus)) { - this.controller.cacheLivemarkInfo(aPlacesNode, aLivemark); - this.invalidateContainer(aPlacesNode); - } - }.bind(this) - ); + PlacesUtils.livemarks.getLivemark({ id: aPlacesNode.itemId }) + .then(aLivemark => { + this.controller.cacheLivemarkInfo(aPlacesNode, aLivemark); + this.invalidateContainer(aPlacesNode); + }, Components.utils.reportError); } }, diff --git a/browser/components/places/content/controller.js b/browser/components/places/content/controller.js index 9f31db66faa..8d7c0dcfe3e 100644 --- a/browser/components/places/content/controller.js +++ b/browser/components/places/content/controller.js @@ -694,14 +694,10 @@ PlacesController.prototype = { var selectedNode = this._view.selectedNode; if (selectedNode) { let itemId = selectedNode.itemId; - PlacesUtils.livemarks.getLivemark( - { id: itemId }, - (function(aStatus, aLivemark) { - if (Components.isSuccessCode(aStatus)) { - aLivemark.reload(true); - } - }).bind(this) - ); + PlacesUtils.livemarks.getLivemark({ id: itemId }) + .then(aLivemark => { + aLivemark.reload(true); + }, Components.utils.reportError); } }, diff --git a/browser/components/places/content/editBookmarkOverlay.js b/browser/components/places/content/editBookmarkOverlay.js index e1105df3766..de587f874fc 100644 --- a/browser/components/places/content/editBookmarkOverlay.js +++ b/browser/components/places/content/editBookmarkOverlay.js @@ -147,17 +147,13 @@ var gEditItemOverlay = { else { this._uri = null; this._isLivemark = false; - PlacesUtils.livemarks.getLivemark( - {id: this._itemId }, - (function (aStatus, aLivemark) { - if (Components.isSuccessCode(aStatus)) { - this._isLivemark = true; - this._initTextField("feedLocationField", aLivemark.feedURI.spec, true); - this._initTextField("siteLocationField", aLivemark.siteURI ? aLivemark.siteURI.spec : "", true); - this._showHideRows(); - } - }).bind(this) - ); + PlacesUtils.livemarks.getLivemark({id: this._itemId }) + .then(aLivemark => { + this._isLivemark = true; + this._initTextField("feedLocationField", aLivemark.feedURI.spec, true); + this._initTextField("siteLocationField", aLivemark.siteURI ? aLivemark.siteURI.spec : "", true); + this._showHideRows(); + }, () => undefined); } // folder picker diff --git a/browser/components/places/content/places.js b/browser/components/places/content/places.js index e68405d7ba7..9c40e781bec 100644 --- a/browser/components/places/content/places.js +++ b/browser/components/places/content/places.js @@ -326,10 +326,13 @@ var PlacesOrganizer = { }, openFlatContainer: function PO_openFlatContainerFlatContainer(aContainer) { - if (aContainer.itemId != -1) + if (aContainer.itemId != -1) { + PlacesUtils.asContainer(this._places.selectedNode).containerOpen = true; this._places.selectItems([aContainer.itemId], false); - else if (PlacesUtils.nodeIsQuery(aContainer)) + } + else if (PlacesUtils.nodeIsQuery(aContainer)) { this._places.selectPlaceURI(aContainer.uri); + } }, /** diff --git a/browser/components/places/content/treeView.js b/browser/components/places/content/treeView.js index 3c9565dbe02..68b389cdfd4 100644 --- a/browser/components/places/content/treeView.js +++ b/browser/components/places/content/treeView.js @@ -786,11 +786,11 @@ PlacesTreeView.prototype = { }, _populateLivemarkContainer: function PTV__populateLivemarkContainer(aNode) { - PlacesUtils.livemarks.getLivemark({ id: aNode.itemId }, - function (aStatus, aLivemark) { + PlacesUtils.livemarks.getLivemark({ id: aNode.itemId }) + .then(aLivemark => { let placesNode = aNode; // Need to check containerOpen since getLivemark is async. - if (!Components.isSuccessCode(aStatus) || !placesNode.containerOpen) + if (!placesNode.containerOpen) return; let children = aLivemark.getNodesForContainer(placesNode); @@ -798,7 +798,7 @@ PlacesTreeView.prototype = { let child = children[i]; this.nodeInserted(placesNode, child, i); } - }.bind(this)); + }, Components.utils.reportError); }, nodeTitleChanged: function PTV_nodeTitleChanged(aNode, aNewTitle) { @@ -847,19 +847,15 @@ PlacesTreeView.prototype = { this._invalidateCellValue(aNode, this.COLUMN_TYPE_DESCRIPTION); } else if (aAnno == PlacesUtils.LMANNO_FEEDURI) { - PlacesUtils.livemarks.getLivemark( - { id: aNode.itemId }, - function (aStatus, aLivemark) { - if (Components.isSuccessCode(aStatus)) { - this._controller.cacheLivemarkInfo(aNode, aLivemark); - let properties = this._cellProperties.get(aNode); - this._cellProperties.set(aNode, properties += " livemark "); + PlacesUtils.livemarks.getLivemark({ id: aNode.itemId }) + .then(aLivemark => { + this._controller.cacheLivemarkInfo(aNode, aLivemark); + let properties = this._cellProperties.get(aNode); + this._cellProperties.set(aNode, properties += " livemark "); - // The livemark attribute is set as a cell property on the title cell. - this._invalidateCellValue(aNode, this.COLUMN_TYPE_TITLE); - } - }.bind(this) - ); + // The livemark attribute is set as a cell property on the title cell. + this._invalidateCellValue(aNode, this.COLUMN_TYPE_TITLE); + }, Components.utils.reportError); } }, @@ -883,26 +879,23 @@ PlacesTreeView.prototype = { return; } - PlacesUtils.livemarks.getLivemark({ id: aNode.itemId }, - function (aStatus, aLivemark) { - if (Components.isSuccessCode(aStatus)) { - let shouldInvalidate = - !this._controller.hasCachedLivemarkInfo(aNode); - this._controller.cacheLivemarkInfo(aNode, aLivemark); - if (aNewState == Components.interfaces.nsINavHistoryContainerResultNode.STATE_OPENED) { - aLivemark.registerForUpdates(aNode, this); - // Prioritize the current livemark. - aLivemark.reload(); - PlacesUtils.livemarks.reloadLivemarks(); - if (shouldInvalidate) - this.invalidateContainer(aNode); - } - else { - aLivemark.unregisterForUpdates(aNode); - } + PlacesUtils.livemarks.getLivemark({ id: aNode.itemId }) + .then(aLivemark => { + let shouldInvalidate = + !this._controller.hasCachedLivemarkInfo(aNode); + this._controller.cacheLivemarkInfo(aNode, aLivemark); + if (aNewState == Components.interfaces.nsINavHistoryContainerResultNode.STATE_OPENED) { + aLivemark.registerForUpdates(aNode, this); + // Prioritize the current livemark. + aLivemark.reload(); + PlacesUtils.livemarks.reloadLivemarks(); + if (shouldInvalidate) + this.invalidateContainer(aNode); } - }.bind(this) - ); + else { + aLivemark.unregisterForUpdates(aNode); + } + }, () => undefined); } }, @@ -1174,17 +1167,13 @@ PlacesTreeView.prototype = { properties += " livemark"; } else { - PlacesUtils.livemarks.getLivemark( - { id: node.itemId }, - function (aStatus, aLivemark) { - if (Components.isSuccessCode(aStatus)) { - this._controller.cacheLivemarkInfo(node, aLivemark); - properties += " livemark"; - // The livemark attribute is set as a cell property on the title cell. - this._invalidateCellValue(node, this.COLUMN_TYPE_TITLE); - } - }.bind(this) - ); + PlacesUtils.livemarks.getLivemark({ id: node.itemId }) + .then(aLivemark => { + this._controller.cacheLivemarkInfo(node, aLivemark); + properties += " livemark"; + // The livemark attribute is set as a cell property on the title cell. + this._invalidateCellValue(node, this.COLUMN_TYPE_TITLE); + }, () => undefined); } } diff --git a/browser/components/places/tests/browser/browser.ini b/browser/components/places/tests/browser/browser.ini index eb4450d4f7c..d1817864eb6 100644 --- a/browser/components/places/tests/browser/browser.ini +++ b/browser/components/places/tests/browser/browser.ini @@ -47,3 +47,4 @@ skip-if = true [browser_library_left_pane_select_hierarchy.js] [browser_435851_copy_query.js] [browser_toolbarbutton_menu_context.js] +[browser_library_openFlatContainer.js] diff --git a/browser/components/places/tests/browser/browser_library_openFlatContainer.js b/browser/components/places/tests/browser/browser_library_openFlatContainer.js new file mode 100644 index 00000000000..167b33031e2 --- /dev/null +++ b/browser/components/places/tests/browser/browser_library_openFlatContainer.js @@ -0,0 +1,42 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ + */ + +/** + * Test opening a flat container in the right pane even if its parent in the + * left pane is closed. + */ + +add_task(function* () { + let folder = PlacesUtils.bookmarks + .createFolder(PlacesUtils.unfiledBookmarksFolderId, + "Folder", + PlacesUtils.bookmarks.DEFAULT_INDEX); + let bookmark = PlacesUtils.bookmarks + .insertBookmark(folder, NetUtil.newURI("http://example.com/"), + PlacesUtils.bookmarks.DEFAULT_INDEX, + "Bookmark"); + + let library = yield promiseLibrary("AllBookmarks"); + registerCleanupFunction(function () { + library.close(); + PlacesUtils.bookmarks.removeFolderChildren(PlacesUtils.unfiledBookmarksFolderId); + }); + + // Select unfiled later, to ensure it's closed. + library.PlacesOrganizer.selectLeftPaneQuery("UnfiledBookmarks"); + ok(!library.PlacesOrganizer._places.selectedNode.containerOpen, + "Unfiled container is closed"); + + let folderNode = library.ContentTree.view.view.nodeForTreeIndex(0); + is(folderNode.itemId, folder, + "Found the expected folder in the right pane"); + // Select the folder node in the right pane. + library.ContentTree.view.selectNode(folderNode); + + synthesizeClickOnSelectedTreeCell(library.ContentTree.view, + { clickCount: 2 }); + + is(library.ContentTree.view.view.nodeForTreeIndex(0).itemId, bookmark, + "Found the expected bookmark in the right pane"); +}); diff --git a/browser/components/places/tests/browser/browser_sidebarpanels_click.js b/browser/components/places/tests/browser/browser_sidebarpanels_click.js index e5eb685cb31..e66b2c2d41d 100644 --- a/browser/components/places/tests/browser/browser_sidebarpanels_click.js +++ b/browser/components/places/tests/browser/browser_sidebarpanels_click.js @@ -119,27 +119,6 @@ function test() { }, true); } - function synthesizeClickOnSelectedTreeCell(aTree) { - let tbo = aTree.treeBoxObject; - is(tbo.view.selection.count, 1, - "The test node should be successfully selected"); - // Get selection rowID. - let min = {}, max = {}; - tbo.view.selection.getRangeAt(0, min, max); - let rowID = min.value; - tbo.ensureRowIsVisible(rowID); - - // Calculate the click coordinates. - let x = {}, y = {}, width = {}, height = {}; - tbo.getCoordsForCellItem(rowID, aTree.columns[0], "text", - x, y, width, height); - x = x.value + width.value / 2; - y = y.value + height.value / 2; - // Simulate the click. - EventUtils.synthesizeMouse(aTree.body, x, y, {}, - aTree.ownerDocument.defaultView); - } - function changeSidebarDirection(aDirection) { sidebar.contentDocument.documentElement.style.direction = aDirection; } diff --git a/browser/components/places/tests/browser/head.js b/browser/components/places/tests/browser/head.js index b4c93984875..05fe2bae090 100644 --- a/browser/components/places/tests/browser/head.js +++ b/browser/components/places/tests/browser/head.js @@ -1,7 +1,11 @@ /* Any copyright is dedicated to the Public Domain. http://creativecommons.org/publicdomain/zero/1.0/ */ -Components.utils.import("resource://gre/modules/NetUtil.jsm"); + +XPCOMUtils.defineLazyModuleGetter(this, "NetUtil", + "resource://gre/modules/NetUtil.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "Promise", + "resource://gre/modules/commonjs/sdk/core/promise.js"); // We need to cache this before test runs... let cachedLeftPaneFolderIdGetter; @@ -182,6 +186,22 @@ function addVisits(aPlaceInfo, aWindow, aCallback, aStack) { ); } -XPCOMUtils.defineLazyModuleGetter(this, "Promise", - "resource://gre/modules/commonjs/sdk/core/promise.js"); - +function synthesizeClickOnSelectedTreeCell(aTree, aOptions) { + let tbo = aTree.treeBoxObject; + if (tbo.view.selection.count != 1) + throw new Error("The test node should be successfully selected"); + // Get selection rowID. + let min = {}, max = {}; + tbo.view.selection.getRangeAt(0, min, max); + let rowID = min.value; + tbo.ensureRowIsVisible(rowID); + // Calculate the click coordinates. + let x = {}, y = {}, width = {}, height = {}; + tbo.getCoordsForCellItem(rowID, aTree.columns[0], "text", + x, y, width, height); + x = x.value + width.value / 2; + y = y.value + height.value / 2; + // Simulate the click. + EventUtils.synthesizeMouse(aTree.body, x, y, aOptions || {}, + aTree.ownerDocument.defaultView); +} diff --git a/browser/devtools/debugger/test/browser_dbg_chrome-create.js b/browser/devtools/debugger/test/browser_dbg_chrome-create.js index b704943d238..a2c8a5cbefd 100644 --- a/browser/devtools/debugger/test/browser_dbg_chrome-create.js +++ b/browser/devtools/debugger/test/browser_dbg_chrome-create.js @@ -11,8 +11,8 @@ Services.prefs.setBoolPref("devtools.debugger.log", true); let gProcess; function test() { - // Windows XP test slaves are terribly slow at this test. - requestLongerTimeout(4); + // Windows XP and 8.1 test slaves are terribly slow at this test. + requestLongerTimeout(5); initChromeDebugger(aOnClose).then(aProcess => { gProcess = aProcess; diff --git a/browser/themes/shared/customizableui/panelUIOverlay.inc.css b/browser/themes/shared/customizableui/panelUIOverlay.inc.css index 1efc1b3f902..2c47d5d2707 100644 --- a/browser/themes/shared/customizableui/panelUIOverlay.inc.css +++ b/browser/themes/shared/customizableui/panelUIOverlay.inc.css @@ -217,10 +217,14 @@ toolbarbutton[sdk-button="true"][cui-areatype="menu-panel"] > .toolbarbutton-ico -moz-box-orient: vertical; width: calc(@menuPanelButtonWidth@ - 2px); height: calc(49px + 2.2em); - margin-top: 3px; /* Hack needed to get type=menu-button to properly align vertically. */ border: 0; } +.panelUI-grid .toolbarbutton-1 > .toolbarbutton-menubutton-button > .toolbarbutton-text, +.panelUI-grid .toolbarbutton-1 > .toolbarbutton-menubutton-button > .toolbarbutton-multiline-text { + margin-top: 2px; /* Hack needed to get the label of type=menu-button aligned with other buttons */ +} + .panel-customization-placeholder-child { margin: 6px 0 0; padding: 2px 6px; diff --git a/layout/reftests/font-face/reftest.list b/layout/reftests/font-face/reftest.list index 90787067c6e..afd0a23a106 100644 --- a/layout/reftests/font-face/reftest.list +++ b/layout/reftests/font-face/reftest.list @@ -141,7 +141,7 @@ HTTP(..) == bug533251.html bug533251-ref.html HTTP(..) == font-familiy-whitespace-1.html font-familiy-whitespace-1-ref.html HTTP(..) != font-familiy-whitespace-1.html font-familiy-whitespace-1-notref.html -skip-if(B2G) fails-if(Android) HTTP(..) == ivs-1.html ivs-1-ref.html # bug 773482 +skip-if(B2G) HTTP(..) == ivs-1.html ivs-1-ref.html # bug 773482 skip-if(B2G) HTTP(..) == missing-names.html missing-names-ref.html # bug 773482 diff --git a/mobile/android/app/mobile.js b/mobile/android/app/mobile.js index c0720cd6bb7..c639e558f42 100644 --- a/mobile/android/app/mobile.js +++ b/mobile/android/app/mobile.js @@ -846,3 +846,7 @@ pref("home.sync.updateMode", 0); // How frequently to check if we should sync home provider data. pref("home.sync.checkIntervalSecs", 3600); + +#ifdef NIGHTLY_BUILD +pref("devtools.debugger.remote-enabled", true); +#endif diff --git a/mobile/android/base/BrowserApp.java b/mobile/android/base/BrowserApp.java index 7cdc49a748e..d2a0e1b035f 100644 --- a/mobile/android/base/BrowserApp.java +++ b/mobile/android/base/BrowserApp.java @@ -42,7 +42,6 @@ import org.mozilla.gecko.home.SearchEngine; import org.mozilla.gecko.menu.GeckoMenu; import org.mozilla.gecko.preferences.GeckoPreferences; import org.mozilla.gecko.prompts.Prompt; -import org.mozilla.gecko.prompts.PromptListItem; import org.mozilla.gecko.sync.setup.SyncAccounts; import org.mozilla.gecko.toolbar.AutocompleteHandler; import org.mozilla.gecko.toolbar.BrowserToolbar; @@ -1447,7 +1446,7 @@ abstract public class BrowserApp extends GeckoApp // If we failed to load a favicon, we use the default favicon instead. Tabs.getInstance() .updateFaviconForURL(pageUrl, - (favicon == null) ? Favicons.sDefaultFavicon : favicon); + (favicon == null) ? Favicons.defaultFavicon : favicon); } }; @@ -1694,16 +1693,22 @@ abstract public class BrowserApp extends GeckoApp mHomePager = (HomePager) homePagerStub.inflate(); final HomeBanner homeBanner = (HomeBanner) findViewById(R.id.home_banner); - mHomePager.setBanner(homeBanner); - // Remove the banner from the view hierarchy if it is dismissed. - homeBanner.setOnDismissListener(new HomeBanner.OnDismissListener() { - @Override - public void onDismiss() { - mHomePager.setBanner(null); - mHomePagerContainer.removeView(homeBanner); - } - }); + // Never show the home banner in guest mode. + if (GeckoProfile.get(this).inGuestMode()) { + mHomePagerContainer.removeView(homeBanner); + } else { + mHomePager.setBanner(homeBanner); + + // Remove the banner from the view hierarchy if it is dismissed. + homeBanner.setOnDismissListener(new HomeBanner.OnDismissListener() { + @Override + public void onDismiss() { + mHomePager.setBanner(null); + mHomePagerContainer.removeView(homeBanner); + } + }); + } } mHomePagerContainer.setVisibility(View.VISIBLE); diff --git a/mobile/android/base/GeckoAppShell.java b/mobile/android/base/GeckoAppShell.java index bb8a6e41777..9ae01f922e8 100644 --- a/mobile/android/base/GeckoAppShell.java +++ b/mobile/android/base/GeckoAppShell.java @@ -1121,10 +1121,13 @@ public class GeckoAppShell /** * Given the inputs to getOpenURIIntent, plus an optional * package name and class name, create and fire an intent to open the - * provided URI. + * provided URI. If a class name is specified but a package name is not, + * we will default to using the current fennec package. * * @param targetURI the string spec of the URI to open. * @param mimeType an optional MIME type string. + * @param packageName an optional app package name. + * @param className an optional intent class name. * @param action an Android action specifier, such as * Intent.ACTION_SEND. * @param title the title to use in ACTION_SEND intents. @@ -1145,8 +1148,13 @@ public class GeckoAppShell return false; } - if (packageName.length() > 0 && className.length() > 0) { - intent.setClassName(packageName, className); + if (!TextUtils.isEmpty(className)) { + if (!TextUtils.isEmpty(packageName)) { + intent.setClassName(packageName, className); + } else { + // Default to using the fennec app context. + intent.setClassName(context, className); + } } intent.setFlags(Intent.FLAG_ACTIVITY_CLEAR_TOP); diff --git a/mobile/android/base/favicons/Favicons.java b/mobile/android/base/favicons/Favicons.java index 6c61e8cfaf5..7ab17e46914 100644 --- a/mobile/android/base/favicons/Favicons.java +++ b/mobile/android/base/favicons/Favicons.java @@ -13,7 +13,6 @@ import org.mozilla.gecko.Tab; import org.mozilla.gecko.Tabs; import org.mozilla.gecko.db.BrowserDB; import org.mozilla.gecko.favicons.cache.FaviconCache; -import org.mozilla.gecko.gfx.BitmapUtils; import org.mozilla.gecko.util.GeckoJarReader; import org.mozilla.gecko.util.NonEvictingLruCache; import org.mozilla.gecko.util.ThreadUtils; @@ -29,12 +28,9 @@ import android.util.SparseArray; import java.io.File; import java.net.URI; import java.net.URISyntaxException; -import java.util.ArrayList; import java.util.Arrays; -import java.util.Collections; import java.util.Iterator; import java.util.List; -import java.util.Set; public class Favicons { private static final String LOGTAG = "GeckoFavicons"; @@ -53,25 +49,25 @@ public class Favicons { public static final int FLAG_PERSIST = 2; public static final int FLAG_SCALE = 4; - protected static Context sContext; + protected static Context context; // The default Favicon to show if no other can be found. - public static Bitmap sDefaultFavicon; + public static Bitmap defaultFavicon; // The density-adjusted default Favicon dimensions. - public static int sDefaultFaviconSize; + public static int defaultFaviconSize; // The density-adjusted maximum Favicon dimensions. - public static int sLargestFaviconSize; + public static int largestFaviconSize; - private static final SparseArray sLoadTasks = new SparseArray(); + private static final SparseArray loadTasks = new SparseArray(); // Cache to hold mappings between page URLs and Favicon URLs. Used to avoid going to the DB when // doing so is not necessary. - private static final NonEvictingLruCache sPageURLMappings = new NonEvictingLruCache(NUM_PAGE_URL_MAPPINGS_TO_STORE); + private static final NonEvictingLruCache pageURLMappings = new NonEvictingLruCache(NUM_PAGE_URL_MAPPINGS_TO_STORE); public static String getFaviconURLForPageURLFromCache(String pageURL) { - return sPageURLMappings.get(pageURL); + return pageURLMappings.get(pageURL); } /** @@ -79,10 +75,10 @@ public class Favicons { * Useful for short-circuiting local database access. */ public static void putFaviconURLForPageURLInCache(String pageURL, String faviconURL) { - sPageURLMappings.put(pageURL, faviconURL); + pageURLMappings.put(pageURL, faviconURL); } - private static FaviconCache sFaviconsCache; + private static FaviconCache faviconsCache; /** * Returns either NOT_LOADING, or LOADED if the onFaviconLoaded call could @@ -117,7 +113,7 @@ public class Favicons { * Returns null otherwise. */ public static Bitmap getSizedFaviconForPageFromCache(final String pageURL, int targetSize) { - final String faviconURL = sPageURLMappings.get(pageURL); + final String faviconURL = pageURLMappings.get(pageURL); if (faviconURL == null) { return null; } @@ -143,7 +139,7 @@ public class Favicons { // Do we know the favicon URL for this page already? String cacheURL = faviconURL; if (cacheURL == null) { - cacheURL = sPageURLMappings.get(pageURL); + cacheURL = pageURLMappings.get(pageURL); } // If there's no favicon URL given, try and hit the cache with the default one. @@ -153,7 +149,7 @@ public class Favicons { // If it's something we can't even figure out a default URL for, just give up. if (cacheURL == null) { - return dispatchResult(pageURL, null, sDefaultFavicon, listener); + return dispatchResult(pageURL, null, defaultFavicon, listener); } Bitmap cachedIcon = getSizedFaviconFromCache(cacheURL, targetSize); @@ -162,8 +158,8 @@ public class Favicons { } // Check if favicon has failed. - if (sFaviconsCache.isFailedFavicon(cacheURL)) { - return dispatchResult(pageURL, cacheURL, sDefaultFavicon, listener); + if (faviconsCache.isFailedFavicon(cacheURL)) { + return dispatchResult(pageURL, cacheURL, defaultFavicon, listener); } // Failing that, try and get one from the database or internet. @@ -181,7 +177,7 @@ public class Favicons { * null if no applicable Favicon exists in the cache. */ public static Bitmap getSizedFaviconFromCache(String faviconURL, int targetSize) { - return sFaviconsCache.getFaviconForDimensions(faviconURL, targetSize); + return faviconsCache.getFaviconForDimensions(faviconURL, targetSize); } /** @@ -201,10 +197,10 @@ public class Favicons { public static int getSizedFaviconForPageFromLocal(final String pageURL, final int targetSize, final OnFaviconLoadedListener callback) { // Firstly, try extremely hard to cheat. // Have we cached this favicon URL? If we did, we can consult the memcache right away. - String targetURL = sPageURLMappings.get(pageURL); + String targetURL = pageURLMappings.get(pageURL); if (targetURL != null) { // Check if favicon has failed. - if (sFaviconsCache.isFailedFavicon(targetURL)) { + if (faviconsCache.isFailedFavicon(targetURL)) { return dispatchResult(pageURL, targetURL, null, callback); } @@ -219,15 +215,15 @@ public class Favicons { // No joy using in-memory resources. Go to background thread and ask the database. LoadFaviconTask task = new LoadFaviconTask(ThreadUtils.getBackgroundHandler(), pageURL, targetURL, 0, callback, targetSize, true); int taskId = task.getId(); - synchronized(sLoadTasks) { - sLoadTasks.put(taskId, task); + synchronized(loadTasks) { + loadTasks.put(taskId, task); } task.execute(); return taskId; } public static int getSizedFaviconForPageFromLocal(final String pageURL, final OnFaviconLoadedListener callback) { - return getSizedFaviconForPageFromLocal(pageURL, sDefaultFaviconSize, callback); + return getSizedFaviconForPageFromLocal(pageURL, defaultFaviconSize, callback); } /** @@ -250,7 +246,7 @@ public class Favicons { } } - targetURL = BrowserDB.getFaviconUrlForHistoryUrl(sContext.getContentResolver(), pageURL); + targetURL = BrowserDB.getFaviconUrlForHistoryUrl(context.getContentResolver(), pageURL); if (targetURL == null) { // Nothing in the history database. Fall back to the default URL and hope for the best. targetURL = guessDefaultFaviconURL(pageURL); @@ -286,8 +282,8 @@ public class Favicons { LoadFaviconTask task = new LoadFaviconTask(ThreadUtils.getBackgroundHandler(), pageUrl, faviconUrl, flags, listener, targetSize, false); int taskId = task.getId(); - synchronized(sLoadTasks) { - sLoadTasks.put(taskId, task); + synchronized(loadTasks) { + loadTasks.put(taskId, task); } task.execute(); @@ -296,7 +292,7 @@ public class Favicons { } public static void putFaviconInMemCache(String pageUrl, Bitmap image) { - sFaviconsCache.putSingleFavicon(pageUrl, image); + faviconsCache.putSingleFavicon(pageUrl, image); } /** @@ -308,7 +304,7 @@ public class Favicons { * @param images An iterator over the new favicons to put in the cache. */ public static void putFaviconsInMemCache(String pageUrl, Iterator images, boolean permanently) { - sFaviconsCache.putFavicons(pageUrl, images, permanently); + faviconsCache.putFavicons(pageUrl, images, permanently); } public static void putFaviconsInMemCache(String pageUrl, Iterator images) { @@ -316,12 +312,12 @@ public class Favicons { } public static void clearMemCache() { - sFaviconsCache.evictAll(); - sPageURLMappings.evictAll(); + faviconsCache.evictAll(); + pageURLMappings.evictAll(); } public static void putFaviconInFailedCache(String faviconURL) { - sFaviconsCache.putFailed(faviconURL); + faviconsCache.putFailed(faviconURL); } public static boolean cancelFaviconLoad(int taskId) { @@ -330,13 +326,14 @@ public class Favicons { } boolean cancelled; - synchronized (sLoadTasks) { - if (sLoadTasks.indexOfKey(taskId) < 0) + synchronized (loadTasks) { + if (loadTasks.indexOfKey(taskId) < 0) { return false; + } Log.d(LOGTAG, "Cancelling favicon load (" + taskId + ")"); - LoadFaviconTask task = sLoadTasks.get(taskId); + LoadFaviconTask task = loadTasks.get(taskId); cancelled = task.cancel(false); } return cancelled; @@ -346,12 +343,12 @@ public class Favicons { Log.d(LOGTAG, "Closing Favicons database"); // Cancel any pending tasks - synchronized (sLoadTasks) { - final int count = sLoadTasks.size(); + synchronized (loadTasks) { + final int count = loadTasks.size(); for (int i = 0; i < count; i++) { - cancelFaviconLoad(sLoadTasks.keyAt(i)); + cancelFaviconLoad(loadTasks.keyAt(i)); } - sLoadTasks.clear(); + loadTasks.clear(); } LoadFaviconTask.closeHTTPClient(); @@ -364,7 +361,7 @@ public class Favicons { * @return The dominant colour of the provided Favicon. */ public static int getFaviconColor(String url) { - return sFaviconsCache.getDominantColor(url); + return faviconsCache.getDominantColor(url); } /** @@ -376,24 +373,24 @@ public class Favicons { */ public static void attachToContext(Context context) throws Exception { final Resources res = context.getResources(); - sContext = context; + Favicons.context = context; // Decode the default Favicon ready for use. - sDefaultFavicon = BitmapFactory.decodeResource(res, R.drawable.favicon); - if (sDefaultFavicon == null) { + defaultFavicon = BitmapFactory.decodeResource(res, R.drawable.favicon); + if (defaultFavicon == null) { throw new Exception("Null default favicon was returned from the resources system!"); } - sDefaultFaviconSize = res.getDimensionPixelSize(R.dimen.favicon_bg); + defaultFaviconSize = res.getDimensionPixelSize(R.dimen.favicon_bg); // Screen-density-adjusted upper limit on favicon size. Favicons larger than this are // downscaled to this size or discarded. - sLargestFaviconSize = context.getResources().getDimensionPixelSize(R.dimen.favicon_largest_interesting_size); - sFaviconsCache = new FaviconCache(FAVICON_CACHE_SIZE_BYTES, sLargestFaviconSize); + largestFaviconSize = context.getResources().getDimensionPixelSize(R.dimen.favicon_largest_interesting_size); + faviconsCache = new FaviconCache(FAVICON_CACHE_SIZE_BYTES, largestFaviconSize); // Initialize page mappings for each of our special pages. for (String url : AboutPages.getDefaultIconPages()) { - sPageURLMappings.putWithoutEviction(url, BUILT_IN_FAVICON_URL); + pageURLMappings.putWithoutEviction(url, BUILT_IN_FAVICON_URL); } // Load and cache the built-in favicon in each of its sizes. @@ -453,8 +450,8 @@ public class Favicons { } public static void removeLoadTask(int taskId) { - synchronized(sLoadTasks) { - sLoadTasks.delete(taskId); + synchronized(loadTasks) { + loadTasks.delete(taskId); } } @@ -464,7 +461,7 @@ public class Favicons { * @param faviconURL Favicon URL to check for failure. */ static boolean isFailedFavicon(String faviconURL) { - return sFaviconsCache.isFailedFavicon(faviconURL); + return faviconsCache.isFailedFavicon(faviconURL); } /** diff --git a/mobile/android/base/favicons/LoadFaviconTask.java b/mobile/android/base/favicons/LoadFaviconTask.java index 68f2e52fc65..ce5d41a55b2 100644 --- a/mobile/android/base/favicons/LoadFaviconTask.java +++ b/mobile/android/base/favicons/LoadFaviconTask.java @@ -22,7 +22,7 @@ import org.mozilla.gecko.favicons.decoders.LoadFaviconResult; import org.mozilla.gecko.util.GeckoJarReader; import org.mozilla.gecko.util.ThreadUtils; import org.mozilla.gecko.util.UiAsyncTask; -import static org.mozilla.gecko.favicons.Favicons.sContext; +import static org.mozilla.gecko.favicons.Favicons.context; import java.io.IOException; import java.io.InputStream; @@ -53,21 +53,21 @@ public class LoadFaviconTask extends UiAsyncTask { // by the server. private static final int DEFAULT_FAVICON_BUFFER_SIZE = 25000; - private static AtomicInteger mNextFaviconLoadId = new AtomicInteger(0); - private int mId; - private String mPageUrl; - private String mFaviconUrl; - private OnFaviconLoadedListener mListener; - private int mFlags; + private static AtomicInteger nextFaviconLoadId = new AtomicInteger(0); + private int id; + private String pageUrl; + private String faviconURL; + private OnFaviconLoadedListener listener; + private int flags; - private final boolean mOnlyFromLocal; + private final boolean onlyFromLocal; // Assuming square favicons, judging by width only is acceptable. - protected int mTargetWidth; - private LinkedList mChainees; - private boolean mIsChaining; + protected int targetWidth; + private LinkedList chainees; + private boolean isChaining; - static AndroidHttpClient sHttpClient = AndroidHttpClient.newInstance(GeckoAppShell.getGeckoInterface().getDefaultUAString()); + static AndroidHttpClient httpClient = AndroidHttpClient.newInstance(GeckoAppShell.getGeckoInterface().getDefaultUAString()); public LoadFaviconTask(Handler backgroundThreadHandler, String pageUrl, String faviconUrl, int flags, @@ -76,33 +76,33 @@ public class LoadFaviconTask extends UiAsyncTask { } public LoadFaviconTask(Handler backgroundThreadHandler, String pageUrl, String faviconUrl, int flags, - OnFaviconLoadedListener aListener, int targetSize, boolean fromLocal) { + OnFaviconLoadedListener listener, int targetWidth, boolean onlyFromLocal) { super(backgroundThreadHandler); - mId = mNextFaviconLoadId.incrementAndGet(); + id = nextFaviconLoadId.incrementAndGet(); - mPageUrl = pageUrl; - mFaviconUrl = faviconUrl; - mListener = aListener; - mFlags = flags; - mTargetWidth = targetSize; - mOnlyFromLocal = fromLocal; + this.pageUrl = pageUrl; + this.faviconURL = faviconUrl; + this.listener = listener; + this.flags = flags; + this.targetWidth = targetWidth; + this.onlyFromLocal = onlyFromLocal; } // Runs in background thread private LoadFaviconResult loadFaviconFromDb() { - ContentResolver resolver = sContext.getContentResolver(); - return BrowserDB.getFaviconForFaviconUrl(resolver, mFaviconUrl); + ContentResolver resolver = context.getContentResolver(); + return BrowserDB.getFaviconForFaviconUrl(resolver, faviconURL); } // Runs in background thread private void saveFaviconToDb(final byte[] encodedFavicon) { - if ((mFlags & FLAG_PERSIST) == 0) { + if ((flags & FLAG_PERSIST) == 0) { return; } - ContentResolver resolver = sContext.getContentResolver(); - BrowserDB.updateFaviconForUrl(resolver, mPageUrl, encodedFavicon, mFaviconUrl); + ContentResolver resolver = context.getContentResolver(); + BrowserDB.updateFaviconForUrl(resolver, pageUrl, encodedFavicon, faviconURL); } /** @@ -121,7 +121,7 @@ public class LoadFaviconTask extends UiAsyncTask { } HttpGet request = new HttpGet(faviconURI); - HttpResponse response = sHttpClient.execute(request); + HttpResponse response = httpClient.execute(request); if (response == null) { return null; } @@ -172,7 +172,7 @@ public class LoadFaviconTask extends UiAsyncTask { if (uri.startsWith("jar:jar:")) { Log.d(LOGTAG, "Fetching favicon from JAR."); try { - return GeckoJarReader.getBitmap(sContext.getResources(), uri); + return GeckoJarReader.getBitmap(context.getResources(), uri); } catch (Exception e) { // Just about anything could happen here. Log.w(LOGTAG, "Error fetching favicon from JAR.", e); @@ -287,27 +287,27 @@ public class LoadFaviconTask extends UiAsyncTask { // Handle the case of malformed favicon URL. // If favicon is empty, fall back to the stored one. - if (TextUtils.isEmpty(mFaviconUrl)) { + if (TextUtils.isEmpty(faviconURL)) { // Try to get the favicon URL from the memory cache. - storedFaviconUrl = Favicons.getFaviconURLForPageURLFromCache(mPageUrl); + storedFaviconUrl = Favicons.getFaviconURLForPageURLFromCache(pageUrl); // If that failed, try to get the URL from the database. if (storedFaviconUrl == null) { - storedFaviconUrl = Favicons.getFaviconURLForPageURL(mPageUrl); + storedFaviconUrl = Favicons.getFaviconURLForPageURL(pageUrl); if (storedFaviconUrl != null) { // If that succeeded, cache the URL loaded from the database in memory. - Favicons.putFaviconURLForPageURLInCache(mPageUrl, storedFaviconUrl); + Favicons.putFaviconURLForPageURLInCache(pageUrl, storedFaviconUrl); } } // If we found a faviconURL - use it. if (storedFaviconUrl != null) { - mFaviconUrl = storedFaviconUrl; + faviconURL = storedFaviconUrl; } else { // If we don't have a stored one, fall back to the default. - mFaviconUrl = Favicons.guessDefaultFaviconURL(mPageUrl); + faviconURL = Favicons.guessDefaultFaviconURL(pageUrl); - if (TextUtils.isEmpty(mFaviconUrl)) { + if (TextUtils.isEmpty(faviconURL)) { return null; } isUsingDefaultURL = true; @@ -316,7 +316,7 @@ public class LoadFaviconTask extends UiAsyncTask { // Check if favicon has failed - if so, give up. We need this check because, sometimes, we // didn't know the real Favicon URL until we asked the database. - if (Favicons.isFailedFavicon(mFaviconUrl)) { + if (Favicons.isFailedFavicon(faviconURL)) { return null; } @@ -329,10 +329,10 @@ public class LoadFaviconTask extends UiAsyncTask { // If there is, just join the queue and wait for it to finish. If not, we carry on. synchronized(loadsInFlight) { // Another load of the current Favicon is already underway - LoadFaviconTask existingTask = loadsInFlight.get(mFaviconUrl); + LoadFaviconTask existingTask = loadsInFlight.get(faviconURL); if (existingTask != null && !existingTask.isCancelled()) { existingTask.chainTasks(this); - mIsChaining = true; + isChaining = true; // If we are chaining, we want to keep the first task started to do this job as the one // in the hashmap so subsequent tasks will add themselves to its chaining list. @@ -341,7 +341,7 @@ public class LoadFaviconTask extends UiAsyncTask { // We do not want to update the hashmap if the task has chained - other tasks need to // chain onto the same parent task. - loadsInFlight.put(mFaviconUrl, this); + loadsInFlight.put(faviconURL, this); } if (isCancelled()) { @@ -354,20 +354,20 @@ public class LoadFaviconTask extends UiAsyncTask { return pushToCacheAndGetResult(loadedBitmaps); } - if (mOnlyFromLocal || isCancelled()) { + if (onlyFromLocal || isCancelled()) { return null; } // Let's see if it's in a JAR. - image = fetchJARFavicon(mFaviconUrl); + image = fetchJARFavicon(faviconURL); if (imageIsValid(image)) { // We don't want to put this into the DB. - Favicons.putFaviconInMemCache(mFaviconUrl, image); + Favicons.putFaviconInMemCache(faviconURL, image); return image; } try { - loadedBitmaps = downloadFavicon(new URI(mFaviconUrl)); + loadedBitmaps = downloadFavicon(new URI(faviconURL)); } catch (URISyntaxException e) { Log.e(LOGTAG, "The provided favicon URL is not valid"); return null; @@ -381,7 +381,7 @@ public class LoadFaviconTask extends UiAsyncTask { } if (isUsingDefaultURL) { - Favicons.putFaviconInFailedCache(mFaviconUrl); + Favicons.putFaviconInFailedCache(faviconURL); return null; } @@ -390,16 +390,16 @@ public class LoadFaviconTask extends UiAsyncTask { } // If we're not already trying the default URL, try it now. - final String guessed = Favicons.guessDefaultFaviconURL(mPageUrl); + final String guessed = Favicons.guessDefaultFaviconURL(pageUrl); if (guessed == null) { - Favicons.putFaviconInFailedCache(mFaviconUrl); + Favicons.putFaviconInFailedCache(faviconURL); return null; } image = fetchJARFavicon(guessed); if (imageIsValid(image)) { // We don't want to put this into the DB. - Favicons.putFaviconInMemCache(mFaviconUrl, image); + Favicons.putFaviconInMemCache(faviconURL, image); return image; } @@ -428,8 +428,8 @@ public class LoadFaviconTask extends UiAsyncTask { * we are under extreme memory pressure and find ourselves dropping the cache immediately. */ private Bitmap pushToCacheAndGetResult(LoadFaviconResult loadedBitmaps) { - Favicons.putFaviconsInMemCache(mFaviconUrl, loadedBitmaps.getBitmaps()); - Bitmap result = Favicons.getSizedFaviconFromCache(mFaviconUrl, mTargetWidth); + Favicons.putFaviconsInMemCache(faviconURL, loadedBitmaps.getBitmaps()); + Bitmap result = Favicons.getSizedFaviconFromCache(faviconURL, targetWidth); return result; } @@ -441,7 +441,7 @@ public class LoadFaviconTask extends UiAsyncTask { @Override protected void onPostExecute(Bitmap image) { - if (mIsChaining) { + if (isChaining) { return; } @@ -450,10 +450,10 @@ public class LoadFaviconTask extends UiAsyncTask { synchronized (loadsInFlight) { // Prevent any other tasks from chaining on this one. - loadsInFlight.remove(mFaviconUrl); + loadsInFlight.remove(faviconURL); } - // Since any update to mChainees is done while holding the loadsInFlight lock, once we reach + // Since any update to chainees is done while holding the loadsInFlight lock, once we reach // this point no further updates to that list can possibly take place (As far as other tasks // are concerned, there is no longer a task to chain from. The above block will have waited // for any tasks that were adding themselves to the list before reaching this point.) @@ -463,8 +463,8 @@ public class LoadFaviconTask extends UiAsyncTask { // actually happens outside of the strange situations unit tests create. // Share the result with all chained tasks. - if (mChainees != null) { - for (LoadFaviconTask t : mChainees) { + if (chainees != null) { + for (LoadFaviconTask t : chainees) { // In the case that we just decoded multiple favicons, either we're passing the right // image now, or the call into the cache in processResult will fetch the right one. t.processResult(image); @@ -473,35 +473,35 @@ public class LoadFaviconTask extends UiAsyncTask { } private void processResult(Bitmap image) { - Favicons.removeLoadTask(mId); + Favicons.removeLoadTask(id); Bitmap scaled = image; // Notify listeners, scaling if required. - if (mTargetWidth != -1 && image != null && image.getWidth() != mTargetWidth) { - scaled = Favicons.getSizedFaviconFromCache(mFaviconUrl, mTargetWidth); + if (targetWidth != -1 && image != null && image.getWidth() != targetWidth) { + scaled = Favicons.getSizedFaviconFromCache(faviconURL, targetWidth); } - Favicons.dispatchResult(mPageUrl, mFaviconUrl, scaled, mListener); + Favicons.dispatchResult(pageUrl, faviconURL, scaled, listener); } @Override protected void onCancelled() { - Favicons.removeLoadTask(mId); + Favicons.removeLoadTask(id); synchronized(loadsInFlight) { // Only remove from the hashmap if the task there is the one that's being canceled. // Cancellation of a task that would have chained is not interesting to the hashmap. - final LoadFaviconTask primary = loadsInFlight.get(mFaviconUrl); + final LoadFaviconTask primary = loadsInFlight.get(faviconURL); if (primary == this) { - loadsInFlight.remove(mFaviconUrl); + loadsInFlight.remove(faviconURL); return; } if (primary == null) { // This shouldn't happen. return; } - if (primary.mChainees != null) { - primary.mChainees.remove(this); + if (primary.chainees != null) { + primary.chainees.remove(this); } } @@ -518,15 +518,15 @@ public class LoadFaviconTask extends UiAsyncTask { * @param aChainee LoadFaviconTask */ private void chainTasks(LoadFaviconTask aChainee) { - if (mChainees == null) { - mChainees = new LinkedList(); + if (chainees == null) { + chainees = new LinkedList(); } - mChainees.add(aChainee); + chainees.add(aChainee); } int getId() { - return mId; + return id; } static void closeHTTPClient() { @@ -534,8 +534,8 @@ public class LoadFaviconTask extends UiAsyncTask { // the connection pool, which typically involves closing a connection -- // which counts as network activity. if (ThreadUtils.isOnBackgroundThread()) { - if (sHttpClient != null) { - sHttpClient.close(); + if (httpClient != null) { + httpClient.close(); } return; } diff --git a/mobile/android/base/favicons/cache/FaviconCache.java b/mobile/android/base/favicons/cache/FaviconCache.java index faa91e76d14..0973960e9f1 100644 --- a/mobile/android/base/favicons/cache/FaviconCache.java +++ b/mobile/android/base/favicons/cache/FaviconCache.java @@ -20,7 +20,7 @@ import java.util.concurrent.atomic.AtomicInteger; * When a favicon at a particular URL is decoded, it will yield one or more bitmaps. * While in memory, these bitmaps are stored in a list, sorted in ascending order of size, in a * FaviconsForURL object. - * The collection of FaviconsForURL objects currently in the cache is stored in mBackingMap, keyed + * The collection of FaviconsForURL objects currently in the cache is stored in backingMap, keyed * by favicon URL. * * A second map exists for permanent cache entries -- ones that are never expired. These entries @@ -59,7 +59,7 @@ import java.util.concurrent.atomic.AtomicInteger; * as well as the bitmap, a pointer to the encapsulating FaviconsForURL object (Used by the LRU * culler), the size of the encapsulated image, a flag indicating if this is a primary favicon, and * a flag indicating if the entry is invalid. - * All FaviconCacheElement objects are tracked in the mOrdering LinkedList. This is used to record + * All FaviconCacheElement objects are tracked in the ordering LinkedList. This is used to record * LRU information about FaviconCacheElements. In particular, the most recently used FaviconCacheElement * will be at the start of the list, the least recently used at the end of the list. * @@ -98,7 +98,7 @@ public class FaviconCache { private static final int NUM_FAVICON_SIZES = 4; // Dimensions of the largest favicon to store in the cache. Everything is downscaled to this. - public final int mMaxCachedWidth; + public final int maxCachedWidth; // Retry failed favicons after 20 minutes. public static final long FAILURE_RETRY_MILLISECONDS = 1000 * 60 * 20; @@ -107,16 +107,16 @@ public class FaviconCache { // Since favicons may be container formats holding multiple icons, the underlying type holds a // sorted list of bitmap payloads in ascending order of size. The underlying type may be queried // for the least larger payload currently present. - private final ConcurrentHashMap mBackingMap = new ConcurrentHashMap(); + private final ConcurrentHashMap backingMap = new ConcurrentHashMap(); // And the same, but never evicted. - private final ConcurrentHashMap mPermanentBackingMap = new ConcurrentHashMap(); + private final ConcurrentHashMap permanentBackingMap = new ConcurrentHashMap(); // A linked list used to implement a queue, defining the LRU properties of the cache. Elements // contained within the various FaviconsForURL objects are held here, the least recently used // of which at the end of the list. When space needs to be reclaimed, the appropriate bitmap is // culled. - private final LinkedList mOrdering = new LinkedList(); + private final LinkedList ordering = new LinkedList(); // The above structures, if used correctly, enable this cache to exhibit LRU semantics across all // favicon payloads in the system, as well as enabling the dynamic selection from the cache of @@ -124,60 +124,48 @@ public class FaviconCache { // are provided by the underlying file format). // Current size, in bytes, of the bitmap data present in the LRU cache. - private final AtomicInteger mCurrentSize = new AtomicInteger(0); + private final AtomicInteger currentSize = new AtomicInteger(0); // The maximum quantity, in bytes, of bitmap data which may be stored in the cache. - private final int mMaxSizeBytes; + private final int maxSizeBytes; // Tracks the number of ongoing read operations. Enables the first one in to lock writers out and // the last one out to let them in. - private final AtomicInteger mOngoingReads = new AtomicInteger(0); + private final AtomicInteger ongoingReads = new AtomicInteger(0); // Used to ensure transaction fairness - each txn acquires and releases this as the first operation. // The effect is an orderly, inexpensive ordering enforced on txns to prevent writer starvation. - private final Semaphore mTurnSemaphore = new Semaphore(1); + private final Semaphore turnSemaphore = new Semaphore(1); // A deviation from the usual MRSW solution - this semaphore is used to guard modification to the // ordering map. This allows for read transactions to update the most-recently-used value without // needing to take out the write lock. - private final Semaphore mReorderingSemaphore = new Semaphore(1); + private final Semaphore reorderingSemaphore = new Semaphore(1); // The semaphore one must acquire in order to perform a write. - private final Semaphore mWriteLock = new Semaphore(1); + private final Semaphore writeLock = new Semaphore(1); /** * Called by txns performing only reads as they start. Prevents writer starvation with a turn * semaphore and locks writers out if this is the first concurrent reader txn starting up. */ private void startRead() { - mTurnSemaphore.acquireUninterruptibly(); - mTurnSemaphore.release(); + turnSemaphore.acquireUninterruptibly(); + turnSemaphore.release(); - if (mOngoingReads.incrementAndGet() == 1) { + if (ongoingReads.incrementAndGet() == 1) { // First one in. Wait for writers to finish and lock them out. - mWriteLock.acquireUninterruptibly(); + writeLock.acquireUninterruptibly(); } } - /** - * An alternative to startWrite to be used when in a read transaction and wanting to upgrade it - * to a write transaction. Such a transaction should be terminated with finishWrite. - */ - private void upgradeReadToWrite() { - mTurnSemaphore.acquireUninterruptibly(); - if (mOngoingReads.decrementAndGet() == 0) { - mWriteLock.release(); - } - mWriteLock.acquireUninterruptibly(); - } - /** * Called by transactions performing only reads as they finish. Ensures that if this is the last * concluding read transaction then then writers are subsequently allowed in. */ private void finishRead() { - if (mOngoingReads.decrementAndGet() == 0) { - mWriteLock.release(); + if (ongoingReads.decrementAndGet() == 0) { + writeLock.release(); } } @@ -186,21 +174,21 @@ public class FaviconCache { * Upon return, no other txns will be executing concurrently. */ private void startWrite() { - mTurnSemaphore.acquireUninterruptibly(); - mWriteLock.acquireUninterruptibly(); + turnSemaphore.acquireUninterruptibly(); + writeLock.acquireUninterruptibly(); } /** * Called by a concluding write transaction - unlocks the structure. */ private void finishWrite() { - mTurnSemaphore.release(); - mWriteLock.release(); + turnSemaphore.release(); + writeLock.release(); } public FaviconCache(int maxSize, int maxWidthToCache) { - mMaxSizeBytes = maxSize; - mMaxCachedWidth = maxWidthToCache; + maxSizeBytes = maxSize; + maxCachedWidth = maxWidthToCache; } /** @@ -217,57 +205,42 @@ public class FaviconCache { startRead(); - boolean isExpired = false; - boolean isAborting = false; - try { // If we don't have it in the cache, it certainly isn't a known failure. // Non-evictable favicons are never failed, so we don't need to - // check mPermanentBackingMap. - if (!mBackingMap.containsKey(faviconURL)) { + // check permanentBackingMap. + if (!backingMap.containsKey(faviconURL)) { return false; } - FaviconsForURL container = mBackingMap.get(faviconURL); + FaviconsForURL container = backingMap.get(faviconURL); // If the has failed flag is not set, it's certainly not a known failure. - if (!container.mHasFailed) { + if (!container.hasFailed) { return false; } - final long failureTimestamp = container.mDownloadTimestamp; + final long failureTimestamp = container.downloadTimestamp; // Calculate elapsed time since the failing download. final long failureDiff = System.currentTimeMillis() - failureTimestamp; - // If long enough has passed, mark it as no longer a failure. - if (failureDiff > FAILURE_RETRY_MILLISECONDS) { - isExpired = true; - } else { + // If the expiry is still in effect, return. Otherwise, continue and unmark the failure. + if (failureDiff < FAILURE_RETRY_MILLISECONDS) { return true; } } catch (Exception unhandled) { - // Handle any exception thrown and return the locks to a sensible state. - finishRead(); - - // Flag to prevent finally from doubly-unlocking. - isAborting = true; Log.e(LOGTAG, "FaviconCache exception!", unhandled); return true; } finally { - if (!isAborting) { - if (isExpired) { - // No longer expired. - upgradeReadToWrite(); - } else { - finishRead(); - } - } + finishRead(); } + startWrite(); + + // If the entry is no longer failed, remove the record of it from the cache. try { - recordRemoved(mBackingMap.get(faviconURL)); - mBackingMap.remove(faviconURL); + recordRemoved(backingMap.remove(faviconURL)); return false; } finally { finishWrite(); @@ -282,14 +255,12 @@ public class FaviconCache { public void putFailed(String faviconURL) { startWrite(); - if (mBackingMap.containsKey(faviconURL)) { - recordRemoved(mBackingMap.get(faviconURL)); + try { + FaviconsForURL container = new FaviconsForURL(0, true); + recordRemoved(backingMap.put(faviconURL, container)); + } finally { + finishWrite(); } - - FaviconsForURL container = new FaviconsForURL(0, true); - mBackingMap.put(faviconURL, container); - - finishWrite(); } /** @@ -309,9 +280,7 @@ public class FaviconCache { return null; } - boolean doingWrites = false; boolean shouldComputeColour = false; - boolean isAborting = false; boolean wasPermanent = false; FaviconsForURL container; final Bitmap newBitmap; @@ -319,9 +288,9 @@ public class FaviconCache { startRead(); try { - container = mPermanentBackingMap.get(faviconURL); + container = permanentBackingMap.get(faviconURL); if (container == null) { - container = mBackingMap.get(faviconURL); + container = backingMap.get(faviconURL); if (container == null) { // We don't have it! return null; @@ -338,22 +307,22 @@ public class FaviconCache { // cacheElementIndex now holds either the index of the next least largest bitmap from // targetSize, or -1 if targetSize > all bitmaps. if (cacheElementIndex != -1) { - // If cacheElementIndex is not the sentinel value, then it is a valid index into mFavicons. - cacheElement = container.mFavicons.get(cacheElementIndex); + // If cacheElementIndex is not the sentinel value, then it is a valid index into favicons. + cacheElement = container.favicons.get(cacheElementIndex); - if (cacheElement.mInvalidated) { + if (cacheElement.invalidated) { return null; } // If we found exactly what we wanted - we're done. - if (cacheElement.mImageSize == targetSize) { - setMostRecentlyUsed(cacheElement); - return cacheElement.mFaviconPayload; + if (cacheElement.imageSize == targetSize) { + setMostRecentlyUsedWithinRead(cacheElement); + return cacheElement.faviconPayload; } } else { // We requested an image larger than all primaries. Set the element to start the search // from to the element beyond the end of the array, so the search runs backwards. - cacheElementIndex = container.mFavicons.size(); + cacheElementIndex = container.favicons.size(); } // We did not find exactly what we wanted, but now have set cacheElementIndex to the index @@ -370,17 +339,12 @@ public class FaviconCache { if (targetSize == -1) { // We got the biggest primary, so that's what we'll return. - return cacheElement.mFaviconPayload; + return cacheElement.faviconPayload; } - // Having got this far, we'll be needing to write the new secondary to the cache, which - // involves us falling through to the next try block. This flag lets us do this (Other - // paths prior to this end in returns.) - doingWrites = true; - // Scaling logic... - Bitmap largestElementBitmap = cacheElement.mFaviconPayload; - int largestSize = cacheElement.mImageSize; + Bitmap largestElementBitmap = cacheElement.faviconPayload; + int largestSize = cacheElement.imageSize; if (largestSize >= targetSize) { // The largest we have is larger than the target - downsize to target. @@ -401,24 +365,16 @@ public class FaviconCache { } } } catch (Exception unhandled) { - isAborting = true; - // Handle any exception thrown and return the locks to a sensible state. - finishRead(); // Flag to prevent finally from doubly-unlocking. Log.e(LOGTAG, "FaviconCache exception!", unhandled); return null; } finally { - if (!isAborting) { - if (doingWrites) { - upgradeReadToWrite(); - } else { - finishRead(); - } - } + finishRead(); } + startWrite(); try { if (shouldComputeColour) { // And since we failed, we'll need the dominant colour. @@ -432,8 +388,9 @@ public class FaviconCache { FaviconCacheElement newElement = container.addSecondary(newBitmap, targetSize); if (!wasPermanent) { - setMostRecentlyUsed(newElement); - mCurrentSize.addAndGet(newElement.sizeOf()); + if (setMostRecentlyUsedWithinWrite(newElement)) { + currentSize.addAndGet(newElement.sizeOf()); + } } } finally { finishWrite(); @@ -452,19 +409,18 @@ public class FaviconCache { startRead(); try { - FaviconsForURL element = mPermanentBackingMap.get(key); + FaviconsForURL element = permanentBackingMap.get(key); if (element == null) { - element = mBackingMap.get(key); + element = backingMap.get(key); } if (element == null) { Log.w(LOGTAG, "Cannot compute dominant color of non-cached favicon. Cache fullness " + - mCurrentSize.get() + '/' + mMaxSizeBytes); + currentSize.get() + '/' + maxSizeBytes); finishRead(); return 0xFFFFFF; } - return element.ensureDominantColor(); } finally { finishRead(); @@ -472,8 +428,8 @@ public class FaviconCache { } /** - * Remove all payloads stored in the given container from the LRU cache. Must be called while - * holding the write lock. + * Remove all payloads stored in the given container from the LRU cache. + * Must be called while holding the write lock. * * @param wasRemoved The container to purge from the cache. */ @@ -485,40 +441,59 @@ public class FaviconCache { int sizeRemoved = 0; - for (FaviconCacheElement e : wasRemoved.mFavicons) { + for (FaviconCacheElement e : wasRemoved.favicons) { sizeRemoved += e.sizeOf(); - mOrdering.remove(e); + ordering.remove(e); } - mCurrentSize.addAndGet(-sizeRemoved); + currentSize.addAndGet(-sizeRemoved); } private Bitmap produceCacheableBitmap(Bitmap favicon) { // Never cache the default Favicon, or the null Favicon. - if (favicon == Favicons.sDefaultFavicon || favicon == null) { + if (favicon == Favicons.defaultFavicon || favicon == null) { return null; } // Some sites serve up insanely huge Favicons (Seen 512x512 ones...) // While we want to cache nice big icons, we apply a limit based on screen density for the // sake of space. - if (favicon.getWidth() > mMaxCachedWidth) { - return Bitmap.createScaledBitmap(favicon, mMaxCachedWidth, mMaxCachedWidth, true); + if (favicon.getWidth() > maxCachedWidth) { + return Bitmap.createScaledBitmap(favicon, maxCachedWidth, maxCachedWidth, true); } + return favicon; } /** - * Set an existing element as the most recently used element. May be called from either type of - * transaction. + * Set an existing element as the most recently used element. Intended for use from read transactions. While + * write transactions may safely use this method, it will perform slightly worse than its unsafe counterpart below. * * @param element The element that is to become the most recently used one. + * @return true if this element already existed in the list, false otherwise. (Useful for preventing multiple-insertion.) */ - private void setMostRecentlyUsed(FaviconCacheElement element) { - mReorderingSemaphore.acquireUninterruptibly(); - mOrdering.remove(element); - mOrdering.offer(element); - mReorderingSemaphore.release(); + private boolean setMostRecentlyUsedWithinRead(FaviconCacheElement element) { + reorderingSemaphore.acquireUninterruptibly(); + try { + boolean contained = ordering.remove(element); + ordering.offer(element); + return contained; + } finally { + reorderingSemaphore.release(); + } + } + + /** + * Functionally equivalent to setMostRecentlyUsedWithinRead, but operates without taking the reordering semaphore. + * Only safe for use when called from a write transaction, or there is a risk of concurrent modification. + * + * @param element The element that is to become the most recently used one. + * @return true if this element already existed in the list, false otherwise. (Useful for preventing multiple-insertion.) + */ + private boolean setMostRecentlyUsedWithinWrite(FaviconCacheElement element) { + boolean contained = ordering.remove(element); + ordering.offer(element); + return contained; } /** @@ -546,13 +521,13 @@ public class FaviconCache { startWrite(); try { // Set the new element as the most recently used one. - setMostRecentlyUsed(newElement); + setMostRecentlyUsedWithinWrite(newElement); - mCurrentSize.addAndGet(newElement.sizeOf()); + currentSize.addAndGet(newElement.sizeOf()); // Update the value in the LruCache... FaviconsForURL wasRemoved; - wasRemoved = mBackingMap.put(faviconURL, toInsert); + wasRemoved = backingMap.put(faviconURL, toInsert); recordRemoved(wasRemoved); } finally { @@ -584,42 +559,23 @@ public class FaviconCache { sizeGained += newElement.sizeOf(); } - startRead(); - - boolean abortingRead = false; - - // Not using setMostRecentlyUsed, because the elements are known to be new. This can be done - // without taking the write lock, via the magic of the reordering semaphore. - mReorderingSemaphore.acquireUninterruptibly(); - try { - if (!permanently) { - for (FaviconCacheElement newElement : toInsert.mFavicons) { - mOrdering.offer(newElement); - } - } - } catch (Exception e) { - abortingRead = true; - mReorderingSemaphore.release(); - finishRead(); - - Log.e(LOGTAG, "Favicon cache exception!", e); - return; - } finally { - if (!abortingRead) { - mReorderingSemaphore.release(); - upgradeReadToWrite(); - } - } - + startWrite(); try { if (permanently) { - mPermanentBackingMap.put(faviconURL, toInsert); - } else { - mCurrentSize.addAndGet(sizeGained); - - // Update the value in the LruCache... - recordRemoved(mBackingMap.put(faviconURL, toInsert)); + permanentBackingMap.put(faviconURL, toInsert); + return; } + + for (FaviconCacheElement newElement : toInsert.favicons) { + setMostRecentlyUsedWithinWrite(newElement); + } + + // In the event this insertion is being made to a key that already held a value, the subsequent recordRemoved + // call will subtract the size of the old value, preventing double-counting. + currentSize.addAndGet(sizeGained); + + // Update the value in the LruCache... + recordRemoved(backingMap.put(faviconURL, toInsert)); } finally { finishWrite(); } @@ -632,24 +588,24 @@ public class FaviconCache { * Otherwise, do nothing. */ private void cullIfRequired() { - Log.d(LOGTAG, "Favicon cache fullness: " + mCurrentSize.get() + '/' + mMaxSizeBytes); + Log.d(LOGTAG, "Favicon cache fullness: " + currentSize.get() + '/' + maxSizeBytes); - if (mCurrentSize.get() <= mMaxSizeBytes) { + if (currentSize.get() <= maxSizeBytes) { return; } startWrite(); try { - while (mCurrentSize.get() > mMaxSizeBytes) { + while (currentSize.get() > maxSizeBytes) { // Cull the least recently used element. FaviconCacheElement victim; - victim = mOrdering.poll(); + victim = ordering.poll(); - mCurrentSize.addAndGet(-victim.sizeOf()); + currentSize.addAndGet(-victim.sizeOf()); victim.onEvictedFromCache(); - Log.d(LOGTAG, "After cull: " + mCurrentSize.get() + '/' + mMaxSizeBytes); + Log.d(LOGTAG, "After cull: " + currentSize.get() + '/' + maxSizeBytes); } } finally { finishWrite(); @@ -664,9 +620,9 @@ public class FaviconCache { // Note that we neither clear, nor track the size of, the permanent map. try { - mCurrentSize.set(0); - mBackingMap.clear(); - mOrdering.clear(); + currentSize.set(0); + backingMap.clear(); + ordering.clear(); } finally { finishWrite(); diff --git a/mobile/android/base/favicons/cache/FaviconCacheElement.java b/mobile/android/base/favicons/cache/FaviconCacheElement.java index aaecc2df683..f0d313df15e 100644 --- a/mobile/android/base/favicons/cache/FaviconCacheElement.java +++ b/mobile/android/base/favicons/cache/FaviconCacheElement.java @@ -12,49 +12,49 @@ import android.graphics.Bitmap; */ public class FaviconCacheElement implements Comparable { // Was this Favicon computed via scaling another primary Favicon, or is this a primary Favicon? - final boolean mIsPrimary; + final boolean isPrimary; // The Favicon bitmap. - Bitmap mFaviconPayload; + Bitmap faviconPayload; - // If set, mFaviconPayload is absent. Since the underlying ICO may contain multiple primary + // If set, faviconPayload is absent. Since the underlying ICO may contain multiple primary // payloads, primary payloads are never truly deleted from the cache, but instead have their // payload deleted and this flag set on their FaviconCacheElement. That way, the cache always // has a record of the existence of a primary payload, even if it is no longer in the cache. // This means that when a request comes in that will be best served using a primary that is in // the database but no longer cached, we know that it exists and can go get it (Useful when ICO // support is added). - volatile boolean mInvalidated; + volatile boolean invalidated; - final int mImageSize; + final int imageSize; // Used for LRU pruning. - final FaviconsForURL mBackpointer; + final FaviconsForURL backpointer; - public FaviconCacheElement(Bitmap payload, boolean isPrimary, int imageSize, FaviconsForURL backpointer) { - mFaviconPayload = payload; - mIsPrimary = isPrimary; - mImageSize = imageSize; - mBackpointer = backpointer; + public FaviconCacheElement(Bitmap payload, boolean primary, int size, FaviconsForURL backpointer) { + this.faviconPayload = payload; + this.isPrimary = primary; + this.imageSize = size; + this.backpointer = backpointer; } - public FaviconCacheElement(Bitmap payload, boolean isPrimary, FaviconsForURL backpointer) { - mFaviconPayload = payload; - mIsPrimary = isPrimary; - mBackpointer = backpointer; + public FaviconCacheElement(Bitmap faviconPayload, boolean isPrimary, FaviconsForURL backpointer) { + this.faviconPayload = faviconPayload; + this.isPrimary = isPrimary; + this.backpointer = backpointer; - if (payload != null) { - mImageSize = payload.getWidth(); + if (faviconPayload != null) { + imageSize = faviconPayload.getWidth(); } else { - mImageSize = 0; + imageSize = 0; } } public int sizeOf() { - if (mInvalidated) { + if (invalidated) { return 0; } - return mFaviconPayload.getRowBytes() * mFaviconPayload.getHeight(); + return faviconPayload.getRowBytes() * faviconPayload.getHeight(); } /** @@ -68,20 +68,20 @@ public class FaviconCacheElement implements Comparable { */ @Override public int compareTo(FaviconCacheElement another) { - if (mInvalidated && !another.mInvalidated) { + if (invalidated && !another.invalidated) { return -1; } - if (!mInvalidated && another.mInvalidated) { + if (!invalidated && another.invalidated) { return 1; } - if (mInvalidated) { + if (invalidated) { return 0; } - final int w1 = mImageSize; - final int w2 = another.mImageSize; + final int w1 = imageSize; + final int w2 = another.imageSize; if (w1 > w2) { return 1; } else if (w2 > w1) { @@ -96,20 +96,20 @@ public class FaviconCacheElement implements Comparable { * If primary, drop the payload and set invalid. If secondary, just unlink from parent node. */ public void onEvictedFromCache() { - if (mIsPrimary) { + if (isPrimary) { // So we keep a record of which primaries exist in the database for this URL, we // don't actually delete the entry for primaries. Instead, we delete their payload // and flag them as invalid. This way, we can later figure out that what a request // really want is one of the primaries that have been dropped from the cache, and we // can go get it. - mInvalidated = true; - mFaviconPayload = null; + invalidated = true; + faviconPayload = null; } else { // Secondaries don't matter - just delete them. - if (mBackpointer == null) { + if (backpointer == null) { return; } - mBackpointer.mFavicons.remove(this); + backpointer.favicons.remove(this); } } } diff --git a/mobile/android/base/favicons/cache/FaviconsForURL.java b/mobile/android/base/favicons/cache/FaviconsForURL.java index 2e6400ed1cb..4351b44cfa0 100644 --- a/mobile/android/base/favicons/cache/FaviconsForURL.java +++ b/mobile/android/base/favicons/cache/FaviconsForURL.java @@ -14,21 +14,21 @@ import java.util.Collections; public class FaviconsForURL { private static final String LOGTAG = "FaviconForURL"; - private volatile int mDominantColor = -1; + private volatile int dominantColor = -1; - final long mDownloadTimestamp; - final ArrayList mFavicons; + final long downloadTimestamp; + final ArrayList favicons; - public final boolean mHasFailed; + public final boolean hasFailed; public FaviconsForURL(int size) { this(size, false); } - public FaviconsForURL(int size, boolean hasFailed) { - mHasFailed = hasFailed; - mDownloadTimestamp = System.currentTimeMillis(); - mFavicons = new ArrayList(size); + public FaviconsForURL(int size, boolean failed) { + hasFailed = failed; + downloadTimestamp = System.currentTimeMillis(); + favicons = new ArrayList(size); } public FaviconCacheElement addSecondary(Bitmap favicon, int imageSize) { @@ -42,15 +42,19 @@ public class FaviconsForURL { private FaviconCacheElement addInternal(Bitmap favicon, boolean isPrimary, int imageSize) { FaviconCacheElement c = new FaviconCacheElement(favicon, isPrimary, imageSize, this); - int index = Collections.binarySearch(mFavicons, c); + int index = Collections.binarySearch(favicons, c); + + // We've already got an equivalent one. We don't care about this new one. This only occurs in certain obscure + // case conditions. + if (index >= 0) { + return favicons.get(index); + } // binarySearch returns -x - 1 where x is the insertion point of the element. Convert // this to the actual insertion point.. - if (index < 0) { - index++; - index = -index; - } - mFavicons.add(index, c); + index++; + index = -index; + favicons.add(index, c); return c; } @@ -66,7 +70,7 @@ public class FaviconsForURL { // Create a dummy object to hold the target value for comparable. FaviconCacheElement dummy = new FaviconCacheElement(null, false, targetSize, null); - int index = Collections.binarySearch(mFavicons, dummy); + int index = Collections.binarySearch(favicons, dummy); // The search routine returns the index of an element equal to dummy, if present. // Otherwise, it returns -x - 1, where x is the index in the ArrayList where dummy would be @@ -78,11 +82,11 @@ public class FaviconsForURL { // index is now 'x', as described above. - // The routine will return mFavicons.size() as the index iff dummy is larger than all elements + // The routine will return favicons.size() as the index iff dummy is larger than all elements // present (So the "index at which it should be inserted" is the index after the end. // In this case, we set the sentinel value -1 to indicate that we just requested something // larger than all primaries. - if (index == mFavicons.size()) { + if (index == favicons.size()) { index = -1; } @@ -97,19 +101,19 @@ public class FaviconsForURL { * primary at all (The input index may be a secondary which is larger than the actual available * primary.) * - * @param fromIndex The index into mFavicons from which to start the search. + * @param fromIndex The index into favicons from which to start the search. * @return The FaviconCacheElement of the next valid primary from the given index. If none exists, * then returns the previous valid primary. If none exists, returns null (Insanity.). */ public FaviconCacheElement getNextPrimary(final int fromIndex) { - final int numIcons = mFavicons.size(); + final int numIcons = favicons.size(); int searchIndex = fromIndex; while (searchIndex < numIcons) { - FaviconCacheElement element = mFavicons.get(searchIndex); + FaviconCacheElement element = favicons.get(searchIndex); - if (element.mIsPrimary) { - if (element.mInvalidated) { + if (element.isPrimary) { + if (element.invalidated) { // We return null here, despite the possible existence of other primaries, // because we know the most suitable primary for this request exists, but is // no longer in the cache. By returning null, we cause the caller to load the @@ -124,10 +128,10 @@ public class FaviconsForURL { // No larger primary available. Let's look for smaller ones... searchIndex = fromIndex - 1; while (searchIndex >= 0) { - FaviconCacheElement element = mFavicons.get(searchIndex); + FaviconCacheElement element = favicons.get(searchIndex); - if (element.mIsPrimary) { - if (element.mInvalidated) { + if (element.isPrimary) { + if (element.invalidated) { return null; } return element; @@ -144,17 +148,17 @@ public class FaviconsForURL { * Ensure the dominant colour field is populated for this favicon. */ public int ensureDominantColor() { - if (mDominantColor == -1) { + if (dominantColor == -1) { // Find a payload, any payload, that is not invalidated. - for (FaviconCacheElement element : mFavicons) { - if (!element.mInvalidated) { - mDominantColor = BitmapUtils.getDominantColor(element.mFaviconPayload); - return mDominantColor; + for (FaviconCacheElement element : favicons) { + if (!element.invalidated) { + dominantColor = BitmapUtils.getDominantColor(element.faviconPayload); + return dominantColor; } } - mDominantColor = 0xFFFFFF; + dominantColor = 0xFFFFFF; } - return mDominantColor; + return dominantColor; } } diff --git a/mobile/android/base/favicons/decoders/FaviconDecoder.java b/mobile/android/base/favicons/decoders/FaviconDecoder.java index caaff343b75..38b38693457 100644 --- a/mobile/android/base/favicons/decoders/FaviconDecoder.java +++ b/mobile/android/base/favicons/decoders/FaviconDecoder.java @@ -89,14 +89,14 @@ public class FaviconDecoder { LoadFaviconResult result; if (isDecodableByAndroid(buffer, offset)) { result = new LoadFaviconResult(); - result.mOffset = offset; - result.mLength = length; - result.mIsICO = false; + result.offset = offset; + result.length = length; + result.isICO = false; // We assume here that decodeByteArray doesn't hold on to the entire supplied // buffer -- worst case, each of our buffers will be twice the necessary size. - result.mBitmapsDecoded = new SingleBitmapIterator(BitmapUtils.decodeByteArray(buffer, offset, length)); - result.mFaviconBytes = buffer; + result.bitmapsDecoded = new SingleBitmapIterator(BitmapUtils.decodeByteArray(buffer, offset, length)); + result.faviconBytes = buffer; return result; } @@ -193,10 +193,10 @@ public class FaviconDecoder { * Iterator to hold a single bitmap. */ static class SingleBitmapIterator implements Iterator { - private Bitmap mBitmap; + private Bitmap bitmap; public SingleBitmapIterator(Bitmap b) { - mBitmap = b; + bitmap = b; } /** @@ -207,22 +207,22 @@ public class FaviconDecoder { * @return The bitmap carried by this SingleBitmapIterator. */ public Bitmap peek() { - return mBitmap; + return bitmap; } @Override public boolean hasNext() { - return mBitmap != null; + return bitmap != null; } @Override public Bitmap next() { - if (mBitmap == null) { + if (bitmap == null) { throw new NoSuchElementException("Element already returned from SingleBitmapIterator."); } - Bitmap ret = mBitmap; - mBitmap = null; + Bitmap ret = bitmap; + bitmap = null; return ret; } diff --git a/mobile/android/base/favicons/decoders/ICODecoder.java b/mobile/android/base/favicons/decoders/ICODecoder.java index 47758b0914b..74fdbda1f6b 100644 --- a/mobile/android/base/favicons/decoders/ICODecoder.java +++ b/mobile/android/base/favicons/decoders/ICODecoder.java @@ -10,7 +10,6 @@ import org.mozilla.gecko.gfx.BitmapUtils; import android.util.SparseArray; -import java.util.Collection; import java.util.Iterator; import java.util.NoSuchElementException; @@ -79,55 +78,55 @@ public class ICODecoder implements Iterable { public static final int ICO_ICONDIRENTRY_LENGTH_BYTES = 16; // The buffer containing bytes to attempt to decode. - private byte[] mDecodand; + private byte[] decodand; // The region of the decodand to decode. - private int mOffset; - private int mLen; + private int offset; + private int len; - private IconDirectoryEntry[] mIconDirectory; - private boolean mIsValid; - private boolean mHasDecoded; + private IconDirectoryEntry[] iconDirectory; + private boolean isValid; + private boolean hasDecoded; - public ICODecoder(byte[] buffer, int offset, int len) { - mDecodand = buffer; - mOffset = offset; - mLen = len; + public ICODecoder(byte[] decodand, int offset, int len) { + this.decodand = decodand; + this.offset = offset; + this.len = len; } /** - * Decode the Icon Directory for this ICO and store the result in mIconDirectory. + * Decode the Icon Directory for this ICO and store the result in iconDirectory. * * @return true if ICO decoding was considered to probably be a success, false if it certainly * was a failure. */ private boolean decodeIconDirectoryAndPossiblyPrune() { - mHasDecoded = true; + hasDecoded = true; // Fail if the end of the described range is out of bounds. - if (mOffset + mLen > mDecodand.length) { + if (offset + len > decodand.length) { return false; } // Fail if we don't have enough space for the header. - if (mLen < ICO_HEADER_LENGTH_BYTES) { + if (len < ICO_HEADER_LENGTH_BYTES) { return false; } // Check that the reserved fields in the header are indeed zero, and that the type field // specifies ICO. If not, we've probably been given something that isn't really an ICO. - if (mDecodand[mOffset] != 0 || - mDecodand[mOffset + 1] != 0 || - mDecodand[mOffset + 2] != 1 || - mDecodand[mOffset + 3] != 0) { + if (decodand[offset] != 0 || + decodand[offset + 1] != 0 || + decodand[offset + 2] != 1 || + decodand[offset + 3] != 0) { return false; } // Here, and in many other places, byte values are ANDed with 0xFF. This is because Java // bytes are signed - to obtain a numerical value of a longer type which holds the unsigned // interpretation of the byte of interest, we do this. - int numEncodedImages = (mDecodand[mOffset + 4] & 0xFF) | - (mDecodand[mOffset + 5] & 0xFF) << 8; + int numEncodedImages = (decodand[offset + 4] & 0xFF) | + (decodand[offset + 5] & 0xFF) << 8; // Fail if there are no images or the field is corrupt. @@ -139,12 +138,12 @@ public class ICODecoder implements Iterable { // Fail if there is not enough space in the buffer for the stated number of icondir entries, // let alone the data. - if (mLen < headerAndDirectorySize) { + if (len < headerAndDirectorySize) { return false; } // Put the pointer on the first byte of the first Icon Directory Entry. - int bufferIndex = mOffset + ICO_HEADER_LENGTH_BYTES; + int bufferIndex = offset + ICO_HEADER_LENGTH_BYTES; // We now iterate over the Icon Directory, decoding each entry as we go. We also need to // discard all entries except one >= the maximum interesting size. @@ -157,35 +156,35 @@ public class ICODecoder implements Iterable { for (int i = 0; i < numEncodedImages; i++, bufferIndex += ICO_ICONDIRENTRY_LENGTH_BYTES) { // Decode the Icon Directory Entry at this offset. - IconDirectoryEntry newEntry = IconDirectoryEntry.createFromBuffer(mDecodand, mOffset, mLen, bufferIndex); - newEntry.mIndex = i; + IconDirectoryEntry newEntry = IconDirectoryEntry.createFromBuffer(decodand, offset, len, bufferIndex); + newEntry.index = i; - if (newEntry.mIsErroneous) { + if (newEntry.isErroneous) { continue; } - if (newEntry.mWidth > Favicons.sLargestFaviconSize) { + if (newEntry.width > Favicons.largestFaviconSize) { // If we already have a smaller image larger than the maximum size of interest, we // don't care about the new one which is larger than the smallest image larger than // the maximum size. - if (newEntry.mWidth >= minimumMaximum) { + if (newEntry.width >= minimumMaximum) { continue; } // Remove the previous minimum-maximum. preferenceArray.delete(minimumMaximum); - minimumMaximum = newEntry.mWidth; + minimumMaximum = newEntry.width; } - IconDirectoryEntry oldEntry = preferenceArray.get(newEntry.mWidth); + IconDirectoryEntry oldEntry = preferenceArray.get(newEntry.width); if (oldEntry == null) { - preferenceArray.put(newEntry.mWidth, newEntry); + preferenceArray.put(newEntry.width, newEntry); continue; } if (oldEntry.compareTo(newEntry) < 0) { - preferenceArray.put(newEntry.mWidth, newEntry); + preferenceArray.put(newEntry.width, newEntry); } } @@ -197,24 +196,24 @@ public class ICODecoder implements Iterable { } // Allocate space for the icon directory entries in the decoded directory. - mIconDirectory = new IconDirectoryEntry[count]; + iconDirectory = new IconDirectoryEntry[count]; // The size of the data in the buffer that we find useful. int retainedSpace = ICO_HEADER_LENGTH_BYTES; for (int i = 0; i < count; i++) { IconDirectoryEntry e = preferenceArray.valueAt(i); - retainedSpace += ICO_ICONDIRENTRY_LENGTH_BYTES + e.mPayloadSize; - mIconDirectory[i] = e; + retainedSpace += ICO_ICONDIRENTRY_LENGTH_BYTES + e.payloadSize; + iconDirectory[i] = e; } - mIsValid = true; + isValid = true; // Set the number of images field in the buffer to reflect the number of retained entries. - mDecodand[mOffset + 4] = (byte) mIconDirectory.length; - mDecodand[mOffset + 5] = (byte) (mIconDirectory.length >>> 8); + decodand[offset + 4] = (byte) iconDirectory.length; + decodand[offset + 5] = (byte) (iconDirectory.length >>> 8); - if ((mLen - retainedSpace) > COMPACT_THRESHOLD) { + if ((len - retainedSpace) > COMPACT_THRESHOLD) { compactingCopy(retainedSpace); } @@ -228,19 +227,19 @@ public class ICODecoder implements Iterable { byte[] buf = new byte[spaceRetained]; // Copy the header. - System.arraycopy(mDecodand, mOffset, buf, 0, ICO_HEADER_LENGTH_BYTES); + System.arraycopy(decodand, offset, buf, 0, ICO_HEADER_LENGTH_BYTES); int headerPtr = ICO_HEADER_LENGTH_BYTES; - int payloadPtr = ICO_HEADER_LENGTH_BYTES + (mIconDirectory.length * ICO_ICONDIRENTRY_LENGTH_BYTES); + int payloadPtr = ICO_HEADER_LENGTH_BYTES + (iconDirectory.length * ICO_ICONDIRENTRY_LENGTH_BYTES); int ind = 0; - for (IconDirectoryEntry entry : mIconDirectory) { + for (IconDirectoryEntry entry : iconDirectory) { // Copy this entry. - System.arraycopy(mDecodand, mOffset + entry.getOffset(), buf, headerPtr, ICO_ICONDIRENTRY_LENGTH_BYTES); + System.arraycopy(decodand, offset + entry.getOffset(), buf, headerPtr, ICO_ICONDIRENTRY_LENGTH_BYTES); // Copy its payload. - System.arraycopy(mDecodand, mOffset + entry.mPayloadOffset, buf, payloadPtr, entry.mPayloadSize); + System.arraycopy(decodand, offset + entry.payloadOffset, buf, payloadPtr, entry.payloadSize); // Update the offset field. buf[headerPtr + 12] = (byte) payloadPtr; @@ -248,17 +247,17 @@ public class ICODecoder implements Iterable { buf[headerPtr + 14] = (byte) (payloadPtr >>> 16); buf[headerPtr + 15] = (byte) (payloadPtr >>> 24); - entry.mPayloadOffset = payloadPtr; - entry.mIndex = ind; + entry.payloadOffset = payloadPtr; + entry.index = ind; - payloadPtr += entry.mPayloadSize; + payloadPtr += entry.payloadSize; headerPtr += ICO_ICONDIRENTRY_LENGTH_BYTES; ind++; } - mDecodand = buf; - mOffset = 0; - mLen = spaceRetained; + decodand = buf; + offset = 0; + len = spaceRetained; } /** @@ -269,16 +268,16 @@ public class ICODecoder implements Iterable { * fails. */ public Bitmap decodeBitmapAtIndex(int index) { - final IconDirectoryEntry iconDirEntry = mIconDirectory[index]; + final IconDirectoryEntry iconDirEntry = iconDirectory[index]; - if (iconDirEntry.mPayloadIsPNG) { + if (iconDirEntry.payloadIsPNG) { // PNG payload. Simply extract it and decode it. - return BitmapUtils.decodeByteArray(mDecodand, mOffset + iconDirEntry.mPayloadOffset, iconDirEntry.mPayloadSize); + return BitmapUtils.decodeByteArray(decodand, offset + iconDirEntry.payloadOffset, iconDirEntry.payloadSize); } // The payload is a BMP, so we need to do some magic to get the decoder to do what we want. // We construct an ICO containing just the image we want, and let Android do the rest. - byte[] decodeTarget = new byte[ICO_HEADER_LENGTH_BYTES + ICO_ICONDIRENTRY_LENGTH_BYTES + iconDirEntry.mPayloadSize]; + byte[] decodeTarget = new byte[ICO_HEADER_LENGTH_BYTES + ICO_ICONDIRENTRY_LENGTH_BYTES + iconDirEntry.payloadSize]; // Set the type field in the ICO header. decodeTarget[2] = 1; @@ -287,11 +286,11 @@ public class ICODecoder implements Iterable { decodeTarget[4] = 1; // Copy the ICONDIRENTRY we need into the new buffer. - System.arraycopy(mDecodand, mOffset + iconDirEntry.getOffset(), decodeTarget, ICO_HEADER_LENGTH_BYTES, ICO_ICONDIRENTRY_LENGTH_BYTES); + System.arraycopy(decodand, offset + iconDirEntry.getOffset(), decodeTarget, ICO_HEADER_LENGTH_BYTES, ICO_ICONDIRENTRY_LENGTH_BYTES); // Copy the payload into the new buffer. final int singlePayloadOffset = ICO_HEADER_LENGTH_BYTES + ICO_ICONDIRENTRY_LENGTH_BYTES; - System.arraycopy(mDecodand, mOffset + iconDirEntry.mPayloadOffset, decodeTarget, singlePayloadOffset, iconDirEntry.mPayloadSize); + System.arraycopy(decodand, offset + iconDirEntry.payloadOffset, decodeTarget, singlePayloadOffset, iconDirEntry.payloadSize); // Update the offset field of the ICONDIRENTRY to make the new ICO valid. decodeTarget[ICO_HEADER_LENGTH_BYTES + 12] = (byte) singlePayloadOffset; @@ -311,12 +310,12 @@ public class ICODecoder implements Iterable { @Override public ICOIterator iterator() { // If a previous call to decode concluded this ICO is invalid, abort. - if (mHasDecoded && !mIsValid) { + if (hasDecoded && !isValid) { return null; } // If we've not been decoded before, but now fail to make any sense of the ICO, abort. - if (!mHasDecoded) { + if (!hasDecoded) { if (!decodeIconDirectoryAndPossiblyPrune()) { return null; } @@ -339,11 +338,11 @@ public class ICODecoder implements Iterable { LoadFaviconResult result = new LoadFaviconResult(); - result.mBitmapsDecoded = bitmaps; - result.mFaviconBytes = mDecodand; - result.mOffset = mOffset; - result.mLength = mLen; - result.mIsICO = true; + result.bitmapsDecoded = bitmaps; + result.faviconBytes = decodand; + result.offset = offset; + result.length = len; + result.isICO = true; return result; } @@ -356,12 +355,12 @@ public class ICODecoder implements Iterable { @Override public boolean hasNext() { - return mIndex < mIconDirectory.length; + return mIndex < iconDirectory.length; } @Override public Bitmap next() { - if (mIndex > mIconDirectory.length) { + if (mIndex > iconDirectory.length) { throw new NoSuchElementException("No more elements in this ICO."); } return decodeBitmapAtIndex(mIndex++); @@ -369,10 +368,10 @@ public class ICODecoder implements Iterable { @Override public void remove() { - if (mIconDirectory[mIndex] == null) { + if (iconDirectory[mIndex] == null) { throw new IllegalStateException("Remove already called for element " + mIndex); } - mIconDirectory[mIndex] = null; + iconDirectory[mIndex] = null; } } } diff --git a/mobile/android/base/favicons/decoders/IconDirectoryEntry.java b/mobile/android/base/favicons/decoders/IconDirectoryEntry.java index d73c356c458..a322c7d60e0 100644 --- a/mobile/android/base/favicons/decoders/IconDirectoryEntry.java +++ b/mobile/android/base/favicons/decoders/IconDirectoryEntry.java @@ -9,28 +9,28 @@ package org.mozilla.gecko.favicons.decoders; */ public class IconDirectoryEntry implements Comparable { - public static int sMaxBPP; + public static int maxBPP; - int mWidth; - int mHeight; - int mPaletteSize; - int mBitsPerPixel; - int mPayloadSize; - int mPayloadOffset; - boolean mPayloadIsPNG; + int width; + int height; + int paletteSize; + int bitsPerPixel; + int payloadSize; + int payloadOffset; + boolean payloadIsPNG; // Tracks the index in the Icon Directory of this entry. Useful only for pruning. - int mIndex; - boolean mIsErroneous; + int index; + boolean isErroneous; public IconDirectoryEntry(int width, int height, int paletteSize, int bitsPerPixel, int payloadSize, int payloadOffset, boolean payloadIsPNG) { - mWidth = width; - mHeight = height; - mPaletteSize = paletteSize; - mBitsPerPixel = bitsPerPixel; - mPayloadSize = payloadSize; - mPayloadOffset = payloadOffset; - mPayloadIsPNG = payloadIsPNG; + this.width = width; + this.height = height; + this.paletteSize = paletteSize; + this.bitsPerPixel = bitsPerPixel; + this.payloadSize = payloadSize; + this.payloadOffset = payloadOffset; + this.payloadIsPNG = payloadIsPNG; } /** @@ -40,7 +40,7 @@ public class IconDirectoryEntry implements Comparable { */ public static IconDirectoryEntry getErroneousEntry() { IconDirectoryEntry ret = new IconDirectoryEntry(-1, -1, -1, -1, -1, -1, false); - ret.mIsErroneous = true; + ret.isErroneous = true; return ret; } @@ -118,63 +118,63 @@ public class IconDirectoryEntry implements Comparable { * Get the number of bytes from the start of the ICO file to the beginning of this entry. */ public int getOffset() { - return ICODecoder.ICO_HEADER_LENGTH_BYTES + (mIndex * ICODecoder.ICO_ICONDIRENTRY_LENGTH_BYTES); + return ICODecoder.ICO_HEADER_LENGTH_BYTES + (index * ICODecoder.ICO_ICONDIRENTRY_LENGTH_BYTES); } @Override public int compareTo(IconDirectoryEntry another) { - if (mWidth > another.mWidth) { + if (width > another.width) { return 1; } - if (mWidth < another.mWidth) { + if (width < another.width) { return -1; } // Where both images exceed the max BPP, take the smaller of the two BPP values. - if (mBitsPerPixel >= sMaxBPP && another.mBitsPerPixel >= sMaxBPP) { - if (mBitsPerPixel < another.mBitsPerPixel) { + if (bitsPerPixel >= maxBPP && another.bitsPerPixel >= maxBPP) { + if (bitsPerPixel < another.bitsPerPixel) { return 1; } - if (mBitsPerPixel > another.mBitsPerPixel) { + if (bitsPerPixel > another.bitsPerPixel) { return -1; } } // Otherwise, take the larger of the BPP values. - if (mBitsPerPixel > another.mBitsPerPixel) { + if (bitsPerPixel > another.bitsPerPixel) { return 1; } - if (mBitsPerPixel < another.mBitsPerPixel) { + if (bitsPerPixel < another.bitsPerPixel) { return -1; } // Prefer large palettes. - if (mPaletteSize > another.mPaletteSize) { + if (paletteSize > another.paletteSize) { return 1; } - if (mPaletteSize < another.mPaletteSize) { + if (paletteSize < another.paletteSize) { return -1; } // Prefer smaller payloads. - if (mPayloadSize < another.mPayloadSize) { + if (payloadSize < another.payloadSize) { return 1; } - if (mPayloadSize > another.mPayloadSize) { + if (payloadSize > another.payloadSize) { return -1; } // If all else fails, prefer PNGs over BMPs. They tend to be smaller. - if (mPayloadIsPNG && !another.mPayloadIsPNG) { + if (payloadIsPNG && !another.payloadIsPNG) { return 1; } - if (!mPayloadIsPNG && another.mPayloadIsPNG) { + if (!payloadIsPNG && another.payloadIsPNG) { return -1; } @@ -182,20 +182,20 @@ public class IconDirectoryEntry implements Comparable { } public static void setMaxBPP(int maxBPP) { - sMaxBPP = maxBPP; + IconDirectoryEntry.maxBPP = maxBPP; } @Override public String toString() { return "IconDirectoryEntry{" + - "\nmWidth=" + mWidth + - ", \nmHeight=" + mHeight + - ", \nmPaletteSize=" + mPaletteSize + - ", \nmBitsPerPixel=" + mBitsPerPixel + - ", \nmPayloadSize=" + mPayloadSize + - ", \nmPayloadOffset=" + mPayloadOffset + - ", \nmPayloadIsPNG=" + mPayloadIsPNG + - ", \nmIndex=" + mIndex + + "\nwidth=" + width + + ", \nheight=" + height + + ", \npaletteSize=" + paletteSize + + ", \nbitsPerPixel=" + bitsPerPixel + + ", \npayloadSize=" + payloadSize + + ", \npayloadOffset=" + payloadOffset + + ", \npayloadIsPNG=" + payloadIsPNG + + ", \nindex=" + index + '}'; } } diff --git a/mobile/android/base/favicons/decoders/LoadFaviconResult.java b/mobile/android/base/favicons/decoders/LoadFaviconResult.java index 61f4601f4c8..cba88483ae2 100644 --- a/mobile/android/base/favicons/decoders/LoadFaviconResult.java +++ b/mobile/android/base/favicons/decoders/LoadFaviconResult.java @@ -21,15 +21,15 @@ import java.util.Iterator; public class LoadFaviconResult { private static final String LOGTAG = "LoadFaviconResult"; - byte[] mFaviconBytes; - int mOffset; - int mLength; + byte[] faviconBytes; + int offset; + int length; - boolean mIsICO; - Iterator mBitmapsDecoded; + boolean isICO; + Iterator bitmapsDecoded; public Iterator getBitmaps() { - return mBitmapsDecoded; + return bitmapsDecoded; } /** @@ -40,17 +40,17 @@ public class LoadFaviconResult { */ public byte[] getBytesForDatabaseStorage() { // Begin by normalising the buffer. - if (mOffset != 0 || mLength != mFaviconBytes.length) { - final byte[] normalised = new byte[mLength]; - System.arraycopy(mFaviconBytes, mOffset, normalised, 0, mLength); - mOffset = 0; - mFaviconBytes = normalised; + if (offset != 0 || length != faviconBytes.length) { + final byte[] normalised = new byte[length]; + System.arraycopy(faviconBytes, offset, normalised, 0, length); + offset = 0; + faviconBytes = normalised; } // For results containing a single image, we re-encode the result as a PNG in an effort to // save space. - if (!mIsICO) { - Bitmap favicon = ((FaviconDecoder.SingleBitmapIterator) mBitmapsDecoded).peek(); + if (!isICO) { + Bitmap favicon = ((FaviconDecoder.SingleBitmapIterator) bitmapsDecoded).peek(); byte[] data = null; ByteArrayOutputStream stream = new ByteArrayOutputStream(); @@ -68,7 +68,7 @@ public class LoadFaviconResult { // We may instead want to consider re-encoding the entire ICO as a collection of efficiently // encoded PNGs. This may not be worth the CPU time (Indeed, the encoding of single-image // favicons may also not be worth the time/space tradeoff.). - return mFaviconBytes; + return faviconBytes; } } diff --git a/mobile/android/base/home/BrowserSearch.java b/mobile/android/base/home/BrowserSearch.java index 2fb3022d492..eff35212205 100644 --- a/mobile/android/base/home/BrowserSearch.java +++ b/mobile/android/base/home/BrowserSearch.java @@ -568,7 +568,7 @@ public class BrowserSearch extends HomeFragment mSuggestionsOptInPrompt = ((ViewStub) mView.findViewById(R.id.suggestions_opt_in_prompt)).inflate(); TextView promptText = (TextView) mSuggestionsOptInPrompt.findViewById(R.id.suggestions_prompt_title); - promptText.setText(getResources().getString(R.string.suggestions_prompt, mSearchEngines.get(0).name)); + promptText.setText(getResources().getString(R.string.suggestions_prompt)); final View yesButton = mSuggestionsOptInPrompt.findViewById(R.id.suggestions_prompt_yes); final View noButton = mSuggestionsOptInPrompt.findViewById(R.id.suggestions_prompt_no); diff --git a/mobile/android/base/locales/en-US/android_strings.dtd b/mobile/android/base/locales/en-US/android_strings.dtd index 7ad4b582145..79da8a1839c 100644 --- a/mobile/android/base/locales/en-US/android_strings.dtd +++ b/mobile/android/base/locales/en-US/android_strings.dtd @@ -378,9 +378,7 @@ just addresses the organization to follow, e.g. "This site is run by " --> from Android"> - - + - &suggestions_prompt2; + &suggestions_prompt3; &suggestion_for_engine; diff --git a/mobile/android/base/tests/testHomeProvider.js b/mobile/android/base/tests/testHomeProvider.js index d9f6fbfa46d..10fe973dcfc 100644 --- a/mobile/android/base/tests/testHomeProvider.js +++ b/mobile/android/base/tests/testHomeProvider.js @@ -13,6 +13,7 @@ Cu.import("resource://gre/modules/Task.jsm"); const TEST_DATASET_ID = "test-dataset-id"; const TEST_URL = "http://test.com"; +const TEST_TITLE = "Test"; const PREF_SYNC_CHECK_INTERVAL_SECS = "home.sync.checkIntervalSecs"; const TEST_INTERVAL_SECS = 1; @@ -47,7 +48,7 @@ add_test(function test_periodic_sync() { add_task(function test_save_and_delete() { // Use the HomeProvider API to save some data. let storage = HomeProvider.getStorage(TEST_DATASET_ID); - yield storage.save([{ url: TEST_URL }]); + yield storage.save([{ title: TEST_TITLE, url: TEST_URL }]); // Peek in the DB to make sure we have the right data. let db = yield Sqlite.openConnection({ path: DB_PATH }); @@ -71,4 +72,61 @@ add_task(function test_save_and_delete() { db.close(); }); +add_task(function test_row_validation() { + // Use the HomeProvider API to save some data. + let storage = HomeProvider.getStorage(TEST_DATASET_ID); + + let invalidRows = [ + { url: "url" }, + { title: "title" }, + { description: "description" }, + { image_url: "image_url" } + ]; + + // None of these save calls should save anything + for (let row of invalidRows) { + try { + yield storage.save([row]); + } catch (e if e instanceof HomeProvider.ValidationError) { + // Just catch and ignore validation errors + } + } + + // Peek in the DB to make sure we have the right data. + let db = yield Sqlite.openConnection({ path: DB_PATH }); + + // Make sure no data has been saved. + let result = yield db.execute("SELECT * FROM items"); + do_check_eq(result.length, 0); + + db.close(); +}); + +add_task(function test_save_transaction() { + // Use the HomeProvider API to save some data. + let storage = HomeProvider.getStorage(TEST_DATASET_ID); + + // One valid, one invalid + let rows = [ + { title: TEST_TITLE, url: TEST_URL }, + { image_url: "image_url" } + ]; + + // Try to save all the rows at once + try { + yield storage.save(rows); + } catch (e if e instanceof HomeProvider.ValidationError) { + // Just catch and ignore validation errors + } + + // Peek in the DB to make sure we have the right data. + let db = yield Sqlite.openConnection({ path: DB_PATH }); + + // Make sure no data has been saved. + let result = yield db.execute("SELECT * FROM items"); + do_check_eq(result.length, 0); + + db.close(); +}); + run_next_test(); diff --git a/mobile/android/components/HelperAppDialog.js b/mobile/android/components/HelperAppDialog.js index a8c246560c2..c303c8d49d6 100644 --- a/mobile/android/components/HelperAppDialog.js +++ b/mobile/android/components/HelperAppDialog.js @@ -43,12 +43,10 @@ HelperAppLauncherDialog.prototype = { * Returns true otherwise. */ _canDownload: function (url, alreadyResolved=false) { - Services.console.logStringMessage("_canDownload: " + url); // The common case. if (url.schemeIs("http") || url.schemeIs("https") || url.schemeIs("ftp")) { - Services.console.logStringMessage("_canDownload: true\n"); return true; } @@ -57,7 +55,6 @@ HelperAppLauncherDialog.prototype = { url.schemeIs("jar") || url.schemeIs("resource") || url.schemeIs("wyciwyg")) { - Services.console.logStringMessage("_canDownload: false\n"); return false; } @@ -66,7 +63,6 @@ HelperAppLauncherDialog.prototype = { let ioSvc = Cc["@mozilla.org/network/io-service;1"].getService(Components.interfaces.nsIIOService); let innerURI = ioSvc.newChannelFromURI(url).URI; if (!url.equals(innerURI)) { - Services.console.logStringMessage("_canDownload: recursing.\n"); return this._canDownload(innerURI, true); } } @@ -81,17 +77,14 @@ HelperAppLauncherDialog.prototype = { let appRoot = FileUtils.getFile("XREExeF", []); if (appRoot.contains(file, true)) { - Services.console.logStringMessage("_canDownload: appRoot.\n"); return false; } let profileRoot = FileUtils.getFile("ProfD", []); if (profileRoot.contains(file, true)) { - Services.console.logStringMessage("_canDownload: prof dir.\n"); return false; } - Services.console.logStringMessage("_canDownload: safe.\n"); return true; } diff --git a/mobile/android/installer/package-manifest.in b/mobile/android/installer/package-manifest.in index 9ef60964223..84f3cdeabed 100644 --- a/mobile/android/installer/package-manifest.in +++ b/mobile/android/installer/package-manifest.in @@ -382,6 +382,7 @@ @BINPATH@/components/nsINIProcessor.js @BINPATH@/components/nsPrompter.manifest @BINPATH@/components/nsPrompter.js +@BINPATH@/components/servicesComponents.manifest @BINPATH@/components/TelemetryStartup.js @BINPATH@/components/TelemetryStartup.manifest @BINPATH@/components/Webapps.js diff --git a/mobile/android/modules/HomeProvider.jsm b/mobile/android/modules/HomeProvider.jsm index 7e83e70f592..f58c36ae364 100644 --- a/mobile/android/modules/HomeProvider.jsm +++ b/mobile/android/modules/HomeProvider.jsm @@ -112,7 +112,20 @@ function syncTimerCallback(timer) { } } +this.HomeStorage = function(datasetId) { + this.datasetId = datasetId; +}; + +this.ValidationError = function(message) { + this.name = "ValidationError"; + this.message = message; +}; +ValidationError.prototype = new Error(); +ValidationError.prototype.constructor = ValidationError; + this.HomeProvider = Object.freeze({ + ValidationError: ValidationError, + /** * Returns a storage associated with a given dataset identifer. * @@ -249,9 +262,23 @@ function getDatabaseConnection() { }); } -this.HomeStorage = function(datasetId) { - this.datasetId = datasetId; -}; +/** + * Validates an item to be saved to the DB. + * + * @param item + * (object) item object to be validated. + */ +function validateItem(datasetId, item) { + if (!item.url) { + throw new ValidationError('HomeStorage: All rows must have an URL: datasetId = ' + + datasetId); + } + + if (!item.image_url && !item.title && !item.description) { + throw new ValidationError('HomeStorage: All rows must have at least an image URL, ' + + 'or a title or a description: datasetId = ' + datasetId); + } +} HomeStorage.prototype = { /** @@ -267,20 +294,24 @@ HomeStorage.prototype = { return Task.spawn(function save_task() { let db = yield getDatabaseConnection(); try { - // Insert data into DB. - for (let item of data) { - // XXX: Directly pass item as params? More validation for item? Batch insert? - let params = { - dataset_id: this.datasetId, - url: item.url, - title: item.title, - description: item.description, - image_url: item.image_url, - filter: item.filter, - created: Date.now() - }; - yield db.executeCached(SQL.insertItem, params); - } + yield db.executeTransaction(function save_transaction() { + // Insert data into DB. + for (let item of data) { + validateItem(this.datasetId, item); + + // XXX: Directly pass item as params? More validation for item? + let params = { + dataset_id: this.datasetId, + url: item.url, + title: item.title, + description: item.description, + image_url: item.image_url, + filter: item.filter, + created: Date.now() + }; + yield db.executeCached(SQL.insertItem, params); + } + }.bind(this)); } finally { yield db.close(); } diff --git a/netwerk/base/src/Dashboard.cpp b/netwerk/base/src/Dashboard.cpp index f88ce28f414..1beba010cec 100644 --- a/netwerk/base/src/Dashboard.cpp +++ b/netwerk/base/src/Dashboard.cpp @@ -18,19 +18,316 @@ #include "nsURLHelper.h" using mozilla::AutoSafeJSContext; +using mozilla::dom::Sequence; + namespace mozilla { namespace net { -NS_IMPL_ISUPPORTS5(Dashboard, nsIDashboard, nsIDashboardEventNotifier, - nsITransportEventSink, nsITimerCallback, - nsIDNSListener) -using mozilla::dom::Sequence; - -struct ConnStatus +class SocketData + : public nsISupports { - nsString creationSts; +public: + NS_DECL_THREADSAFE_ISUPPORTS + + SocketData() + { + mTotalSent = 0; + mTotalRecv = 0; + mThread = nullptr; + } + + virtual ~SocketData() + { + } + + uint64_t mTotalSent; + uint64_t mTotalRecv; + nsTArray mData; + nsCOMPtr mCallback; + nsIThread *mThread; }; +NS_IMPL_ISUPPORTS0(SocketData) + + +class HttpData + : public nsISupports +{ +public: + NS_DECL_THREADSAFE_ISUPPORTS + + HttpData() + { + mThread = nullptr; + } + + virtual ~HttpData() + { + } + + nsTArray mData; + nsCOMPtr mCallback; + nsIThread *mThread; +}; + +NS_IMPL_ISUPPORTS0(HttpData) + + +class WebSocketRequest + : public nsISupports +{ +public: + NS_DECL_THREADSAFE_ISUPPORTS + + WebSocketRequest() + { + mThread = nullptr; + } + + virtual ~WebSocketRequest() + { + } + + nsCOMPtr mCallback; + nsIThread *mThread; +}; + +NS_IMPL_ISUPPORTS0(WebSocketRequest) + + +class DnsData + : public nsISupports +{ +public: + NS_DECL_THREADSAFE_ISUPPORTS + + DnsData() + { + mThread = nullptr; + } + + virtual ~DnsData() + { + } + + nsTArray mData; + nsCOMPtr mCallback; + nsIThread *mThread; +}; + +NS_IMPL_ISUPPORTS0(DnsData) + + +class ConnectionData + : public nsITransportEventSink + , public nsITimerCallback +{ +public: + NS_DECL_THREADSAFE_ISUPPORTS + NS_DECL_NSITRANSPORTEVENTSINK + NS_DECL_NSITIMERCALLBACK + + void StartTimer(uint32_t aTimeout); + void StopTimer(); + + ConnectionData(Dashboard *target) + { + mThread = nullptr; + mDashboard = target; + } + + virtual ~ConnectionData() + { + if (mTimer) { + mTimer->Cancel(); + } + } + + nsCOMPtr mSocket; + nsCOMPtr mStreamIn; + nsCOMPtr mTimer; + nsCOMPtr mCallback; + nsIThread *mThread; + Dashboard *mDashboard; + + nsCString mHost; + uint32_t mPort; + const char *mProtocol; + uint32_t mTimeout; + + nsString mStatus; +}; + +NS_IMPL_ISUPPORTS2(ConnectionData, nsITransportEventSink, nsITimerCallback) + +NS_IMETHODIMP +ConnectionData::OnTransportStatus(nsITransport *aTransport, nsresult aStatus, + uint64_t aProgress, uint64_t aProgressMax) +{ + if (aStatus == NS_NET_STATUS_CONNECTED_TO) { + StopTimer(); + } + + CopyASCIItoUTF16(Dashboard::GetErrorString(aStatus), mStatus); + nsCOMPtr event = + NS_NewRunnableMethodWithArg > + (mDashboard, &Dashboard::GetConnectionStatus, this); + mThread->Dispatch(event, NS_DISPATCH_NORMAL); + + return NS_OK; +} + +NS_IMETHODIMP +ConnectionData::Notify(nsITimer *aTimer) +{ + MOZ_ASSERT(aTimer == mTimer); + + if (mSocket) { + mSocket->Close(NS_ERROR_ABORT); + mSocket = nullptr; + mStreamIn = nullptr; + } + + mTimer = nullptr; + + mStatus.Assign(NS_LITERAL_STRING("NS_ERROR_NET_TIMEOUT")); + nsCOMPtr event = + NS_NewRunnableMethodWithArg > + (mDashboard, &Dashboard::GetConnectionStatus, this); + mThread->Dispatch(event, NS_DISPATCH_NORMAL); + + return NS_OK; +} + +void +ConnectionData::StartTimer(uint32_t aTimeout) +{ + if (!mTimer) { + mTimer = do_CreateInstance("@mozilla.org/timer;1"); + } + + mTimer->InitWithCallback(this, aTimeout * 1000, + nsITimer::TYPE_ONE_SHOT); +} + +void +ConnectionData::StopTimer() +{ + if (mTimer) { + mTimer->Cancel(); + mTimer = nullptr; + } +} + + +class LookupHelper; + +class LookupArgument + : public nsISupports +{ +public: + NS_DECL_THREADSAFE_ISUPPORTS + + LookupArgument(nsIDNSRecord *aRecord, LookupHelper *aHelper) + { + mRecord = aRecord; + mHelper = aHelper; + } + + virtual ~LookupArgument() + { + } + + nsCOMPtr mRecord; + nsRefPtr mHelper; +}; + +NS_IMPL_ISUPPORTS0(LookupArgument) + + +class LookupHelper + : public nsIDNSListener +{ +public: + NS_DECL_THREADSAFE_ISUPPORTS + NS_DECL_NSIDNSLISTENER + + LookupHelper() { + } + + virtual ~LookupHelper() + { + if (mCancel) { + mCancel->Cancel(NS_ERROR_ABORT); + } + } + + nsresult ConstructAnswer(LookupArgument *aArgument); +public: + nsCOMPtr mCancel; + nsCOMPtr mCallback; + nsIThread *mThread; + nsresult mStatus; +}; + +NS_IMPL_ISUPPORTS1(LookupHelper, nsIDNSListener) + +NS_IMETHODIMP +LookupHelper::OnLookupComplete(nsICancelable *aRequest, + nsIDNSRecord *aRecord, nsresult aStatus) +{ + MOZ_ASSERT(aRequest == mCancel); + mCancel = nullptr; + mStatus = aStatus; + + nsRefPtr arg = new LookupArgument(aRecord, this); + nsCOMPtr event = + NS_NewRunnableMethodWithArg >( + this, &LookupHelper::ConstructAnswer, arg); + mThread->Dispatch(event, NS_DISPATCH_NORMAL); + + return NS_OK; +} + +nsresult +LookupHelper::ConstructAnswer(LookupArgument *aArgument) +{ + + nsIDNSRecord *aRecord = aArgument->mRecord; + AutoSafeJSContext cx; + + mozilla::dom::DNSLookupDict dict; + dict.mAddress.Construct(); + + Sequence &addresses = dict.mAddress.Value(); + + if (NS_SUCCEEDED(mStatus)) { + dict.mAnswer = true; + bool hasMore; + aRecord->HasMore(&hasMore); + while (hasMore) { + nsCString nextAddress; + aRecord->GetNextAddrAsString(nextAddress); + CopyASCIItoUTF16(nextAddress, *addresses.AppendElement()); + aRecord->HasMore(&hasMore); + } + } else { + dict.mAnswer = false; + CopyASCIItoUTF16(Dashboard::GetErrorString(mStatus), dict.mError); + } + + JS::RootedValue val(cx); + if (!dict.ToObject(cx, JS::NullPtr(), &val)) { + return NS_ERROR_FAILURE; + } + + this->mCallback->OnDashboardDataAvailable(val); + + return NS_OK; +} + +NS_IMPL_ISUPPORTS2(Dashboard, nsIDashboard, nsIDashboardEventNotifier) + Dashboard::Dashboard() { mEnableLogging = false; @@ -38,39 +335,41 @@ Dashboard::Dashboard() Dashboard::~Dashboard() { - if (mDnsup.cancel) - mDnsup.cancel->Cancel(NS_ERROR_ABORT); } NS_IMETHODIMP -Dashboard::RequestSockets(NetDashboardCallback* cb) +Dashboard::RequestSockets(NetDashboardCallback *aCallback) { - if (mSock.cb) - return NS_ERROR_FAILURE; - mSock.cb = cb; - mSock.data.Clear(); - mSock.thread = NS_GetCurrentThread(); - - nsCOMPtr event = NS_NewRunnableMethod(this, &Dashboard::GetSocketsDispatch); + nsRefPtr socketData = new SocketData(); + socketData->mCallback = aCallback; + socketData->mThread = NS_GetCurrentThread(); + nsCOMPtr event = + NS_NewRunnableMethodWithArg > + (this, &Dashboard::GetSocketsDispatch, socketData); gSocketTransportService->Dispatch(event, NS_DISPATCH_NORMAL); return NS_OK; } -void -Dashboard::GetSocketsDispatch() +nsresult +Dashboard::GetSocketsDispatch(SocketData *aSocketData) { + nsRefPtr socketData = aSocketData; if (gSocketTransportService) { - gSocketTransportService->GetSocketConnections(&mSock.data); - mSock.totalSent = gSocketTransportService->GetSentBytes(); - mSock.totalRecv = gSocketTransportService->GetReceivedBytes(); + gSocketTransportService->GetSocketConnections(&socketData->mData); + socketData->mTotalSent = gSocketTransportService->GetSentBytes(); + socketData->mTotalRecv = gSocketTransportService->GetReceivedBytes(); } - nsCOMPtr event = NS_NewRunnableMethod(this, &Dashboard::GetSockets); - mSock.thread->Dispatch(event, NS_DISPATCH_NORMAL); + nsCOMPtr event = + NS_NewRunnableMethodWithArg > + (this, &Dashboard::GetSockets, socketData); + socketData->mThread->Dispatch(event, NS_DISPATCH_NORMAL); + return NS_OK; } nsresult -Dashboard::GetSockets() +Dashboard::GetSockets(SocketData *aSocketData) { + nsRefPtr socketData = aSocketData; AutoSafeJSContext cx; mozilla::dom::SocketsDict dict; @@ -80,66 +379,65 @@ Dashboard::GetSockets() Sequence &sockets = dict.mSockets.Value(); - uint32_t length = mSock.data.Length(); + uint32_t length = socketData->mData.Length(); if (!sockets.SetCapacity(length)) { - mSock.cb = nullptr; - mSock.data.Clear(); JS_ReportOutOfMemory(cx); return NS_ERROR_OUT_OF_MEMORY; } - for (uint32_t i = 0; i < mSock.data.Length(); i++) { - mozilla::dom::SocketElement &socket = *sockets.AppendElement(); - CopyASCIItoUTF16(mSock.data[i].host, socket.mHost); - socket.mPort = mSock.data[i].port; - socket.mActive = mSock.data[i].active; - socket.mTcp = mSock.data[i].tcp; - socket.mSent = (double) mSock.data[i].sent; - socket.mReceived = (double) mSock.data[i].received; - dict.mSent += mSock.data[i].sent; - dict.mReceived += mSock.data[i].received; + for (uint32_t i = 0; i < socketData->mData.Length(); i++) { + mozilla::dom::SocketElement &mSocket = *sockets.AppendElement(); + CopyASCIItoUTF16(socketData->mData[i].host, mSocket.mHost); + mSocket.mPort = socketData->mData[i].port; + mSocket.mActive = socketData->mData[i].active; + mSocket.mTcp = socketData->mData[i].tcp; + mSocket.mSent = (double) socketData->mData[i].sent; + mSocket.mReceived = (double) socketData->mData[i].received; + dict.mSent += socketData->mData[i].sent; + dict.mReceived += socketData->mData[i].received; } - dict.mSent += mSock.totalSent; - dict.mReceived += mSock.totalRecv; + dict.mSent += socketData->mTotalSent; + dict.mReceived += socketData->mTotalRecv; JS::RootedValue val(cx); - if (!dict.ToObject(cx, JS::NullPtr(), &val)) { - mSock.cb = nullptr; - mSock.data.Clear(); + if (!dict.ToObject(cx, JS::NullPtr(), &val)) return NS_ERROR_FAILURE; - } - mSock.cb->OnDashboardDataAvailable(val); - mSock.cb = nullptr; + socketData->mCallback->OnDashboardDataAvailable(val); return NS_OK; } NS_IMETHODIMP -Dashboard::RequestHttpConnections(NetDashboardCallback* cb) +Dashboard::RequestHttpConnections(NetDashboardCallback *aCallback) { - if (mHttp.cb) - return NS_ERROR_FAILURE; - mHttp.cb = cb; - mHttp.data.Clear(); - mHttp.thread = NS_GetCurrentThread(); + nsRefPtr httpData = new HttpData(); + httpData->mCallback = aCallback; + httpData->mThread = NS_GetCurrentThread(); - nsCOMPtr event = NS_NewRunnableMethod(this, &Dashboard::GetHttpDispatch); + nsCOMPtr event = + NS_NewRunnableMethodWithArg > + (this, &Dashboard::GetHttpDispatch, httpData); gSocketTransportService->Dispatch(event, NS_DISPATCH_NORMAL); return NS_OK; } -void -Dashboard::GetHttpDispatch() +nsresult +Dashboard::GetHttpDispatch(HttpData *aHttpData) { - HttpInfo::GetHttpConnectionData(&mHttp.data); - nsCOMPtr event = NS_NewRunnableMethod(this, &Dashboard::GetHttpConnections); - mHttp.thread->Dispatch(event, NS_DISPATCH_NORMAL); + nsRefPtr httpData = aHttpData; + HttpInfo::GetHttpConnectionData(&httpData->mData); + nsCOMPtr event = + NS_NewRunnableMethodWithArg > + (this, &Dashboard::GetHttpConnections, httpData); + httpData->mThread->Dispatch(event, NS_DISPATCH_NORMAL); + return NS_OK; } nsresult -Dashboard::GetHttpConnections() +Dashboard::GetHttpConnections(HttpData *aHttpData) { + nsRefPtr httpData = aHttpData; AutoSafeJSContext cx; mozilla::dom::HttpConnDict dict; @@ -150,21 +448,21 @@ Dashboard::GetHttpConnections() using mozilla::dom::HttpConnInfo; Sequence &connections = dict.mConnections.Value(); - uint32_t length = mHttp.data.Length(); + uint32_t length = httpData->mData.Length(); if (!connections.SetCapacity(length)) { - mHttp.cb = nullptr; - mHttp.data.Clear(); + httpData->mCallback = nullptr; + httpData->mData.Clear(); JS_ReportOutOfMemory(cx); return NS_ERROR_OUT_OF_MEMORY; } - for (uint32_t i = 0; i < mHttp.data.Length(); i++) { + for (uint32_t i = 0; i < httpData->mData.Length(); i++) { HttpConnectionElement &connection = *connections.AppendElement(); - CopyASCIItoUTF16(mHttp.data[i].host, connection.mHost); - connection.mPort = mHttp.data[i].port; - connection.mSpdy = mHttp.data[i].spdy; - connection.mSsl = mHttp.data[i].ssl; + CopyASCIItoUTF16(httpData->mData[i].host, connection.mHost); + connection.mPort = httpData->mData[i].port; + connection.mSpdy = httpData->mData[i].spdy; + connection.mSsl = httpData->mData[i].ssl; connection.mActive.Construct(); connection.mIdle.Construct(); @@ -174,50 +472,48 @@ Dashboard::GetHttpConnections() Sequence &idle = connection.mIdle.Value(); Sequence &halfOpens = connection.mHalfOpens.Value(); - if (!active.SetCapacity(mHttp.data[i].active.Length()) || - !idle.SetCapacity(mHttp.data[i].idle.Length()) || - !halfOpens.SetCapacity(mHttp.data[i].halfOpens.Length())) { - mHttp.cb = nullptr; - mHttp.data.Clear(); + if (!active.SetCapacity(httpData->mData[i].active.Length()) || + !idle.SetCapacity(httpData->mData[i].idle.Length()) || + !halfOpens.SetCapacity(httpData->mData[i].halfOpens.Length())) { + httpData->mCallback = nullptr; + httpData->mData.Clear(); JS_ReportOutOfMemory(cx); return NS_ERROR_OUT_OF_MEMORY; } - for (uint32_t j = 0; j < mHttp.data[i].active.Length(); j++) { + for (uint32_t j = 0; j < httpData->mData[i].active.Length(); j++) { HttpConnInfo &info = *active.AppendElement(); - info.mRtt = mHttp.data[i].active[j].rtt; - info.mTtl = mHttp.data[i].active[j].ttl; - info.mProtocolVersion = mHttp.data[i].active[j].protocolVersion; + info.mRtt = httpData->mData[i].active[j].rtt; + info.mTtl = httpData->mData[i].active[j].ttl; + info.mProtocolVersion = + httpData->mData[i].active[j].protocolVersion; } - for (uint32_t j = 0; j < mHttp.data[i].idle.Length(); j++) { + for (uint32_t j = 0; j < httpData->mData[i].idle.Length(); j++) { HttpConnInfo &info = *idle.AppendElement(); - info.mRtt = mHttp.data[i].idle[j].rtt; - info.mTtl = mHttp.data[i].idle[j].ttl; - info.mProtocolVersion = mHttp.data[i].idle[j].protocolVersion; + info.mRtt = httpData->mData[i].idle[j].rtt; + info.mTtl = httpData->mData[i].idle[j].ttl; + info.mProtocolVersion = httpData->mData[i].idle[j].protocolVersion; } - for (uint32_t j = 0; j < mHttp.data[i].halfOpens.Length(); j++) { + for (uint32_t j = 0; j < httpData->mData[i].halfOpens.Length(); j++) { HalfOpenInfoDict &info = *halfOpens.AppendElement(); - info.mSpeculative = mHttp.data[i].halfOpens[j].speculative; + info.mSpeculative = httpData->mData[i].halfOpens[j].speculative; } } JS::RootedValue val(cx); if (!dict.ToObject(cx, JS::NullPtr(), &val)) { - mHttp.cb = nullptr; - mHttp.data.Clear(); return NS_ERROR_FAILURE; } - mHttp.cb->OnDashboardDataAvailable(val); - mHttp.cb = nullptr; + + httpData->mCallback->OnDashboardDataAvailable(val); return NS_OK; } - NS_IMETHODIMP -Dashboard::GetEnableLogging(bool* value) +Dashboard::GetEnableLogging(bool *value) { *value = mEnableLogging; return NS_OK; @@ -235,11 +531,13 @@ Dashboard::AddHost(const nsACString& aHost, uint32_t aSerial, bool aEncrypted) { if (mEnableLogging) { mozilla::MutexAutoLock lock(mWs.lock); - LogData data(nsCString(aHost), aSerial, aEncrypted); - if (mWs.data.Contains(data)) + LogData mData(nsCString(aHost), aSerial, aEncrypted); + if (mWs.data.Contains(mData)) { return NS_OK; - if (!mWs.data.AppendElement(data)) + } + if (!mWs.data.AppendElement(mData)) { return NS_ERROR_OUT_OF_MEMORY; + } return NS_OK; } return NS_ERROR_FAILURE; @@ -290,32 +588,33 @@ Dashboard::NewMsgReceived(const nsACString& aHost, uint32_t aSerial, uint32_t aL } NS_IMETHODIMP -Dashboard::RequestWebsocketConnections(NetDashboardCallback* cb) +Dashboard::RequestWebsocketConnections(NetDashboardCallback *aCallback) { - if (mWs.cb) - return NS_ERROR_FAILURE; - mWs.cb = cb; - mWs.thread = NS_GetCurrentThread(); + nsRefPtr wsRequest = new WebSocketRequest(); + wsRequest->mCallback = aCallback; + wsRequest->mThread = NS_GetCurrentThread(); - nsCOMPtr event = NS_NewRunnableMethod(this, &Dashboard::GetWebSocketConnections); - mWs.thread->Dispatch(event, NS_DISPATCH_NORMAL); + nsCOMPtr event = + NS_NewRunnableMethodWithArg > + (this, &Dashboard::GetWebSocketConnections, wsRequest); + wsRequest->mThread->Dispatch(event, NS_DISPATCH_NORMAL); return NS_OK; } nsresult -Dashboard::GetWebSocketConnections() +Dashboard::GetWebSocketConnections(WebSocketRequest *aWsRequest) { + nsRefPtr wsRequest = aWsRequest; AutoSafeJSContext cx; mozilla::dom::WebSocketDict dict; dict.mWebsockets.Construct(); - Sequence &websockets = dict.mWebsockets.Value(); + Sequence &websockets = + dict.mWebsockets.Value(); mozilla::MutexAutoLock lock(mWs.lock); uint32_t length = mWs.data.Length(); if (!websockets.SetCapacity(length)) { - mWs.cb = nullptr; - mWs.data.Clear(); JS_ReportOutOfMemory(cx); return NS_ERROR_OUT_OF_MEMORY; } @@ -332,48 +631,53 @@ Dashboard::GetWebSocketConnections() JS::RootedValue val(cx); if (!dict.ToObject(cx, JS::NullPtr(), &val)) { - mWs.cb = nullptr; - mWs.data.Clear(); return NS_ERROR_FAILURE; } - mWs.cb->OnDashboardDataAvailable(val); - mWs.cb = nullptr; + wsRequest->mCallback->OnDashboardDataAvailable(val); return NS_OK; } NS_IMETHODIMP -Dashboard::RequestDNSInfo(NetDashboardCallback* cb) +Dashboard::RequestDNSInfo(NetDashboardCallback *aCallback) { - if (mDns.cb) - return NS_ERROR_FAILURE; - mDns.cb = cb; - nsresult rv; - mDns.data.Clear(); - mDns.thread = NS_GetCurrentThread(); + nsRefPtr dnsData = new DnsData(); + dnsData->mCallback = aCallback; - if (!mDns.serv) { - mDns.serv = do_GetService("@mozilla.org/network/dns-service;1", &rv); - if (NS_FAILED(rv)) + nsresult rv; + dnsData->mData.Clear(); + dnsData->mThread = NS_GetCurrentThread(); + + if (!mDnsService) { + mDnsService = do_GetService("@mozilla.org/network/dns-service;1", &rv); + if (NS_FAILED(rv)) { return rv; + } } - nsCOMPtr event = NS_NewRunnableMethod(this, &Dashboard::GetDnsInfoDispatch); + nsCOMPtr event = + NS_NewRunnableMethodWithArg > + (this, &Dashboard::GetDnsInfoDispatch, dnsData); gSocketTransportService->Dispatch(event, NS_DISPATCH_NORMAL); return NS_OK; } -void -Dashboard::GetDnsInfoDispatch() +nsresult +Dashboard::GetDnsInfoDispatch(DnsData *aDnsData) { - if (mDns.serv) - mDns.serv->GetDNSCacheEntries(&mDns.data); - nsCOMPtr event = NS_NewRunnableMethod(this, &Dashboard::GetDNSCacheEntries); - mDns.thread->Dispatch(event, NS_DISPATCH_NORMAL); + nsRefPtr dnsData = aDnsData; + if (mDnsService) { + mDnsService->GetDNSCacheEntries(&dnsData->mData); + } + nsCOMPtr event = + NS_NewRunnableMethodWithArg > + (this, &Dashboard::GetDNSCacheEntries, dnsData); + dnsData->mThread->Dispatch(event, NS_DISPATCH_NORMAL); + return NS_OK; } nsresult -Dashboard::GetDNSCacheEntries() +Dashboard::GetDNSCacheEntries(DnsData *dnsData) { AutoSafeJSContext cx; @@ -381,111 +685,66 @@ Dashboard::GetDNSCacheEntries() dict.mEntries.Construct(); Sequence &entries = dict.mEntries.Value(); - uint32_t length = mDns.data.Length(); + uint32_t length = dnsData->mData.Length(); if (!entries.SetCapacity(length)) { - mDns.cb = nullptr; - mDns.data.Clear(); JS_ReportOutOfMemory(cx); return NS_ERROR_OUT_OF_MEMORY; } - for (uint32_t i = 0; i < mDns.data.Length(); i++) { + for (uint32_t i = 0; i < dnsData->mData.Length(); i++) { mozilla::dom::DnsCacheEntry &entry = *entries.AppendElement(); entry.mHostaddr.Construct(); Sequence &addrs = entry.mHostaddr.Value(); - if (!addrs.SetCapacity(mDns.data[i].hostaddr.Length())) { - mDns.cb = nullptr; - mDns.data.Clear(); + if (!addrs.SetCapacity(dnsData->mData[i].hostaddr.Length())) { JS_ReportOutOfMemory(cx); return NS_ERROR_OUT_OF_MEMORY; } - CopyASCIItoUTF16(mDns.data[i].hostname, entry.mHostname); - entry.mExpiration = mDns.data[i].expiration; + CopyASCIItoUTF16(dnsData->mData[i].hostname, entry.mHostname); + entry.mExpiration = dnsData->mData[i].expiration; - for (uint32_t j = 0; j < mDns.data[i].hostaddr.Length(); j++) { - CopyASCIItoUTF16(mDns.data[i].hostaddr[j], *addrs.AppendElement()); + for (uint32_t j = 0; j < dnsData->mData[i].hostaddr.Length(); j++) { + CopyASCIItoUTF16(dnsData->mData[i].hostaddr[j], + *addrs.AppendElement()); } - if (mDns.data[i].family == PR_AF_INET6) + if (dnsData->mData[i].family == PR_AF_INET6) { CopyASCIItoUTF16("ipv6", entry.mFamily); - else + } else { CopyASCIItoUTF16("ipv4", entry.mFamily); + } } JS::RootedValue val(cx); if (!dict.ToObject(cx, JS::NullPtr(), &val)) { - mDns.cb = nullptr; - mDns.data.Clear(); return NS_ERROR_FAILURE; } - mDns.cb->OnDashboardDataAvailable(val); - mDns.cb = nullptr; + dnsData->mCallback->OnDashboardDataAvailable(val); return NS_OK; } NS_IMETHODIMP -Dashboard::RequestDNSLookup(const nsACString &aHost, NetDashboardCallback *cb) +Dashboard::RequestDNSLookup(const nsACString &aHost, + NetDashboardCallback *mCallback) { - if (mDnsup.cb) - return NS_ERROR_FAILURE; nsresult rv; - if (!mDnsup.serv) { - mDnsup.serv = do_GetService("@mozilla.org/network/dns-service;1", &rv); - if (NS_FAILED(rv)) + if (!mDnsService) { + mDnsService = do_GetService("@mozilla.org/network/dns-service;1", &rv); + if (NS_FAILED(rv)) { return rv; - } - - mDnsup.cb = cb; - rv = mDnsup.serv->AsyncResolve(aHost, 0, this, NS_GetCurrentThread(), getter_AddRefs(mDnsup.cancel)); - if (NS_FAILED(rv)) { - mDnsup.cb = nullptr; - return rv; - } - - return NS_OK; -} - -NS_IMETHODIMP -Dashboard::OnLookupComplete(nsICancelable *aRequest, nsIDNSRecord *aRecord, nsresult aStatus) -{ - MOZ_ASSERT(aRequest == mDnsup.cancel); - mDnsup.cancel = nullptr; - - AutoSafeJSContext cx; - - mozilla::dom::DNSLookupDict dict; - dict.mAddress.Construct(); - - Sequence &addresses = dict.mAddress.Value(); - - if (NS_SUCCEEDED(aStatus)) { - dict.mAnswer = true; - bool hasMore; - aRecord->HasMore(&hasMore); - while(hasMore) { - nsCString nextAddress; - aRecord->GetNextAddrAsString(nextAddress); - CopyASCIItoUTF16(nextAddress, *addresses.AppendElement()); - aRecord->HasMore(&hasMore); } - } else { - dict.mAnswer = false; - CopyASCIItoUTF16(GetErrorString(aStatus), dict.mError); } - JS::RootedValue val(cx); - if (!dict.ToObject(cx, JS::NullPtr(), &val)) { - mDnsup.cb = nullptr; - return NS_ERROR_FAILURE; - } - mDnsup.cb->OnDashboardDataAvailable(val); - mDnsup.cb = nullptr; - - return NS_OK; + nsRefPtr helper = new LookupHelper(); + helper->mCallback = mCallback; + helper->mThread = NS_GetCurrentThread(); + rv = mDnsService->AsyncResolve(aHost, 0, helper.get(), + NS_GetCurrentThread(), + getter_AddRefs(helper->mCancel)); + return rv; } void @@ -525,19 +784,25 @@ HttpConnInfo::SetHTTP2ProtocolVersion(uint8_t pv) NS_IMETHODIMP Dashboard::RequestConnection(const nsACString& aHost, uint32_t aPort, const char *aProtocol, uint32_t aTimeout, - NetDashboardCallback* cb) + NetDashboardCallback *aCallback) { nsresult rv; - mConn.cb = cb; - mConn.thread = NS_GetCurrentThread(); + nsRefPtr connectionData = new ConnectionData(this); + connectionData->mHost = aHost; + connectionData->mPort = aPort; + connectionData->mProtocol = aProtocol; + connectionData->mTimeout = aTimeout; - rv = TestNewConnection(aHost, aPort, aProtocol, aTimeout); + connectionData->mCallback = aCallback; + connectionData->mThread = NS_GetCurrentThread(); + + rv = TestNewConnection(connectionData); if (NS_FAILED(rv)) { - ConnStatus status; - CopyASCIItoUTF16(GetErrorString(rv), status.creationSts); + CopyASCIItoUTF16(GetErrorString(rv), connectionData->mStatus); nsCOMPtr event = - NS_NewRunnableMethodWithArg(this, &Dashboard::GetConnectionStatus, status); - mConn.thread->Dispatch(event, NS_DISPATCH_NORMAL); + NS_NewRunnableMethodWithArg > + (this, &Dashboard::GetConnectionStatus, connectionData); + connectionData->mThread->Dispatch(event, NS_DISPATCH_NORMAL); return rv; } @@ -545,107 +810,68 @@ Dashboard::RequestConnection(const nsACString& aHost, uint32_t aPort, } nsresult -Dashboard::GetConnectionStatus(ConnStatus aStatus) +Dashboard::GetConnectionStatus(ConnectionData *aConnectionData) { + nsRefPtr connectionData = aConnectionData; AutoSafeJSContext cx; mozilla::dom::ConnStatusDict dict; - dict.mStatus = aStatus.creationSts; + dict.mStatus = connectionData->mStatus; JS::RootedValue val(cx); - if (!dict.ToObject(cx, JS::NullPtr(), &val)) { - mConn.cb = nullptr; + if (!dict.ToObject(cx, JS::NullPtr(), &val)) return NS_ERROR_FAILURE; - } - mConn.cb->OnDashboardDataAvailable(val); + + connectionData->mCallback->OnDashboardDataAvailable(val); return NS_OK; } nsresult -Dashboard::TestNewConnection(const nsACString& aHost, uint32_t aPort, - const char *aProtocol, uint32_t aTimeout) +Dashboard::TestNewConnection(ConnectionData *aConnectionData) { + nsRefPtr connectionData = aConnectionData; + nsresult rv; - if (!aHost.Length() || !net_IsValidHostName(aHost)) + if (!connectionData->mHost.Length() || + !net_IsValidHostName(connectionData->mHost)) { return NS_ERROR_UNKNOWN_HOST; + } - if (aProtocol && NS_LITERAL_STRING("ssl").EqualsASCII(aProtocol)) - rv = gSocketTransportService->CreateTransport(&aProtocol, 1, aHost, - aPort, nullptr, - getter_AddRefs(mConn.socket)); - else - rv = gSocketTransportService->CreateTransport(nullptr, 0, aHost, - aPort, nullptr, - getter_AddRefs(mConn.socket)); - if (NS_FAILED(rv)) + if (connectionData->mProtocol && + NS_LITERAL_STRING("ssl").EqualsASCII(connectionData->mProtocol)) { + rv = gSocketTransportService->CreateTransport( + &connectionData->mProtocol, 1, connectionData->mHost, + connectionData->mPort, nullptr, + getter_AddRefs(connectionData->mSocket)); + } else { + rv = gSocketTransportService->CreateTransport( + nullptr, 0, connectionData->mHost, + connectionData->mPort, nullptr, + getter_AddRefs(connectionData->mSocket)); + } + if (NS_FAILED(rv)) { return rv; + } - rv = mConn.socket->SetEventSink(this, NS_GetCurrentThread()); - if (NS_FAILED(rv)) + rv = connectionData->mSocket->SetEventSink(connectionData, + NS_GetCurrentThread()); + if (NS_FAILED(rv)) { return rv; + } - rv = mConn.socket->OpenInputStream(nsITransport::OPEN_BLOCKING, 0, 0, - getter_AddRefs(mConn.streamIn)); - if (NS_FAILED(rv)) + rv = connectionData->mSocket->OpenInputStream( + nsITransport::OPEN_BLOCKING, 0, 0, + getter_AddRefs(connectionData->mStreamIn)); + if (NS_FAILED(rv)) { return rv; + } - StartTimer(aTimeout); + connectionData->StartTimer(connectionData->mTimeout); return rv; } -NS_IMETHODIMP -Dashboard::OnTransportStatus(nsITransport *aTransport, nsresult aStatus, - uint64_t aProgress, uint64_t aProgressMax) -{ - if (aStatus == NS_NET_STATUS_CONNECTED_TO) - StopTimer(); - - ConnStatus status; - CopyASCIItoUTF16(GetErrorString(aStatus), status.creationSts); - nsCOMPtr event = NS_NewRunnableMethodWithArg(this, &Dashboard::GetConnectionStatus, status); - mConn.thread->Dispatch(event, NS_DISPATCH_NORMAL); - - return NS_OK; -} - -NS_IMETHODIMP -Dashboard::Notify(nsITimer *timer) -{ - if (mConn.socket) { - mConn.socket->Close(NS_ERROR_ABORT); - mConn.socket = nullptr; - mConn.streamIn = nullptr; - } - - mConn.timer = nullptr; - - ConnStatus status; - status.creationSts.Assign(NS_LITERAL_STRING("NS_ERROR_NET_TIMEOUT")); - nsCOMPtr event = NS_NewRunnableMethodWithArg(this, &Dashboard::GetConnectionStatus, status); - mConn.thread->Dispatch(event, NS_DISPATCH_NORMAL); - - return NS_OK; -} - -void -Dashboard::StartTimer(uint32_t aTimeout) -{ - if (!mConn.timer) - mConn.timer = do_CreateInstance("@mozilla.org/timer;1"); - mConn.timer->InitWithCallback(this, aTimeout * 1000, nsITimer::TYPE_ONE_SHOT); -} - -void -Dashboard::StopTimer() -{ - if (mConn.timer) { - mConn.timer->Cancel(); - mConn.timer = nullptr; - } -} - typedef struct { nsresult key; @@ -675,13 +901,15 @@ Dashboard::GetErrorString(nsresult rv) { int length = sizeof(socketTransportStatuses) / sizeof(ErrorEntry); for (int i = 0;i < length;i++) - if (socketTransportStatuses[i].key == rv) + if (socketTransportStatuses[i].key == rv) { return socketTransportStatuses[i].error; + } length = sizeof(errors) / sizeof(ErrorEntry); for (int i = 0;i < length;i++) - if (errors[i].key == rv) + if (errors[i].key == rv) { return errors[i].error; + } return nullptr; } diff --git a/netwerk/base/src/Dashboard.h b/netwerk/base/src/Dashboard.h index bb82a23d37f..9ff64f3c3a4 100644 --- a/netwerk/base/src/Dashboard.h +++ b/netwerk/base/src/Dashboard.h @@ -21,57 +21,26 @@ class nsIThread; namespace mozilla { namespace net { -class Dashboard: - public nsIDashboard, - public nsIDashboardEventNotifier, - public nsITransportEventSink, - public nsITimerCallback, - public nsIDNSListener +class SocketData; +class HttpData; +class DnsData; +class WebSocketRequest; +class ConnectionData; + +class Dashboard + : public nsIDashboard + , public nsIDashboardEventNotifier { public: NS_DECL_THREADSAFE_ISUPPORTS NS_DECL_NSIDASHBOARD NS_DECL_NSIDASHBOARDEVENTNOTIFIER - NS_DECL_NSITRANSPORTEVENTSINK - NS_DECL_NSITIMERCALLBACK - NS_DECL_NSIDNSLISTENER Dashboard(); - friend class DashConnStatusRunnable; static const char *GetErrorString(nsresult rv); -private: - virtual ~Dashboard(); - - void GetSocketsDispatch(); - void GetHttpDispatch(); - void GetDnsInfoDispatch(); - void StartTimer(uint32_t aTimeout); - void StopTimer(); - nsresult TestNewConnection(const nsACString& aHost, uint32_t aPort, - const char *aProtocol, uint32_t aTimeout); - - /* Helper methods that pass the JSON to the callback function. */ - nsresult GetSockets(); - nsresult GetHttpConnections(); - nsresult GetWebSocketConnections(); - nsresult GetDNSCacheEntries(); - nsresult GetConnectionStatus(struct ConnStatus aStatus); + nsresult GetConnectionStatus(ConnectionData *aConnectionData); private: - struct SocketData - { - uint64_t totalSent; - uint64_t totalRecv; - nsTArray data; - nsCOMPtr cb; - nsIThread* thread; - }; - - struct HttpData { - nsTArray data; - nsCOMPtr cb; - nsIThread* thread; - }; struct LogData { @@ -109,42 +78,27 @@ private: } nsTArray data; mozilla::Mutex lock; - nsCOMPtr cb; - nsIThread* thread; }; - struct DnsData - { - nsCOMPtr serv; - nsTArray data; - nsCOMPtr cb; - nsIThread* thread; - }; - - struct DnsLookup - { - nsCOMPtr serv; - nsCOMPtr cancel; - nsCOMPtr cb; - }; - - struct ConnectionData - { - nsCOMPtr socket; - nsCOMPtr streamIn; - nsCOMPtr timer; - nsCOMPtr cb; - nsIThread* thread; - }; bool mEnableLogging; + WebSocketData mWs; - struct SocketData mSock; - struct HttpData mHttp; - struct WebSocketData mWs; - struct DnsData mDns; - struct DnsLookup mDnsup; - struct ConnectionData mConn; +private: + virtual ~Dashboard(); + + nsresult GetSocketsDispatch(SocketData *); + nsresult GetHttpDispatch(HttpData *); + nsresult GetDnsInfoDispatch(DnsData *); + nsresult TestNewConnection(ConnectionData *); + + /* Helper methods that pass the JSON to the callback function. */ + nsresult GetSockets(SocketData *); + nsresult GetHttpConnections(HttpData *); + nsresult GetDNSCacheEntries(DnsData *); + nsresult GetWebSocketConnections(WebSocketRequest *); + + nsCOMPtr mDnsService; }; } } // namespace mozilla::net diff --git a/services/sync/modules/engines/bookmarks.js b/services/sync/modules/engines/bookmarks.js index 3484dbee46b..fe082547970 100644 --- a/services/sync/modules/engines/bookmarks.js +++ b/services/sync/modules/engines/bookmarks.js @@ -731,10 +731,10 @@ BookmarksStore.prototype = { feedURI: Utils.makeURI(record.feedUri), siteURI: siteURI, guid: record.id}; - PlacesUtils.livemarks.addLivemark(livemarkObj, - function (aStatus, aLivemark) { - spinningCb(null, [aStatus, aLivemark]); - }); + PlacesUtils.livemarks.addLivemark(livemarkObj).then( + aLivemark => { spinningCb(null, [Components.results.NS_OK, aLivemark]) }, + () => { spinningCb(null, [Components.results.NS_ERROR_UNEXPECTED, aLivemark]) } + ); let [status, livemark] = spinningCb.wait(); if (!Components.isSuccessCode(status)) { diff --git a/services/sync/modules/policies.js b/services/sync/modules/policies.js index f3a8093f692..9f820a8e957 100644 --- a/services/sync/modules/policies.js +++ b/services/sync/modules/policies.js @@ -38,7 +38,14 @@ SyncScheduler.prototype = { setDefaults: function setDefaults() { this._log.trace("Setting SyncScheduler policy values to defaults."); - this.singleDeviceInterval = Svc.Prefs.get("scheduler.singleDeviceInterval") * 1000; + let service = Cc["@mozilla.org/weave/service;1"] + .getService(Ci.nsISupports) + .wrappedJSObject; + + let part = service.fxAccountsEnabled ? "fxa" : "sync11"; + let prefSDInterval = "scheduler." + part + ".singleDeviceInterval"; + this.singleDeviceInterval = Svc.Prefs.get(prefSDInterval) * 1000; + this.idleInterval = Svc.Prefs.get("scheduler.idleInterval") * 1000; this.activeInterval = Svc.Prefs.get("scheduler.activeInterval") * 1000; this.immediateInterval = Svc.Prefs.get("scheduler.immediateInterval") * 1000; diff --git a/services/sync/services-sync.js b/services/sync/services-sync.js index d9fe420993d..0cd62309273 100644 --- a/services/sync/services-sync.js +++ b/services/sync/services-sync.js @@ -14,12 +14,14 @@ pref("services.sync.lastversion", "firstrun"); pref("services.sync.sendVersionInfo", true); pref("services.sync.scheduler.eolInterval", 604800); // 1 week -pref("services.sync.scheduler.singleDeviceInterval", 86400); // 1 day pref("services.sync.scheduler.idleInterval", 3600); // 1 hour pref("services.sync.scheduler.activeInterval", 600); // 10 minutes pref("services.sync.scheduler.immediateInterval", 90); // 1.5 minutes pref("services.sync.scheduler.idleTime", 300); // 5 minutes +pref("services.sync.scheduler.fxa.singleDeviceInterval", 3600); // 1 hour +pref("services.sync.scheduler.sync11.singleDeviceInterval", 86400); // 1 day + pref("services.sync.errorhandler.networkFailureReportTimeout", 1209600); // 2 weeks pref("services.sync.engine.addons", true); diff --git a/services/sync/tests/unit/test_syncscheduler.js b/services/sync/tests/unit/test_syncscheduler.js index 8f3f2b126b6..04b6ffa31cc 100644 --- a/services/sync/tests/unit/test_syncscheduler.js +++ b/services/sync/tests/unit/test_syncscheduler.js @@ -125,7 +125,7 @@ add_test(function test_prefAttributes() { _("Intervals correspond to default preferences."); do_check_eq(scheduler.singleDeviceInterval, - Svc.Prefs.get("scheduler.singleDeviceInterval") * 1000); + Svc.Prefs.get("scheduler.sync11.singleDeviceInterval") * 1000); do_check_eq(scheduler.idleInterval, Svc.Prefs.get("scheduler.idleInterval") * 1000); do_check_eq(scheduler.activeInterval, @@ -134,7 +134,7 @@ add_test(function test_prefAttributes() { Svc.Prefs.get("scheduler.immediateInterval") * 1000); _("Custom values for prefs will take effect after a restart."); - Svc.Prefs.set("scheduler.singleDeviceInterval", 42); + Svc.Prefs.set("scheduler.sync11.singleDeviceInterval", 42); Svc.Prefs.set("scheduler.idleInterval", 23); Svc.Prefs.set("scheduler.activeInterval", 18); Svc.Prefs.set("scheduler.immediateInterval", 31415); diff --git a/services/sync/tps/extensions/tps/modules/bookmarks.jsm b/services/sync/tps/extensions/tps/modules/bookmarks.jsm index 3dc832846c9..d7f1e85a20c 100644 --- a/services/sync/tps/extensions/tps/modules/bookmarks.jsm +++ b/services/sync/tps/extensions/tps/modules/bookmarks.jsm @@ -784,10 +784,10 @@ Livemark.prototype = { // Until this can handle asynchronous creation, we need to spin. let spinningCb = Async.makeSpinningCallback(); - PlacesUtils.livemarks.addLivemark(livemarkObj, - function (aStatus, aLivemark) { - spinningCb(null, [aStatus, aLivemark]); - }); + PlacesUtils.livemarks.addLivemark(livemarkObj).then( + aLivemark => { spinningCb(null, [Components.results.NS_OK, aLivemark]) }, + () => { spinningCb(null, [Components.results.NS_ERROR_UNEXPECTED, aLivemark]) } + ); let [status, livemark] = spinningCb.wait(); if (!Components.isSuccessCode(status)) { diff --git a/toolkit/components/places/BookmarkHTMLUtils.jsm b/toolkit/components/places/BookmarkHTMLUtils.jsm index d9dc9ce0127..c41806cbd0b 100644 --- a/toolkit/components/places/BookmarkHTMLUtils.jsm +++ b/toolkit/components/places/BookmarkHTMLUtils.jsm @@ -669,7 +669,7 @@ BookmarkImporter.prototype = { "index": PlacesUtils.bookmarks.DEFAULT_INDEX, "feedURI": frame.previousFeed, "siteURI": frame.previousLink, - }); + }).then(null, Cu.reportError); } else if (frame.previousLink) { // This is a common bookmark. PlacesUtils.bookmarks.setItemTitle(frame.previousId, diff --git a/toolkit/components/places/BookmarkJSONUtils.jsm b/toolkit/components/places/BookmarkJSONUtils.jsm index 91575ccbcf7..f2bb9863f6f 100644 --- a/toolkit/components/places/BookmarkJSONUtils.jsm +++ b/toolkit/components/places/BookmarkJSONUtils.jsm @@ -386,15 +386,13 @@ BookmarkImporter.prototype = { index: aIndex, lastModified: aData.lastModified, siteURI: siteURI - }, function(aStatus, aLivemark) { - if (Components.isSuccessCode(aStatus)) { - let id = aLivemark.id; - if (aData.dateAdded) - PlacesUtils.bookmarks.setItemDateAdded(id, aData.dateAdded); - if (aData.annos && aData.annos.length) - PlacesUtils.setAnnotationsForItem(id, aData.annos); - } - }); + }).then(function (aLivemark) { + let id = aLivemark.id; + if (aData.dateAdded) + PlacesUtils.bookmarks.setItemDateAdded(id, aData.dateAdded); + if (aData.annos && aData.annos.length) + PlacesUtils.setAnnotationsForItem(id, aData.annos); + }, Cu.reportError); } } else { id = PlacesUtils.bookmarks.createFolder( diff --git a/toolkit/components/places/PlacesTransactions.jsm b/toolkit/components/places/PlacesTransactions.jsm new file mode 100644 index 00000000000..dffc9e42331 --- /dev/null +++ b/toolkit/components/places/PlacesTransactions.jsm @@ -0,0 +1,1182 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +"use strict"; + +this.EXPORTED_SYMBOLS = ["PlacesTransactions"]; + +/** + * Overview + * -------- + * This modules serves as the transactions manager for Places, and implements + * all the standard transactions for its UI commands (creating items, editing + * various properties, etc.). It shares most of its semantics with common + * command pattern implementations, the HTML5 Undo Manager in particular. + * However, the asynchronous design of [future] Places APIs, combined with the + * commitment to serialize all UI operations, makes things a little bit + * different. For example, when |undo| is called in order to undo the top undo + * entry, the caller cannot tell for sure what entry would it be because the + * execution of some transaction is either in process, or queued. + * + * GUIDs and item-ids + * ------------------- + * The Bookmarks API still relies heavily on item-ids, but since those do not + * play nicely with the concept of undo and redo (especially not in an + * asynchronous environment), this API only accepts bookmark GUIDs, both for + * input (e.g. for specifying the parent folder for a new bookmark) and for + * output (when the GUID for such a bookmark is propagated). + * + * GUIDs are readily available when dealing with the "output" of this API and + * when result nodes are used (see nsINavHistoryResultNode::bookmarkGUID). + * If you only have item-ids in hand, use PlacesUtils.promiseItemGUID for + * converting them. Should you need to convert them back into itemIds, use + * PlacesUtils.promiseItemId. + * + * The Standard Transactions + * ------------------------- + * At the bottom of this module you will find implementations for all Places UI + * commands (One should almost never fallback to raw Places APIs. Please file + * a bug if you find anything uncovered). The transactions' constructors are + * set on the PlacesTransactions object (e.g. PlacesTransactions.NewFolder). + * The input for this constructors is taken in the form of a single argument + * plain object. Input properties may be either required (e.g. the |keyword| + * property for the EditKeyword transaction) or optional (e.g. the |keyword| + * property for NewBookmark). Once a transaction is created, you may pass it + * to |transact| or use it in the for batching (see next section). + * + * The constructors throw right away when any required input is missing or when + * some input is invalid "on the surface" (e.g. GUID values are validated to be + * 12-characters strings, but are not validated to point to existing item. Such + * an error will reveal when the transaction is executed). + * + * To make things simple, a given input property has the same basic meaning and + * valid values across all transactions which accept it in the input object. + * Here is a list of all supported input properties along with their expected + * values: + * - uri: an nsIURI object. + * - feedURI: an nsIURI object, holding the url for a live bookmark. + * - siteURI: an nsIURI object, holding the url for the site with which + * a live bookmark is associated. + * - GUID, parentGUID, newParentGUID: a valid places GUID string. + * - title: a string + * - index, newIndex: the position of an item in its containing folder, + * starting from 0. + * integer and PlacesUtils.bookmarks.DEFAULT_INDEX + * - annotationObject: see PlacesUtils.setAnnotationsForItem + * - annotations: an array of annotation objects as above. + * - tags: an array of strings. + * + * Batching transactions + * --------------------- + * Sometimes it is useful to "batch" or "merge" transactions. For example, + * "Bookmark All Tabs" may be implemented as one NewFolder transaction followed + * by numerous NewBookmark transactions - all to be undone or redone in a single + * command. The |transact| method makes this possible using a generator + * function as an input. These generators have the same semantics as in + * Task.jsm except that when you yield a transaction, it's executed, and the + * resolution (e.g. the new bookmark GUID) is sent to the generator so you can + * use it as the input for another transaction. See |transact| for the details. + * + * "Custom" transactions + * ---------------------- + * While it is technically possible to pass in your own transactions to + * |transact|, it is highly recommended to avoid that for two reasons: + * (*) Slippery memory leak - A transaction implemented in the context of a + * window reference that window through its prototype (and may also + * do so due to js closures, prototypes for non-primitives and so on). + * Because this module strongly references all transactions until they + * are cleared from the transactions history (if at all), such a transaction + * would block the release of the window object (along with a lot of other + * stuff) from memory when the window is closed. + * (*) The Places Bookmarks API is due for a major overhaul - We expect + * nsINavBookmarks to be replaced by an asynchronous interface sometime in + * the near future. This API is expected to remain mostly unchanged in + * that process. + * + * It's almost always possible to avoid a custom transaction by batching the + * standard transactions. However, if you must implement a custom transaction, + * do so in a JS module. + * + * The transactions-history structure + * ---------------------------------- + * The transactions-history is a two-dimensional stack of transactions: the + * transactions are ordered in reverse to the order they were committed. + * It's two-dimensional because the undo manager allows batching transactions + * together for the purpose of undo or redo (batched transactions can never be + * undone or redone partially). + * + * The undoPosition property is set to the index of the top entry. If there is + * no entry at that index, there is nothing to undo. + * Entries prior to undoPosition, if any, are redo entries, the first one being + * the top redo entry. + * + * [ [2nd redo txn, 1st redo txn], <= 2nd redo entry + * [2nd redo txn, 1st redo txn], <= 1st redo entry + * [1st undo txn, 2nd undo txn], <= 1st undo entry + * [1st undo txn, 2nd undo txn] <= 2nd undo entry ] + * undoPostion: 2. + * + * Note that when a new entry is created, all redo entries are removed. + */ + +Components.utils.import("resource://gre/modules/XPCOMUtils.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "Promise", + "resource://gre/modules/Promise.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "Task", + "resource://gre/modules/Task.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "NetUtil", + "resource://gre/modules/NetUtil.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "PlacesUtils", + "resource://gre/modules/PlacesUtils.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "console", + "resource://gre/modules/devtools/Console.jsm"); + +// The internal object for managing the transactions history. +// The public API is included in PlacesTransactions. +// TODO bug 982099: extending the array "properly" makes it painful to implement +// getters. If/when ES6 gets proper array subclassing we can revise this. +let TransactionsHistory = []; +TransactionsHistory.__proto__ = { + __proto__: Array.prototype, + + // The index of the first undo entry (if any) - See the documentation + // at the top of this file. + _undoPosition: 0, + get undoPosition() this._undoPosition, + + // Handy shortcuts + get topUndoEntry() this.undoPosition < this.length ? + this[this.undoPosition] : null, + get topRedoEntry() this.undoPosition > 0 ? + this[this.undoPosition - 1] : null, + + /** + * Undo the top undo entry, if any, and update the undo position accordingly. + */ + undo: function* () { + let entry = this.topUndoEntry; + if (!entry) + return; + + for (let transaction of entry) { + try { + yield transaction.undo(); + } + catch(ex) { + // If one transaction is broken, it's not safe to work with any other + // undo entry. Report the error and clear the undo history. + console.error(ex, + "Couldn't undo a transaction, clearing all undo entries."); + this.clearUndoEntries(); + return; + } + } + this._undoPosition++; + }, + + /** + * Redo the top redo entry, if any, and update the undo position accordingly. + */ + redo: function* () { + let entry = this.topRedoEntry; + if (!entry) + return; + + for (let i = entry.length - 1; i >= 0; i--) { + let transaction = entry[i]; + try { + if (transaction.redo) + yield transaction.redo(); + else + yield transaction.execute(); + } + catch(ex) { + // If one transaction is broken, it's not safe to work with any other + // redo entry. Report the error and clear the undo history. + console.error(ex, + "Couldn't redo a transaction, clearing all redo entries."); + this.clearRedoEntries(); + return; + } + } + this._undoPosition--; + }, + + /** + * Add a transaction either as a new entry, if forced or if there are no undo + * entries, or to the top undo entry. + * + * @param aTransaction + * the transaction object to be added to the transaction history. + * @param [optional] aForceNewEntry + * Force a new entry for the transaction. Default: false. + * If false, an entry will we created only if there's no undo entry + * to extend. + */ + add: function (aTransaction, aForceNewEntry = false) { + if (this.length == 0 || aForceNewEntry) { + this.clearRedoEntries(); + this.unshift([aTransaction]); + } + else { + this[this.undoPosition].unshift(aTransaction); + } + }, + + /** + * Clear all undo entries. + */ + clearUndoEntries: function () { + if (this.undoPosition < this.length) + this.splice(this.undoPosition); + }, + + /** + * Clear all redo entries. + */ + clearRedoEntries: function () { + if (this.undoPosition > 0) { + this.splice(0, this.undoPosition); + this._undoPosition = 0; + } + }, + + /** + * Clear all entries. + */ + clearAllEntries: function () { + if (this.length > 0) { + this.splice(0); + this._undoPosition = 0; + } + } +}; + + +// Our transaction manager is asynchronous in the sense that all of its methods +// don't execute synchronously. However, all actions must be serialized. +let currentTask = Promise.resolve(); +function Serialize(aTask) { + // Ignore failures. + return currentTask = currentTask.then( () => Task.spawn(aTask) ) + .then(null, Components.utils.reportError); +} + +let PlacesTransactions = { + /** + * Asynchronously transact either a single transaction, or a sequence of + * transactions that would be treated as a single entry in the transactions + * history. + * + * @param aTransactionOrGeneratorFunction + * Either a transaction object or a generator function (ES6-style only) + * that yields transaction objects. + * + * Generator mode how-to: when a transaction is yielded, it's executed. + * Then, if it was executed successfully, the resolution of |execute| + * is sent to the generator. If |execute| threw or rejected, the + * exception is propagated to the generator. + * Any other value yielded by a generator function is handled the + * same way as in a Task (see Task.jsm). + * + * @return {Promise} + * @resolves either to the resolution of |execute|, in single-transaction mode, + * or to the return value of the generator, in generator-mode. + * @rejects either if |execute| threw, in single-transaction mode, or if + * the generator function threw (or didn't handle) an exception, in generator + * mode. + * + * @note If no transaction was executed successfully, the transactions history + * is not affected. + * + * @note All PlacesTransactions operations are serialized. This means that the + * transactions history state may change by the time the transaction/generator + * is processed. It's guaranteed, however, that a generator function "blocks" + * the queue: that is, it is assured that no other operations are performed + * by or on PlacesTransactions until the generator returns. Keep in mind you + * are not protected from consumers who use the raw places APIs directly. + */ + transact: function (aTransactionOrGeneratorFunction) { + return Serialize(function* () { + // The entry in the transactions history is created once the first + // transaction is committed. This means that if |transact| is called + // in its "generator mode" and no transactions are committed by the + // generator, the transactions history is left unchanged. + // Bug 982115: Depending on how this API is actually used we may revise + // this decision and make it so |transact| always forces a new entry. + let forceNewEntry = true; + function* transactOneTransaction(aTransaction) { + let retval = yield aTransaction.execute(); + TransactionsHistory.add(aTransaction, forceNewEntry); + forceNewEntry = false; + return retval; + } + + function* transactBatch(aGeneratorFunction) { + let generator = aGeneratorFunction(), sendValue = undefined; + while (true) { + let next = generator.next(sendValue); + sendValue = next.value; + if (typeof(sendValue) == "function") { + sendValue = yield transactBatch(sendValue); + } + else if (typeof(sendValue) == "object" && sendValue) { + if ("execute" in sendValue) + sendValue = yield transactOneTransaction(sendValue); + else if ("then" in sendValue) + sendValue = yield sendValue; + } + if (next.done) + break; + } + return sendValue; + } + + if (typeof(aTransactionOrGeneratorFunction) == "function") + return yield transactBatch(aTransactionOrGeneratorFunction); + else + return yield transactOneTransaction(aTransactionOrGeneratorFunction); + }.bind(this)); + }, + + /** + * Asynchronously undo the transaction immediately after the current undo + * position in the transactions history in the reverse order, if any, and + * adjusts the undo position. + * + * @return {Promises). The promise always resolves. + * @note All undo manager operations are queued. This means that transactions + * history may change by the time your request is fulfilled. + */ + undo: function () Serialize(() => TransactionsHistory.undo()), + + /** + * Asynchronously redo the transaction immediately before the current undo + * position in the transactions history, if any, and adjusts the undo + * position. + * + * @return {Promises). The promise always resolves. + * @note All undo manager operations are queued. This means that transactions + * history may change by the time your request is fulfilled. + */ + redo: function () Serialize(() => TransactionsHistory.redo()), + + /** + * Asynchronously clear the undo, redo, or all entries from the transactions + * history. + * + * @param [optional] aUndoEntries + * Whether or not to clear undo entries. Default: true. + * @param [optional] aRedoEntries + * Whether or not to clear undo entries. Default: true. + * + * @return {Promises). The promise always resolves. + * @throws if both aUndoEntries and aRedoEntries are false. + * @note All undo manager operations are queued. This means that transactions + * history may change by the time your request is fulfilled. + */ + clearTransactionsHistory: + function (aUndoEntries = true, aRedoEntries = true) { + return Serialize(function* () { + if (aUndoEntries && aRedoEntries) + TransactionsHistory.clearAllEntries(); + else if (aUndoEntries) + TransactionsHistory.clearUndoEntries(); + else if (aRedoEntries) + TransactionsHistory.clearRedoEntries(); + else + throw new Error("either aUndoEntries or aRedoEntries should be true"); + }); + }, + + /** + * The numbers of entries in the transactions history. + */ + get length() TransactionsHistory.length, + + /** + * Get the transaction history entry at a given index. Each entry consists + * of one or more transaction objects. + * + * THIS METHOD SHOULD BE USED ONLY AS A WAY TO MONITOR THE TRANSACTIONS + * HISTORY STATE. NEVER CALL THE TRANSACTION METHODS (execute, undo, redo) + * DIRECTLY. + * + * @param aIndex + * the index of the entry to retrieve. + * @return an array of transaction objects in their undo order (that is, + * reversely to the order they were executed). + * @throw if aIndex is invalid (< 0 or >= length). + * @note the returned array is a clone of the history entry and is not + * kept in sync with the original entry if it changes. + */ + item: function (aIndex) { + if (!Number.isInteger(aIndex) || aIndex < 0 || aIndex >= this.length) + throw new Error("Invalid index"); + + return TransactionsHistory[aIndex]; + }, + + /** + * The index of the top undo entry in the transactions history. + * If there are no undo entries, it equals to |length|. + * Entries past this point + * Entries at and past this point are redo entries. + */ + get undoPosition() TransactionsHistory.undoPosition, +}; + +/** + * Internal helper for defining the standard transactions and their input. + * It takes the required and optional properties, and generates the public + * constructor (which takes the input in the form of a plain object) which, + * when called, creates the argument-less "public" |execute| method by binding + * the input properties to the function arguments (required properties first, + * then the optional properties). + * + * If this seems confusing, look at the consumers. + * + * This magic serves two purposes: + * (1) It completely hides the transactions' internals from the module + * consumers. + * (2) It keeps each transaction implementation to what is about, bypassing + * all this bureaucracy while still validating input appropriately. + */ +function DefineTransaction(aRequiredProps = [], aOptionalProps = []) { + for (let prop of [...aRequiredProps, ...aOptionalProps]) { + if (!DefineTransaction.inputProps.has(prop)) + throw new Error("Property '" + prop + "' is not defined"); + } + + let ctor = function (aInput) { + // We want to support both syntaxes: + // let t = new PlacesTransactions.NewBookmark(), + // let t = PlacesTransactions.NewBookmark() + if (this == PlacesTransactions) + return new ctor(aInput); + + if (aRequiredProps.length > 0 || aOptionalProps.length > 0) { + // Bind the input properties to the arguments of execute. + let input = DefineTransaction.verifyInput(aInput, aRequiredProps, + aOptionalProps); + let executeArgs = [this, + ...[input[prop] for (prop of aRequiredProps)], + ...[input[prop] for (prop of aOptionalProps)]]; + this.execute = Function.bind.apply(this.execute, executeArgs); + } + return this; + }; + return ctor; +} + +DefineTransaction.isStr = v => typeof(v) == "string"; +DefineTransaction.isURI = v => v instanceof Components.interfaces.nsIURI; +DefineTransaction.isIndex = v => Number.isInteger(v) && + v >= PlacesUtils.bookmarks.DEFAULT_INDEX; +DefineTransaction.isGUID = v => /^[a-zA-Z0-9\-_]{12}$/.test(v); +DefineTransaction.isPrimitive = v => v === null || (typeof(v) != "object" && + typeof(v) != "function"); +DefineTransaction.isAnnotationObject = function (obj) { + let checkProperty = (aPropName, aRequired, aCheckFunc) => { + if (aPropName in obj) + return aCheckFunc(obj[aPropName]); + + return !aRequired; + }; + + if (obj && + checkProperty("name", true, DefineTransaction.isStr) && + checkProperty("expires", false, Number.isInteger) && + checkProperty("flags", false, Number.isInteger) && + checkProperty("value", false, DefineTransaction.isPrimitive) ) { + // Nothing else should be set + let validKeys = ["name", "value", "flags", "expires"]; + if (Object.keys(obj).every( (k) => validKeys.indexOf(k) != -1 )) + return true; + } + return false; +}; + +DefineTransaction.inputProps = new Map(); +DefineTransaction.defineInputProps = +function (aNames, aValidationFunction, aDefaultValue) { + for (let name of aNames) { + this.inputProps.set(name, { + validate: aValidationFunction, + defaultValue: aDefaultValue, + isGUIDProp: false + }); + } +}; + +DefineTransaction.defineArrayInputProp = +function (aName, aValidationFunction, aDefaultValue) { + this.inputProps.set(aName, { + validate: (v) => Array.isArray(v) && v.every(aValidationFunction), + defaultValue: aDefaultValue, + isGUIDProp: false + }); +}; + +DefineTransaction.verifyPropertyValue = +function (aProp, aValue, aRequired) { + if (aValue === undefined) { + if (aRequired) + throw new Error("Required property is missing: " + aProp); + return this.inputProps.get(aProp).defaultValue; + } + + if (!this.inputProps.get(aProp).validate(aValue)) + throw new Error("Invalid value for property: " + aProp); + + if (Array.isArray(aValue)) { + // The original array cannot be referenced by this module because it would + // then implicitly reference its global as well. + return Components.utils.cloneInto(aValue, {}); + } + + return aValue; +}; + +DefineTransaction.verifyInput = +function (aInput, aRequired = [], aOptional = []) { + if (aRequired.length == 0 && aOptional.length == 0) + return {}; + + // If there's just a single required/optional property, we allow passing it + // as is, so, for example, one could do PlacesTransactions.RemoveItem(myGUID) + // rather than PlacesTransactions.RemoveItem({ GUID: myGUID}). + // This shortcut isn't supported for "complex" properties - e.g. one cannot + // pass an annotation object this way (note there is no use case for this at + // the moment anyway). + let isSinglePropertyInput = + this.isPrimitive(aInput) || + (aInput instanceof Components.interfaces.nsISupports); + let fixedInput = { }; + if (aRequired.length > 0) { + if (isSinglePropertyInput) { + if (aRequired.length == 1) { + let prop = aRequired[0], value = aInput; + value = this.verifyPropertyValue(prop, value, true); + fixedInput[prop] = value; + } + else { + throw new Error("Transaction input isn't an object"); + } + } + else { + for (let prop of aRequired) { + let value = this.verifyPropertyValue(prop, aInput[prop], true); + fixedInput[prop] = value; + } + } + } + + if (aOptional.length > 0) { + if (isSinglePropertyInput && !aRequired.length > 0) { + if (aOptional.length == 1) { + let prop = aOptional[0], value = aInput; + value = this.verifyPropertyValue(prop, value, true); + fixedInput[prop] = value; + } + else if (aInput !== null && aInput !== undefined) { + throw new Error("Transaction input isn't an object"); + } + } + else { + for (let prop of aOptional) { + let value = this.verifyPropertyValue(prop, aInput[prop], false); + if (value !== undefined) + fixedInput[prop] = value; + else + fixedInput[prop] = this.defaultValues[prop]; + } + } + } + + return fixedInput; +}; + +// Update the documentation at the top of this module if you add or +// remove properties. +DefineTransaction.defineInputProps(["uri", "feedURI", "siteURI"], + DefineTransaction.isURI, null); +DefineTransaction.defineInputProps(["GUID", "parentGUID", "newParentGUID"], + DefineTransaction.isGUID); +DefineTransaction.defineInputProps(["title", "keyword", "postData"], + DefineTransaction.isStr, ""); +DefineTransaction.defineInputProps(["index", "newIndex"], + DefineTransaction.isIndex, + PlacesUtils.bookmarks.DEFAULT_INDEX); +DefineTransaction.defineInputProps(["annotationObject"], + DefineTransaction.isAnnotationObject); +DefineTransaction.defineArrayInputProp("tags", + DefineTransaction.isStr, null); +DefineTransaction.defineArrayInputProp("annotations", + DefineTransaction.isAnnotationObject, + null); + +/** + * Internal helper for implementing the execute method of NewBookmark, NewFolder + * and NewSeparator. + * + * @param aTransaction + * The transaction object + * @param aParentGUID + * The guid of the parent folder + * @param aCreateItemFunction(aParentId, aGUIDToRestore) + * The function to be called for creating the item on execute and redo. + * It should return the itemId for the new item + * - aGUIDToRestore - the GUID to set for the item (used for redo). + * @param [optional] aOnUndo + * an additional function to call after undo + * @param [optional] aOnRedo + * an additional function to call after redo + */ +function* ExecuteCreateItem(aTransaction, aParentGUID, aCreateItemFunction, + aOnUndo = null, aOnRedo = null) { + let parentId = yield PlacesUtils.promiseItemId(aParentGUID), + itemId = yield aCreateItemFunction(parentId, ""), + guid = yield PlacesUtils.promiseItemGUID(itemId); + + // On redo, we'll restore the date-added and last-modified properties. + let dateAdded = 0, lastModified = 0; + aTransaction.undo = function* () { + if (dateAdded == 0) { + dateAdded = PlacesUtils.bookmarks.getItemDateAdded(itemId); + lastModified = PlacesUtils.bookmarks.getItemLastModified(itemId); + } + PlacesUtils.bookmarks.removeItem(itemId); + if (aOnUndo) { + yield aOnUndo(); + } + }; + aTransaction.redo = function* () { + parentId = yield PlacesUtils.promiseItemId(aParentGUID); + itemId = yield aCreateItemFunction(parentId, guid); + if (aOnRedo) + yield aOnRedo(); + + // aOnRedo is called first to make sure it doesn't override + // lastModified. + PlacesUtils.bookmarks.setItemDateAdded(itemId, dateAdded); + PlacesUtils.bookmarks.setItemLastModified(itemId, lastModified); + }; + return guid; +} + +/***************************************************************************** + * The Standard Places Transactions. + * + * See the documentation at the top of this file. The valid values for input + * are also documented there. + *****************************************************************************/ + +let PT = PlacesTransactions; + +/** + * Transaction for creating a bookmark. + * + * Required Input Properties: uri, parentGUID. + * Optional Input Properties: index, title, keyword, annotations, tags. + * + * When this transaction is executed, it's resolved to the new bookmark's GUID. + */ +PT.NewBookmark = DefineTransaction(["parentGUID", "uri"], + ["index", "title", "keyword", "postData", + "annotations", "tags"]); +PT.NewBookmark.prototype = Object.seal({ + execute: function (aParentGUID, aURI, aIndex, aTitle, + aKeyword, aPostData, aAnnos, aTags) { + return ExecuteCreateItem(this, aParentGUID, + function (parentId, guidToRestore = "") { + let itemId = PlacesUtils.bookmarks.insertBookmark( + parentId, aURI, aIndex, aTitle, guidToRestore); + if (aKeyword) + PlacesUtils.bookmarks.setKeywordForBookmark(itemId, aKeyword); + if (aPostData) + PlacesUtils.setPostDataForBookmark(itemId, aPostData); + if (aAnnos) + PlacesUtils.setAnnotationsForItem(itemId, aAnnos); + if (aTags && aTags.length > 0) { + let currentTags = PlacesUtils.tagging.getTagsForURI(aURI); + aTags = [t for (t of aTags) if (currentTags.indexOf(t) == -1)]; + PlacesUtils.tagging.tagURI(aURI, aTags); + } + + return itemId; + }, + function _additionalOnUndo() { + if (aTags && aTags.length > 0) + PlacesUtils.tagging.untagURI(aURI, aTags); + }); + } +}); + +/** + * Transaction for creating a folder. + * + * Required Input Properties: title, parentGUID. + * Optional Input Properties: index, annotations. + * + * When this transaction is executed, it's resolved to the new folder's GUID. + */ +PT.NewFolder = DefineTransaction(["parentGUID", "title"], + ["index", "annotations"]); +PT.NewFolder.prototype = Object.seal({ + execute: function (aParentGUID, aTitle, aIndex, aAnnos) { + return ExecuteCreateItem(this, aParentGUID, + function(parentId, guidToRestore = "") { + let itemId = PlacesUtils.bookmarks.createFolder( + parentId, aTitle, aIndex, guidToRestore); + if (aAnnos) + PlacesUtils.setAnnotationsForItem(itemId, aAnnos); + return itemId; + }); + } +}); + +/** + * Transaction for creating a separator. + * + * Required Input Properties: parentGUID. + * Optional Input Properties: index. + * + * When this transaction is executed, it's resolved to the new separator's + * GUID. + */ +PT.NewSeparator = DefineTransaction(["parentGUID"], ["index"]); +PT.NewSeparator.prototype = Object.seal({ + execute: function (aParentGUID, aIndex) { + return ExecuteCreateItem(this, aParentGUID, + function (parentId, guidToRestore = "") { + let itemId = PlacesUtils.bookmarks.insertSeparator( + parentId, aIndex, guidToRestore); + return itemId; + }); + } +}); + +/** + * Transaction for creating a live bookmark (see mozIAsyncLivemarks for the + * semantics). + * + * Required Input Properties: feedURI, title, parentGUID. + * Optional Input Properties: siteURI, index, annotations. + * + * When this transaction is executed, it's resolved to the new separators's + * GUID. + */ +PT.NewLivemark = DefineTransaction(["feedURI", "title", "parentGUID"], + ["siteURI", "index", "annotations"]); +PT.NewLivemark.prototype = Object.seal({ + execute: function* (aFeedURI, aTitle, aParentGUID, aSiteURI, aIndex, aAnnos) { + let createItem = function* (aGUID = "") { + let parentId = yield PlacesUtils.promiseItemId(aParentGUID); + let livemarkInfo = { + title: aTitle + , feedURI: aFeedURI + , parentId: parentId + , index: aIndex + , siteURI: aSiteURI }; + if (aGUID) + livemarkInfo.guid = aGUID; + + let livemark = yield PlacesUtils.livemarks.addLivemark(livemarkInfo); + if (aAnnos) + PlacesUtils.setAnnotationsForItem(livemark.id, aAnnos); + + return livemark; + }; + + let guid = (yield createItem()).guid; + this.undo = function* () { + yield PlacesUtils.livemarks.removeLivemark({ guid: guid }); + }; + this.redo = function* () { + yield createItem(guid); + }; + return guid; + } +}); + +/** + * Transaction for moving an item. + * + * Required Input Properties: GUID, newParentGUID, newIndex. + */ +PT.MoveItem = DefineTransaction(["GUID", "newParentGUID", "newIndex"]); +PT.MoveItem.prototype = Object.seal({ + execute: function* (aGUID, aNewParentGUID, aNewIndex) { + let itemId = yield PlacesUtils.promiseItemId(aGUID), + oldParentId = PlacesUtils.bookmarks.getFolderIdForItem(itemId), + oldIndex = PlacesUtils.bookmarks.getItemIndex(itemId), + newParentId = yield PlacesUtils.promiseItemId(aNewParentGUID); + + PlacesUtils.bookmarks.moveItem(itemId, newParentId, aNewIndex); + + let undoIndex = PlacesUtils.bookmarks.getItemIndex(itemId); + this.undo = () => { + // Moving down in the same parent takes in count removal of the item + // so to revert positions we must move to oldIndex + 1 + if (newParentId == oldParentId && oldIndex > undoIndex) + PlacesUtils.bookmarks.moveItem(itemId, oldParentId, oldIndex + 1); + else + PlacesUtils.bookmarks.moveItem(itemId, oldParentId, oldIndex); + }; + } +}); + +/** + * Transaction for setting the title for an item. + * + * Required Input Properties: GUID, title. + */ +PT.EditTitle = DefineTransaction(["GUID", "title"]); +PT.EditTitle.prototype = Object.seal({ + execute: function* (aGUID, aTitle) { + let itemId = yield PlacesUtils.promiseItemId(aGUID), + oldTitle = PlacesUtils.bookmarks.getItemTitle(itemId); + PlacesUtils.bookmarks.setItemTitle(itemId, aTitle); + this.undo = () => { PlacesUtils.bookmarks.setItemTitle(itemId, oldTitle); }; + } +}); + +/** + * Transaction for setting the URI for an item. + * + * Required Input Properties: GUID, uri. + */ +PT.EditURI = DefineTransaction(["GUID", "uri"]); +PT.EditURI.prototype = Object.seal({ + execute: function* (aGUID, aURI) { + let itemId = yield PlacesUtils.promiseItemId(aGUID), + oldURI = PlacesUtils.bookmarks.getBookmarkURI(itemId), + oldURITags = PlacesUtils.tagging.getTagsForURI(oldURI), + newURIAdditionalTags = null; + PlacesUtils.bookmarks.changeBookmarkURI(itemId, aURI); + + // Move tags from old URI to new URI. + if (oldURITags.length > 0) { + // Only untag the old URI if this is the only bookmark. + if (PlacesUtils.getBookmarksForURI(oldURI, {}).length == 0) + PlacesUtils.tagging.untagURI(oldURI, oldURITags); + + let currentNewURITags = PlacesUtils.tagging.getTagsForURI(aURI); + newURIAdditionalTags = [t for (t of oldURITags) + if (currentNewURITags.indexOf(t) == -1)]; + if (newURIAdditionalTags) + PlacesUtils.tagging.tagURI(aURI, newURIAdditionalTags); + } + + this.undo = () => { + PlacesUtils.bookmarks.changeBookmarkURI(itemId, oldURI); + // Move tags from new URI to old URI. + if (oldURITags.length > 0) { + // Only untag the new URI if this is the only bookmark. + if (newURIAdditionalTags && newURIAdditionalTags.length > 0 && + PlacesUtils.getBookmarksForURI(aURI, {}).length == 0) { + PlacesUtils.tagging.untagURI(aURI, newURIAdditionalTags); + } + + PlacesUtils.tagging.tagURI(oldURI, oldURITags); + } + }; + } +}); + +/** + * Transaction for setting an annotation for an item. + * + * Required Input Properties: GUID, annotationObject + */ +PT.SetItemAnnotation = DefineTransaction(["GUID", "annotationObject"]); +PT.SetItemAnnotation.prototype = { + execute: function* (aGUID, aAnno) { + let itemId = yield PlacesUtils.promiseItemId(aGUID), oldAnno; + if (PlacesUtils.annotations.itemHasAnnotation(itemId, aAnno.name)) { + // Fill the old anno if it is set. + let flags = {}, expires = {}; + PlacesUtils.annotations.getItemAnnotationInfo(itemId, aAnno.name, flags, + expires, { }); + let value = PlacesUtils.annotations.getItemAnnotation(itemId, aAnno.name); + oldAnno = { name: aAnno.name, flags: flags.value, + value: value, expires: expires.value }; + } + else { + // An unset value removes the annoation. + oldAnno = { name: aAnno.name }; + } + + PlacesUtils.setAnnotationsForItem(itemId, [aAnno]); + this.undo = () => { PlacesUtils.setAnnotationsForItem(itemId, [oldAnno]); }; + } +}; + +/** + * Transaction for setting the keyword for a bookmark. + * + * Required Input Properties: GUID, keyword. + */ +PT.EditKeyword = DefineTransaction(["GUID", "keyword"]); +PT.EditKeyword.prototype = Object.seal({ + execute: function* (aGUID, aKeyword) { + let itemId = yield PlacesUtils.promiseItemId(aGUID), + oldKeyword = PlacesUtils.bookmarks.getKeywordForBookmark(itemId); + PlacesUtils.bookmarks.setKeywordForBookmark(itemId, aKeyword); + this.undo = () => { + PlacesUtils.bookmarks.setKeywordForBookmark(itemId, oldKeyword); + }; + } +}); + +/** + * Transaction for sorting a folder by name. + * + * Required Input Properties: GUID. + */ +PT.SortByName = DefineTransaction(["GUID"]); +PT.SortByName.prototype = { + execute: function* (aGUID) { + let itemId = yield PlacesUtils.promiseItemId(aGUID), + oldOrder = [], // [itemId] = old index + contents = PlacesUtils.getFolderContents(itemId, false, false).root, + count = contents.childCount; + + // Sort between separators. + let newOrder = [], // nodes, in the new order. + preSep = []; // Temporary array for sorting each group of nodes. + let sortingMethod = (a, b) => { + if (PlacesUtils.nodeIsContainer(a) && !PlacesUtils.nodeIsContainer(b)) + return -1; + if (!PlacesUtils.nodeIsContainer(a) && PlacesUtils.nodeIsContainer(b)) + return 1; + return a.title.localeCompare(b.title); + }; + + for (let i = 0; i < count; ++i) { + let node = contents.getChild(i); + oldOrder[node.itemId] = i; + if (PlacesUtils.nodeIsSeparator(node)) { + if (preSep.length > 0) { + preSep.sort(sortingMethod); + newOrder = newOrder.concat(preSep); + preSep.splice(0, preSep.length); + } + newOrder.push(node); + } + else + preSep.push(node); + } + contents.containerOpen = false; + + if (preSep.length > 0) { + preSep.sort(sortingMethod); + newOrder = newOrder.concat(preSep); + } + + // Set the nex indexes. + let callback = { + runBatched: function() { + for (let i = 0; i < newOrder.length; ++i) { + PlacesUtils.bookmarks.setItemIndex(newOrder[i].itemId, i); + } + } + }; + PlacesUtils.bookmarks.runInBatchMode(callback, null); + + this.undo = () => { + let callback = { + runBatched: function() { + for (let item in oldOrder) { + PlacesUtils.bookmarks.setItemIndex(item, oldOrder[item]); + } + } + }; + PlacesUtils.bookmarks.runInBatchMode(callback, null); + }; + } +}; + +/** + * Transaction for removing an item (any type). + * + * Required Input Properties: GUID. + */ +PT.RemoveItem = DefineTransaction(["GUID"]); +PT.RemoveItem.prototype = { + execute: function* (aGUID) { + const bms = PlacesUtils.bookmarks; + + let itemsToRestoreOnUndo = []; + function* saveItemRestoreData(aItem, aNode = null) { + if (!aItem || !aItem.GUID) + throw new Error("invalid item object"); + + let itemId = aNode ? + aNode.itemId : yield PlacesUtils.promiseItemId(aItem.GUID); + if (itemId == -1) + throw new Error("Unexpected non-bookmarks node"); + + aItem.itemType = function() { + if (aNode) { + switch (aNode.type) { + case aNode.RESULT_TYPE_SEPARATOR: + return bms.TYPE_SEPARATOR; + case aNode.RESULT_TYPE_URI: // regular bookmarks + case aNode.RESULT_TYPE_FOLDER_SHORTCUT: // place:folder= bookmarks + case aNode.RESULT_TYPE_QUERY: // smart bookmarks + return bms.TYPE_BOOKMARK; + case aNode.RESULT_TYPE_FOLDER: + return bms.TYPE_FOLDER; + default: + throw new Error("Unexpected node type"); + } + } + return bms.getItemType(itemId); + }(); + + let node = aNode; + if (!node && aItem.itemType == bms.TYPE_FOLDER) + node = PlacesUtils.getFolderContents(itemId).root; + + // dateAdded, lastModified and annotations apply to all types. + aItem.dateAdded = node ? node.dateAdded : bms.getItemDateAdded(itemId); + aItem.lastModified = node ? + node.lastModified : bms.getItemLastModified(itemId); + aItem.annotations = PlacesUtils.getAnnotationsForItem(itemId); + + // For the first-level item, we don't have the parent. + if (!aItem.parentGUID) { + let parentId = PlacesUtils.bookmarks.getFolderIdForItem(itemId); + aItem.parentGUID = yield PlacesUtils.promiseItemGUID(parentId); + // For the first-level item, we also need the index. + // Note: node.bookmarkIndex doesn't work for root nodes. + aItem.index = bms.getItemIndex(itemId); + } + + // Separators don't have titles. + if (aItem.itemType != bms.TYPE_SEPARATOR) { + aItem.title = node ? node.title : bms.getItemTitle(itemId); + + if (aItem.itemType == bms.TYPE_BOOKMARK) { + aItem.uri = + node ? NetUtil.newURI(node.uri) : bms.getBookmarkURI(itemId); + aItem.keyword = PlacesUtils.bookmarks.getKeywordForBookmark(itemId); + + // This may be the last bookmark (excluding the tag-items themselves) + // for the URI, so we need to preserve the tags. + let tags = PlacesUtils.tagging.getTagsForURI(aItem.uri);; + if (tags.length > 0) + aItem.tags = tags; + } + else { // folder + // We always have the node for folders + aItem.readOnly = node.childrenReadOnly; + for (let i = 0; i < node.childCount; i++) { + let childNode = node.getChild(i); + let childItem = + { GUID: yield PlacesUtils.promiseItemGUID(childNode.itemId) + , parentGUID: aItem.GUID }; + itemsToRestoreOnUndo.push(childItem); + yield saveItemRestoreData(childItem, childNode); + } + node.containerOpen = false; + } + } + } + + let item = { GUID: aGUID, parentGUID: null }; + itemsToRestoreOnUndo.push(item); + yield saveItemRestoreData(item); + + let itemId = yield PlacesUtils.promiseItemId(aGUID); + PlacesUtils.bookmarks.removeItem(itemId); + this.undo = function() { + for (let item of itemsToRestoreOnUndo) { + let parentId = yield PlacesUtils.promiseItemId(item.parentGUID); + let index = "index" in item ? + index : PlacesUtils.bookmarks.DEFAULT_INDEX; + let itemId; + if (item.itemType == bms.TYPE_SEPARATOR) { + itemId = bms.insertSeparator(parentId, index, item.GUID); + } + else if (item.itemType == bms.TYPE_BOOKMARK) { + itemId = bms.insertBookmark(parentId, item.uri, index, item.title, + item.GUID); + } + else { // folder + itemId = bms.createFolder(parentId, item.title, index, item.GUID); + } + + if (item.itemType == bms.TYPE_BOOKMARK) { + if (item.keyword) + bms.setKeywordForBookmark(itemId, item.keyword); + if ("tags" in item) + PlacesUtils.tagging.tagURI(item.uri, item.tags); + } + else if (item.readOnly === true) { + bms.setFolderReadonly(itemId, true); + } + + PlacesUtils.setAnnotationsForItem(itemId, item.annotations); + PlacesUtils.bookmarks.setItemDateAdded(itemId, item.dateAdded); + PlacesUtils.bookmarks.setItemLastModified(itemId, item.lastModified); + } + }; + } +}; + +/** + * Transaction for tagging a URI. + * + * Required Input Properties: uri, tags. + */ +PT.TagURI = DefineTransaction(["uri", "tags"]); +PT.TagURI.prototype = { + execute: function* (aURI, aTags) { + if (PlacesUtils.getMostRecentBookmarkForURI(aURI) == -1) { + // Tagging is only allowed for bookmarked URIs. + let unfileGUID = + yield PlacesUtils.promiseItemGUID(PlacesUtils.unfiledBookmarksFolderId); + let createTxn = PT.NewBookmark({ uri: aURI + , tags: aTags + , parentGUID: unfileGUID }); + yield createTxn.execute(); + this.undo = createTxn.undo.bind(createTxn); + this.redo = createTxn.redo.bind(createTxn); + } + else { + let currentTags = PlacesUtils.tagging.getTagsForURI(aURI); + let newTags = [t for (t of aTags) if (currentTags.indexOf(t) == -1)]; + PlacesUtils.tagging.tagURI(aURI, newTags); + this.undo = () => { PlacesUtils.tagging.untagURI(aURI, newTags); }; + this.redo = () => { PlacesUtils.tagging.tagURI(aURI, newTags); }; + } + } +}; + +/** + * Transaction for removing tags from a URI. + * + * Required Input Properties: uri. + * Optional Input Properties: tags. + * + * If |tags| is not set, all tags set for |uri| are removed. + */ +PT.UntagURI = DefineTransaction(["uri"], ["tags"]); +PT.UntagURI.prototype = { + execute: function* (aURI, aTags) { + let tagsSet = PlacesUtils.tagging.getTagsForURI(aURI); + + if (aTags && aTags.length > 0) + aTags = [t for (t of aTags) if (tagsSet.indexOf(t) != -1)]; + else + aTags = tagsSet; + + PlacesUtils.tagging.untagURI(aURI, aTags); + this.undo = () => { PlacesUtils.tagging.tagURI(aURI, aTags); }; + this.redo = () => { PlacesUtils.tagging.untagURI(aURI, aTags); }; + } +}; diff --git a/toolkit/components/places/PlacesUtils.jsm b/toolkit/components/places/PlacesUtils.jsm index b8055135b67..5124f9aa741 100644 --- a/toolkit/components/places/PlacesUtils.jsm +++ b/toolkit/components/places/PlacesUtils.jsm @@ -1523,7 +1523,31 @@ this.PlacesUtils = { } }); return deferred.promise; - } + }, + + /** + * Get the unique id for an item (a bookmark, a folder or a separator) given + * its item id. + * + * @param aItemId + * an item id + * @return {Promise} + * @resolves to the GUID. + * @rejects if aItemId is invalid. + */ + promiseItemGUID: function (aItemId) GUIDHelper.getItemGUID(aItemId), + + /** + * Get the item id for an item (a bookmark, a folder or a separator) given + * its unique id. + * + * @param aGUID + * an item GUID + * @retrun {Promise} + * @resolves to the GUID. + * @rejects if there's no item for the given GUID. + */ + promiseItemId: function (aGUID) GUIDHelper.getItemId(aGUID) }; /** @@ -1640,6 +1664,138 @@ XPCOMUtils.defineLazyServiceGetter(this, "focusManager", "@mozilla.org/focus-manager;1", "nsIFocusManager"); +// Sometime soon, likely as part of the transition to mozIAsyncBookmarks, +// itemIds will be deprecated in favour of GUIDs, which play much better +// with multiple undo/redo operations. Because these GUIDs are already stored, +// and because we don't want to revise the transactions API once more when this +// happens, transactions are set to work with GUIDs exclusively, in the sense +// that they may never expose itemIds, nor do they accept them as input. +// More importantly, transactions which add or remove items guarantee to +// restore the guids on undo/redo, so that the following transactions that may +// done or undo can assume the items they're interested in are stil accessible +// through the same GUID. +// The current bookmarks API, however, doesn't expose the necessary means for +// working with GUIDs. So, until it does, this helper object accesses the +// Places database directly in order to switch between GUIDs and itemIds, and +// "restore" GUIDs on items re-created items. +const REASON_FINISHED = Ci.mozIStorageStatementCallback.REASON_FINISHED; +let GUIDHelper = { + // Cache for guid<->itemId paris. + GUIDsForIds: new Map(), + idsForGUIDs: new Map(), + + getItemId: function (aGUID) { + if (this.idsForGUIDs.has(aGUID)) + return Promise.resolve(this.idsForGUIDs.get(aGUID)); + + let deferred = Promise.defer(); + let itemId = -1; + + this._getIDStatement.params.guid = aGUID; + this._getIDStatement.executeAsync({ + handleResult: function (aResultSet) { + let row = aResultSet.getNextRow(); + if (row) + itemId = row.getResultByIndex(0); + }, + handleCompletion: function (aReason) { + if (aReason == REASON_FINISHED && itemId != -1) { + deferred.resolve(itemId); + + this.ensureObservingRemovedItems(); + this.idsForGUIDs.set(aGUID, itemId); + } + else if (itemId != -1) { + deferred.reject("no item found for the given guid"); + } + else { + deferred.reject("SQLite Error: " + aReason); + } + } + }); + + return deferred.promise; + }, + + getItemGUID: function (aItemId) { + if (this.GUIDsForIds.has(aItemId)) + return Promise.resolve(this.GUIDsForIds.has(aItemId)); + + let deferred = Promise.defer(); + let guid = ""; + + this._getGUIDStatement.params.id = aItemId; + this._getGUIDStatement.executeAsync({ + handleResult: function (aResultSet) { + let row = aResultSet.getNextRow(); + if (row) { + guid = row.getResultByIndex(1); + } + }, + handleCompletion: function (aReason) { + if (aReason == REASON_FINISHED && guid) { + deferred.resolve(guid); + + this.ensureObservingRemovedItems(); + this.GUIDsForIds.set(aItemId, guid); + } + else if (!guid) { + deferred.reject("no item found for the given itemId"); + } + else { + deferred.reject("SQLite Error: " + aReason); + } + } + }); + + return deferred.promise; + }, + + ensureObservingRemovedItems: function () { + if (!("observer" in this)) { + /** + * This observers serves two purposes: + * (1) Invalidate cached id<->guid paris on when items are removed. + * (2) Cache GUIDs given us free of charge by onItemAdded/onItemRemoved. + * So, for exmaple, when the NewBookmark needs the new GUID, we already + * have it cached. + */ + this.observer = { + onItemAdded: (aItemId, aParentId, aIndex, aItemType, aURI, aTitle, + aDateAdded, aGUID, aParentGUID) => { + this.GUIDsForIds.set(aItemId, aGUID); + this.GUIDsForIds.set(aParentId, aParentGUID); + }, + onItemRemoved: + (aItemId, aParentId, aIndex, aItemTyep, aURI, aGUID, aParentGUID) => { + this.GUIDsForIds.delete(aItemId); + this.idsForGUIDs.delete(aGUID); + this.GUIDsForIds.set(aParentId, aParentGUID); + }, + + QueryInterface: XPCOMUtils.generateQI(Ci.nsINavBookmarkObserver), + __noSuchMethod__: () => {}, // Catch all all onItem* methods. + }; + PlacesUtils.bookmarks.addObserver(this.observer, false); + PlacesUtils.registerShutdownFunction(() => { + PlacesUtils.bookmarks.removeObserver(this.observer); + }); + } + } +}; +XPCOMUtils.defineLazyGetter(GUIDHelper, "_getIDStatement", () => { + let s = PlacesUtils.history.DBConnection.createAsyncStatement( + "SELECT b.id, b.guid from moz_bookmarks b WHERE b.guid = :guid"); + PlacesUtils.registerShutdownFunction( () => s.finalize() ); + return s; +}); +XPCOMUtils.defineLazyGetter(GUIDHelper, "_getGUIDStatement", () => { + let s = PlacesUtils.history.DBConnection.createAsyncStatement( + "SELECT b.id, b.guid from moz_bookmarks b WHERE b.id = :id"); + PlacesUtils.registerShutdownFunction( () => s.finalize() ); + return s; +}); + //////////////////////////////////////////////////////////////////////////////// //// Transactions handlers. @@ -2039,29 +2195,23 @@ PlacesCreateLivemarkTransaction.prototype = { , parentId: this.item.parentId , index: this.item.index , siteURI: this.item.siteURI - }, - (function(aStatus, aLivemark) { - if (Components.isSuccessCode(aStatus)) { - this.item.id = aLivemark.id; - if (this.item.annotations && this.item.annotations.length > 0) { - PlacesUtils.setAnnotationsForItem(this.item.id, - this.item.annotations); - } + }).then(aLivemark => { + this.item.id = aLivemark.id; + if (this.item.annotations && this.item.annotations.length > 0) { + PlacesUtils.setAnnotationsForItem(this.item.id, + this.item.annotations); } - }).bind(this) - ); + }, Cu.reportError); }, undoTransaction: function CLTXN_undoTransaction() { // The getLivemark callback is expected to receive a failure status but it // is used just to serialize, so doesn't matter. - PlacesUtils.livemarks.getLivemark( - { id: this.item.id }, - (function (aStatus, aLivemark) { + PlacesUtils.livemarks.getLivemark({ id: this.item.id }) + .then(null, () => { PlacesUtils.bookmarks.removeItem(this.item.id); - }).bind(this) - ); + }); } }; @@ -2099,17 +2249,12 @@ PlacesRemoveLivemarkTransaction.prototype = { doTransaction: function RLTXN_doTransaction() { - PlacesUtils.livemarks.getLivemark( - { id: this.item.id }, - (function (aStatus, aLivemark) { - if (Components.isSuccessCode(aStatus)) { - this.item.feedURI = aLivemark.feedURI; - this.item.siteURI = aLivemark.siteURI; - - PlacesUtils.bookmarks.removeItem(this.item.id); - } - }).bind(this) - ); + PlacesUtils.livemarks.getLivemark({ id: this.item.id }) + .then(aLivemark => { + this.item.feedURI = aLivemark.feedURI; + this.item.siteURI = aLivemark.siteURI; + PlacesUtils.bookmarks.removeItem(this.item.id); + }, Cu.reportError); }, undoTransaction: function RLTXN_undoTransaction() @@ -2118,26 +2263,21 @@ PlacesRemoveLivemarkTransaction.prototype = { // feedURI and siteURI of the livemark. // The getLivemark callback is expected to receive a failure status but it // is used just to serialize, so doesn't matter. - PlacesUtils.livemarks.getLivemark( - { id: this.item.id }, - (function () { - let addLivemarkCallback = (function(aStatus, aLivemark) { - if (Components.isSuccessCode(aStatus)) { - let itemId = aLivemark.id; - PlacesUtils.bookmarks.setItemDateAdded(itemId, this.item.dateAdded); - PlacesUtils.setAnnotationsForItem(itemId, this.item.annotations); - } - }).bind(this); + PlacesUtils.livemarks.getLivemark({ id: this.item.id }) + .then(null, () => { PlacesUtils.livemarks.addLivemark({ parentId: this.item.parentId , title: this.item.title , siteURI: this.item.siteURI , feedURI: this.item.feedURI , index: this.item.index , lastModified: this.item.lastModified - }, - addLivemarkCallback); - }).bind(this) - ); + }).then( + aLivemark => { + let itemId = aLivemark.id; + PlacesUtils.bookmarks.setItemDateAdded(itemId, this.item.dateAdded); + PlacesUtils.setAnnotationsForItem(itemId, this.item.annotations); + }, Cu.reportError); + }); } }; diff --git a/toolkit/components/places/moz.build b/toolkit/components/places/moz.build index a6170dec23a..b8fe52dc29f 100644 --- a/toolkit/components/places/moz.build +++ b/toolkit/components/places/moz.build @@ -67,6 +67,7 @@ if CONFIG['MOZ_PLACES']: 'ColorConversion.js', 'PlacesBackups.jsm', 'PlacesDBUtils.jsm', + 'PlacesTransactions.jsm', ] EXTRA_PP_JS_MODULES += [ diff --git a/toolkit/components/places/mozIAsyncLivemarks.idl b/toolkit/components/places/mozIAsyncLivemarks.idl index 637c511df15..3bcc1358d2a 100644 --- a/toolkit/components/places/mozIAsyncLivemarks.idl +++ b/toolkit/components/places/mozIAsyncLivemarks.idl @@ -27,6 +27,9 @@ interface mozIAsyncLivemarks : nsISupports * @return {Promise} * @throws NS_ERROR_INVALID_ARG if the supplied information is insufficient * for the creation. + * @deprecated passing a callback is deprecated. Moreover, for backwards + * compatibility reasons, when a callback is provided this method + * won't return a promise. */ jsval addLivemark(in jsval aLivemarkInfo, [optional] in mozILivemarkCallback aCallback); @@ -43,6 +46,9 @@ interface mozIAsyncLivemarks : nsISupports * * @return {Promise} * @throws NS_ERROR_INVALID_ARG if the id/guid is invalid. + * @deprecated passing a callback is deprecated. Moreover, for backwards + * compatibility reasons, when a callback is provided this method + * won't return a promise. */ jsval removeLivemark(in jsval aLivemarkInfo, [optional] in mozILivemarkCallback aCallback); @@ -60,6 +66,9 @@ interface mozIAsyncLivemarks : nsISupports * @return {Promise} * @throws NS_ERROR_INVALID_ARG if the id/guid is invalid or an invalid * callback is provided. + * @deprecated passing a callback is deprecated. Moreover, for backwards + * compatibility reasons, when a callback is provided this method + * won't return a promise. */ jsval getLivemark(in jsval aLivemarkInfo, [optional] in mozILivemarkCallback aCallback); diff --git a/toolkit/components/places/nsLivemarkService.js b/toolkit/components/places/nsLivemarkService.js index 6a57f96d76d..1009a0c915d 100644 --- a/toolkit/components/places/nsLivemarkService.js +++ b/toolkit/components/places/nsLivemarkService.js @@ -18,6 +18,8 @@ XPCOMUtils.defineLazyModuleGetter(this, "NetUtil", "resource://gre/modules/NetUtil.jsm"); XPCOMUtils.defineLazyModuleGetter(this, "Promise", "resource://gre/modules/Promise.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "Deprecated", + "resource://gre/modules/Deprecated.jsm"); //////////////////////////////////////////////////////////////////////////////// //// Services @@ -126,9 +128,9 @@ LivemarkService.prototype = { stmt.finalize(); }, - _onCacheReady: function LS__onCacheReady(aCallback, aWaitForAsyncWrites) + _onCacheReady: function LS__onCacheReady(aCallback) { - if (this._pendingStmt || aWaitForAsyncWrites) { + if (this._pendingStmt) { // The cache is still being populated, so enqueue the job to the Storage // async thread. Ideally this should just dispatch a runnable to it, // that would call back on the main thread, but bug 608142 made that @@ -212,6 +214,12 @@ LivemarkService.prototype = { throw Cr.NS_ERROR_INVALID_ARG; } + if (aLivemarkCallback) { + Deprecated.warning("Passing a callback to Livermarks methods is deprecated. " + + "Please use the returned promise instead.", + "https://developer.mozilla.org/docs/Mozilla/JavaScript_code_modules/Promise.jsm"); + } + // The addition is done synchronously due to the fact importExport service // and JSON backups require that. The notification is async though. // Once bookmarks are async, this may be properly fixed. @@ -235,9 +243,7 @@ LivemarkService.prototype = { }); if (this._itemAdded && this._itemAdded.id == livemark.id) { livemark.index = this._itemAdded.index; - if (!aLivemarkInfo.guid) { - livemark.guid = this._itemAdded.guid; - } + livemark.guid = this._itemAdded.guid; if (!aLivemarkInfo.lastModified) { livemark.lastModified = this._itemAdded.lastModified; } @@ -246,7 +252,7 @@ LivemarkService.prototype = { // Updating the cache even if it has not yet been populated doesn't // matter since it will just be overwritten. this._livemarks[livemark.id] = livemark; - this._guids[aLivemarkInfo.guid] = livemark.id; + this._guids[livemark.guid] = livemark.id; } catch (ex) { addLivemarkEx = ex; @@ -260,8 +266,9 @@ LivemarkService.prototype = { aLivemarkCallback.onCompletion(addLivemarkEx.result, livemark); } catch(ex2) { } + } else { + deferred.reject(addLivemarkEx); } - deferred.reject(addLivemarkEx); } else { if (aLivemarkCallback) { @@ -269,13 +276,14 @@ LivemarkService.prototype = { aLivemarkCallback.onCompletion(Cr.NS_OK, livemark); } catch(ex2) { } + } else { + deferred.resolve(livemark); } - deferred.resolve(livemark); } - }, true); + }); } - return deferred.promise; + return aLivemarkCallback ? null : deferred.promise; }, removeLivemark: function LS_removeLivemark(aLivemarkInfo, aLivemarkCallback) @@ -292,6 +300,12 @@ LivemarkService.prototype = { throw Cr.NS_ERROR_INVALID_ARG; } + if (aLivemarkCallback) { + Deprecated.warning("Passing a callback to Livermarks methods is deprecated. " + + "Please use the returned promise instead.", + "https://developer.mozilla.org/docs/Mozilla/JavaScript_code_modules/Promise.jsm"); + } + // Convert the guid to an id. if (id in this._guids) { id = this._guids[id]; @@ -316,8 +330,9 @@ LivemarkService.prototype = { aLivemarkCallback.onCompletion(removeLivemarkEx.result, null); } catch(ex2) { } + } else { + deferred.reject(removeLivemarkEx); } - deferred.reject(removeLivemarkEx); } else { if (aLivemarkCallback) { @@ -325,13 +340,14 @@ LivemarkService.prototype = { aLivemarkCallback.onCompletion(Cr.NS_OK, null); } catch(ex2) { } + } else { + deferred.resolve(); } - deferred.resolve(); } }); } - return deferred.promise; + return aLivemarkCallback ? null : deferred.promise; }, _reloaded: [], @@ -382,6 +398,12 @@ LivemarkService.prototype = { throw Cr.NS_ERROR_INVALID_ARG; } + if (aLivemarkCallback) { + Deprecated.warning("Passing a callback to Livermarks methods is deprecated. " + + "Please use the returned promise instead.", + "https://developer.mozilla.org/docs/Mozilla/JavaScript_code_modules/Promise.jsm"); + } + let deferred = Promise.defer(); this._onCacheReady( () => { // Convert the guid to an id. @@ -393,20 +415,22 @@ LivemarkService.prototype = { try { aLivemarkCallback.onCompletion(Cr.NS_OK, this._livemarks[id]); } catch (ex) {} + } else { + deferred.resolve(this._livemarks[id]); } - deferred.resolve(this._livemarks[id]); } else { if (aLivemarkCallback) { try { aLivemarkCallback.onCompletion(Cr.NS_ERROR_INVALID_ARG, null); } catch (ex) { } + } else { + deferred.reject(Components.Exception("", Cr.NS_ERROR_INVALID_ARG)); } - deferred.reject(Components.Exception("", Cr.NS_ERROR_INVALID_ARG)); } }); - return deferred.promise; + return aLivemarkCallback ? null : deferred.promise; }, ////////////////////////////////////////////////////////////////////////////// @@ -558,11 +582,9 @@ function Livemark(aLivemarkInfo) // Create a new livemark. this.id = PlacesUtils.bookmarks.createFolder(aLivemarkInfo.parentId, aLivemarkInfo.title, - aLivemarkInfo.index); + aLivemarkInfo.index, + aLivemarkInfo.guid); PlacesUtils.bookmarks.setFolderReadonly(this.id, true); - if (aLivemarkInfo.guid) { - this.writeGuid(aLivemarkInfo.guid); - } this.writeFeedURI(aLivemarkInfo.feedURI); if (aLivemarkInfo.siteURI) { this.writeSiteURI(aLivemarkInfo.siteURI); @@ -630,31 +652,6 @@ Livemark.prototype = { this.siteURI = aSiteURI; }, - writeGuid: function LM_writeGuid(aGUID) - { - // There isn't a way to create a bookmark with a given guid yet, nor to - // set a guid on an existing one. So, for now, just go the dirty way. - let db = PlacesUtils.history.QueryInterface(Ci.nsPIPlacesDatabase) - .DBConnection; - let stmt = db.createAsyncStatement("UPDATE moz_bookmarks " + - "SET guid = :guid " + - "WHERE id = :item_id"); - stmt.params.guid = aGUID; - stmt.params.item_id = this.id; - let livemark = this; - stmt.executeAsync({ - handleError: function () {}, - handleResult: function () {}, - handleCompletion: function ETAT_handleCompletion(aReason) - { - if (aReason == Ci.mozIStorageStatementCallback.REASON_FINISHED) { - livemark._guid = aGUID; - } - } - }); - stmt.finalize(); - }, - set guid(aGUID) { this._guid = aGUID; return aGUID; diff --git a/toolkit/components/places/tests/chrome/test_303567.xul b/toolkit/components/places/tests/chrome/test_303567.xul index ce50d656e3e..81d957e5e3d 100644 --- a/toolkit/components/places/tests/chrome/test_303567.xul +++ b/toolkit/components/places/tests/chrome/test_303567.xul @@ -55,10 +55,8 @@ function runTest() , index: PlacesUtils.bookmarks.DEFAULT_INDEX , feedURI: aLivemarkData.feedURI , siteURI: aLivemarkData.siteURI - }, - function (aStatus, aLivemark) { - ok(Components.isSuccessCode(aStatus), "Get livemark"); - + }) + .then(function (aLivemark) { is (aLivemark.feedURI.spec, aLivemarkData.feedURI.spec, "Get correct feedURI"); if (aLivemarkData.siteURI) { @@ -84,8 +82,10 @@ function runTest() SimpleTest.finish(); } }); + }, function () { + is(true, false, "Should not fail adding a livemark"); } - ) + ); } LIVEMARKS.forEach(testLivemark); diff --git a/toolkit/components/places/tests/chrome/test_341972a.xul b/toolkit/components/places/tests/chrome/test_341972a.xul index dbb59cd08a4..6c6b44c5c21 100644 --- a/toolkit/components/places/tests/chrome/test_341972a.xul +++ b/toolkit/components/places/tests/chrome/test_341972a.xul @@ -37,9 +37,8 @@ function runTest() { , index: PlacesUtils.bookmarks.DEFAULT_INDEX , feedURI: NetUtil.newURI(FEEDSPEC) , siteURI: NetUtil.newURI(INITIALSITESPEC) - }, - function (aStatus, aLivemark) { - ok(Components.isSuccessCode(aStatus), "Get livemark"); + }) + .then(function (aLivemark) { is(aLivemark.siteURI.spec, INITIALSITESPEC, "Has correct initial livemark site URI"); @@ -50,6 +49,8 @@ function runTest() { PlacesUtils.bookmarks.removeItem(aLivemark.id); SimpleTest.finish(); }); + }, function () { + is(true, false, "Should not fail adding a livemark"); } ); } diff --git a/toolkit/components/places/tests/chrome/test_341972b.xul b/toolkit/components/places/tests/chrome/test_341972b.xul index fb9893ac198..24f4587501e 100644 --- a/toolkit/components/places/tests/chrome/test_341972b.xul +++ b/toolkit/components/places/tests/chrome/test_341972b.xul @@ -35,9 +35,8 @@ function runTest() { , parentId: PlacesUtils.toolbarFolderId , index: PlacesUtils.bookmarks.DEFAULT_INDEX , feedURI: NetUtil.newURI(FEEDSPEC) - }, - function (aStatus, aLivemark) { - ok(Components.isSuccessCode(aStatus), "Get livemark"); + }) + .then(function (aLivemark) { is(aLivemark.siteURI, null, "Has null livemark site URI"); waitForLivemarkLoad(aLivemark, function (aLivemark) { @@ -47,6 +46,8 @@ function runTest() { PlacesUtils.bookmarks.removeItem(aLivemark.id); SimpleTest.finish(); }); + }, function () { + is(true, false, "Should not fail adding a livemark"); } ); } diff --git a/toolkit/components/places/tests/chrome/test_342484.xul b/toolkit/components/places/tests/chrome/test_342484.xul index d94c83b2827..e976e0166bd 100644 --- a/toolkit/components/places/tests/chrome/test_342484.xul +++ b/toolkit/components/places/tests/chrome/test_342484.xul @@ -36,10 +36,8 @@ function runTest() { , index: PlacesUtils.bookmarks.DEFAULT_INDEX , feedURI: NetUtil.newURI(FEEDSPEC) , siteURI: NetUtil.newURI("http:/mochi.test/") - }, - function (aStatus, aLivemark) { - ok(Components.isSuccessCode(aStatus), "Get livemark"); - + }) + .then(function (aLivemark) { waitForLivemarkLoad(aLivemark, function (aLivemark) { let nodes = aLivemark.getNodesForContainer({}); @@ -52,6 +50,8 @@ function runTest() { PlacesUtils.bookmarks.removeItem(aLivemark.id); SimpleTest.finish(); }); + }, function () { + is(true, false, "Should not fail adding a livemark"); } ); } diff --git a/toolkit/components/places/tests/chrome/test_381357.xul b/toolkit/components/places/tests/chrome/test_381357.xul index df09673f544..50fdcdf7642 100644 --- a/toolkit/components/places/tests/chrome/test_381357.xul +++ b/toolkit/components/places/tests/chrome/test_381357.xul @@ -35,13 +35,10 @@ function runTest() { , index: PlacesUtils.bookmarks.DEFAULT_INDEX , feedURI: NetUtil.newURI(FEEDSPEC) , siteURI: NetUtil.newURI("http:/mochi.test/") - }, - function (aStatus, aLivemark) { - ok(Components.isSuccessCode(aStatus), "Get livemark"); - + }) + .then(function (aLivemark) { waitForLivemarkLoad(aLivemark, function (aLivemark) { let nodes = aLivemark.getNodesForContainer({}); - ok(Components.isSuccessCode(aStatus), "Get livemark entries"); is(nodes[0].title, "The First Title", "livemark site URI set to value in feed"); @@ -49,6 +46,9 @@ function runTest() { PlacesUtils.bookmarks.removeItem(aLivemark.id); SimpleTest.finish(); }); + }, function () { + is(true, false, "Should not fail adding a livemark"); + SimpleTest.finish(); } ); } diff --git a/toolkit/components/places/tests/chrome/test_reloadLivemarks.xul b/toolkit/components/places/tests/chrome/test_reloadLivemarks.xul index 5047a4b1775..c67c99f3c0f 100644 --- a/toolkit/components/places/tests/chrome/test_reloadLivemarks.xul +++ b/toolkit/components/places/tests/chrome/test_reloadLivemarks.xul @@ -57,15 +57,18 @@ function addLivemarks(aCallback) { info("Adding livemarks"); let count = gLivemarks.length; gLivemarks.forEach(function(aLivemarkData) { - PlacesUtils.livemarks.addLivemark(aLivemarkData, - function (aStatus, aLivemark) { - ok(Components.isSuccessCode(aStatus), "Add livemark should succeed"); + PlacesUtils.livemarks.addLivemark(aLivemarkData) + .then(function (aLivemark) { + ok(aLivemark.feedURI.equals(aLivemarkData.feedURI), "Livemark added"); aLivemarkData.id = aLivemark.id; if (--count == 0) { aCallback(); } - } - ); + }, + function () { + is(true, false, "Should not fail adding a livemark."); + aCallback(); + }); }); } @@ -73,9 +76,9 @@ function reloadLivemarks(aForceUpdate, aCallback) { info("Reloading livemarks with forceUpdate: " + aForceUpdate); let count = gLivemarks.length; gLivemarks.forEach(function(aLivemarkData) { - PlacesUtils.livemarks.getLivemark(aLivemarkData, - function (aStatus, aLivemark) { - ok(Components.isSuccessCode(aStatus), "Get livemark should succeed"); + PlacesUtils.livemarks.getLivemark(aLivemarkData) + .then(aLivemark => { + ok(aLivemark.feedURI.equals(aLivemarkData.feedURI), "Livemark found"); aLivemarkData._observer = new resultObserver(aLivemark, function() { if (++count == gLivemarks.length) { aCallback(); @@ -84,6 +87,10 @@ function reloadLivemarks(aForceUpdate, aCallback) { if (--count == 0) { PlacesUtils.livemarks.reloadLivemarks(aForceUpdate); } + }, + function() { + is(true, false, "Should not fail getting a livemark."); + aCallback(); } ); }); @@ -93,12 +100,15 @@ function removeLivemarks(aCallback) { info("Removing livemarks"); let count = gLivemarks.length; gLivemarks.forEach(function(aLivemarkData) { - PlacesUtils.livemarks.removeLivemark(aLivemarkData, - function (aStatus, aLivemark) { - ok(Components.isSuccessCode(aStatus), "Remove livemark should succeed"); + PlacesUtils.livemarks.removeLivemark(aLivemarkData).then( + function (aLivemark) { if (--count == 0) { aCallback(); } + }, + function() { + is(true, false, "Should not fail adding a livemark."); + aCallback(); } ); }); diff --git a/toolkit/components/places/tests/head_common.js b/toolkit/components/places/tests/head_common.js index 6083488d60f..366b926b1b5 100644 --- a/toolkit/components/places/tests/head_common.js +++ b/toolkit/components/places/tests/head_common.js @@ -36,6 +36,8 @@ XPCOMUtils.defineLazyModuleGetter(this, "BookmarkJSONUtils", "resource://gre/modules/BookmarkJSONUtils.jsm"); XPCOMUtils.defineLazyModuleGetter(this, "PlacesBackups", "resource://gre/modules/PlacesBackups.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "PlacesTransactions", + "resource://gre/modules/PlacesTransactions.jsm"); XPCOMUtils.defineLazyModuleGetter(this, "OS", "resource://gre/modules/osfile.jsm"); diff --git a/toolkit/components/places/tests/queries/head_queries.js b/toolkit/components/places/tests/queries/head_queries.js index 05169548f72..6987dee5506 100644 --- a/toolkit/components/places/tests/queries/head_queries.js +++ b/toolkit/components/places/tests/queries/head_queries.js @@ -162,7 +162,7 @@ function task_populateDB(aArray) , index: qdata.index , feedURI: uri(qdata.feedURI) , siteURI: uri(qdata.uri) - }); + }).then(null, do_throw); } if (qdata.isBookmark) { diff --git a/toolkit/components/places/tests/unit/test_384370.js b/toolkit/components/places/tests/unit/test_384370.js index 61470728402..db9ca5a5985 100644 --- a/toolkit/components/places/tests/unit/test_384370.js +++ b/toolkit/components/places/tests/unit/test_384370.js @@ -113,7 +113,7 @@ function populate() { function validate() { yield testCanonicalBookmarks(); - testToolbarFolder(); + yield testToolbarFolder(); testUnfiledBookmarks(); testTags(); } @@ -217,16 +217,11 @@ function testToolbarFolder() { // title do_check_eq("Latest Headlines", livemark.title); - PlacesUtils.livemarks.getLivemark( - { id: livemark.itemId }, - function (aStatus, aLivemark) { - do_check_true(Components.isSuccessCode(aStatus)); - do_check_eq("http://en-us.fxfeeds.mozilla.com/en-US/firefox/livebookmarks/", - aLivemark.siteURI.spec); - do_check_eq("http://en-us.fxfeeds.mozilla.com/en-US/firefox/headlines.xml", - aLivemark.feedURI.spec); - } - ); + let foundLivemark = yield PlacesUtils.livemarks.getLivemark({ id: livemark.itemId }); + do_check_eq("http://en-us.fxfeeds.mozilla.com/en-US/firefox/livebookmarks/", + foundLivemark.siteURI.spec); + do_check_eq("http://en-us.fxfeeds.mozilla.com/en-US/firefox/headlines.xml", + foundLivemark.feedURI.spec); // test added bookmark data var child = toolbar.getChild(2); diff --git a/toolkit/components/places/tests/unit/test_async_transactions.js b/toolkit/components/places/tests/unit/test_async_transactions.js new file mode 100644 index 00000000000..d7e7703bee7 --- /dev/null +++ b/toolkit/components/places/tests/unit/test_async_transactions.js @@ -0,0 +1,1053 @@ +/* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim:set ts=2 sw=2 sts=2 et: */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +const bmsvc = PlacesUtils.bookmarks; +const tagssvc = PlacesUtils.tagging; +const annosvc = PlacesUtils.annotations; +const PT = PlacesTransactions; + +// Create and add bookmarks observer. +let observer = { + __proto__: NavBookmarkObserver.prototype, + + tagRelatedGUIDs: new Set(), + + reset: function () { + this.itemsAdded = new Map(); + this.itemsRemoved = new Map(); + this.itemsChanged = new Map(); + this.itemsMoved = new Map(); + this.beginUpdateBatch = false; + this.endUpdateBatch = false; + }, + + onBeginUpdateBatch: function () { + this.beginUpdateBatch = true; + }, + + onEndUpdateBatch: function () { + this.endUpdateBatch = true; + }, + + onItemAdded: + function (aItemId, aParentId, aIndex, aItemType, aURI, aTitle, aDateAdded, + aGUID, aParentGUID) { + // Ignore tag items. + if (aParentId == PlacesUtils.tagsFolderId || + (aParentId != PlacesUtils.placesRootId && + bmsvc.getFolderIdForItem(aParentId) == PlacesUtils.tagsFolderId)) { + this.tagRelatedGUIDs.add(aGUID); + return; + } + + this.itemsAdded.set(aGUID, { itemId: aItemId + , parentGUID: aParentGUID + , index: aIndex + , itemType: aItemType + , title: aTitle + , uri: aURI }); + }, + + onItemRemoved: + function (aItemId, aParentId, aIndex, aItemType, aURI, aGUID, aParentGUID) { + if (this.tagRelatedGUIDs.has(aGUID)) + return; + + this.itemsRemoved.set(aGUID, { parentGUID: aParentGUID + , index: aIndex + , itemType: aItemType }); + }, + + onItemChanged: + function (aItemId, aProperty, aIsAnnoProperty, aNewValue, aLastModified, + aItemType, aParentId, aGUID, aParentGUID) { + if (this.tagRelatedGUIDs.has(aGUID)) + return; + + let changesForGUID = this.itemsChanged.get(aGUID); + if (changesForGUID === undefined) { + changesForGUID = new Map(); + this.itemsChanged.set(aGUID, changesForGUID); + } + + let newValue = aNewValue; + if (aIsAnnoProperty) { + if (annosvc.itemHasAnnotation(aItemId, aProperty)) + newValue = annosvc.getItemAnnotation(aItemId, aProperty); + else + newValue = null; + } + let change = { isAnnoProperty: aIsAnnoProperty + , newValue: newValue + , lastModified: aLastModified + , itemType: aItemType }; + changesForGUID.set(aProperty, change); + }, + + onItemVisited: () => {}, + + onItemMoved: + function (aItemId, aOldParent, aOldIndex, aNewParent, aNewIndex, aItemType, + aGUID, aOldParentGUID, aNewParentGUID) { + this.itemsMoved.set(aGUID, { oldParentGUID: aOldParentGUID + , oldIndex: aOldIndex + , newParentGUID: aNewParentGUID + , newIndex: aNewIndex + , itemType: aItemType }); + } +}; +observer.reset(); + +// index at which items should begin +let bmStartIndex = 0; + +// get bookmarks root id +let root = PlacesUtils.bookmarksMenuFolderId; + +function run_test() { + bmsvc.addObserver(observer, false); + do_register_cleanup(function () { + bmsvc.removeObserver(observer); + }); + + run_next_test(); +} + +function ensureUndoState(aEntries = [], aUndoPosition = 0) { + do_check_eq(PT.length, aEntries.length); + do_check_eq(PT.undoPosition, aUndoPosition); + + for (let i = 0; i < aEntries.length; i++) { + let testEntry = aEntries[i]; + let undoEntry = PT.item(i); + do_check_eq(testEntry.length, undoEntry.length); + for (let j = 0; j < testEntry.length; j++) { + do_check_eq(testEntry[j], undoEntry[j]); + } + } +} + +function ensureItemsAdded(...items) { + do_check_eq(observer.itemsAdded.size, items.length); + for (let item of items) { + do_check_true(observer.itemsAdded.has(item.GUID)); + let info = observer.itemsAdded.get(item.GUID); + do_check_eq(info.parentGUID, item.parentGUID); + if ("title" in item) + do_check_eq(info.title, item.title); + if ("index" in item) + do_check_eq(info.index, item.index); + if ("itemType" in item) + do_check_eq(info.itemType, item.itemType); + } +} + +function ensureItemsRemoved(...items) { + do_check_eq(observer.itemsRemoved.size, items.length); + for (let item of items) { + do_check_true(observer.itemsRemoved.has(item.GUID)); + let info = observer.itemsRemoved.get(item.GUID); + do_check_eq(info.parentGUID, item.parentGUID); + if ("index" in item) + do_check_eq(info.index, item.index); + } +} + +function ensureItemsChanged(...items) { + for (let item of items) { + do_check_true(observer.itemsChanged.has(item.GUID)); + let changes = observer.itemsChanged.get(item.GUID); + do_check_true(changes.has(item.property)); + let info = changes.get(item.property); + do_check_eq(info.isAnnoProperty, Boolean(item.isAnnoProperty)); + do_check_eq(info.newValue, item.newValue); + if ("uri" in item) + do_check_true(item.uri.equals(info.uri)); + } +} + +function ensureAnnotationsSet(aGUID, aAnnos) { + do_check_true(observer.itemsChanged.has(aGUID)); + let changes = observer.itemsChanged.get(aGUID); + for (let anno of aAnnos) { + do_check_true(changes.has(anno.name)); + let changeInfo = changes.get(anno.name); + do_check_true(changeInfo.isAnnoProperty); + do_check_eq(changeInfo.newValue, anno.value); + } +} + +function ensureItemsMoved(...items) { + do_check_true(observer.itemsMoved.size, items.length); + for (let item of items) { + do_check_true(observer.itemsMoved.has(item.GUID)); + let info = observer.itemsMoved.get(item.GUID); + do_check_eq(info.oldParentGUID, item.oldParentGUID); + do_check_eq(info.oldIndex, item.oldIndex); + do_check_eq(info.newParentGUID, item.newParentGUID); + do_check_eq(info.newIndex, item.newIndex); + } +} + +function ensureTimestampsUpdated(aGUID, aCheckDateAdded = false) { + do_check_true(observer.itemsChanged.has(aGUID)); + let changes = observer.itemsChanged.get(aGUID); + if (aCheckDateAdded) + do_check_true(changes.has("dateAdded")) + do_check_true(changes.has("lastModified")); +} + +function ensureTagsForURI(aURI, aTags) { + let tagsSet = tagssvc.getTagsForURI(aURI); + do_check_eq(tagsSet.length, aTags.length); + do_check_true(aTags.every( t => tagsSet.indexOf(t) != -1 )); +} + +function* createTestFolderInfo(aTitle = "Test Folder") { + return { parentGUID: yield PlacesUtils.promiseItemGUID(root) + , title: "Test Folder" }; +} + +add_task(function* test_new_folder_with_annotation() { + const ANNO = { name: "TestAnno", value: "TestValue" }; + let folder_info = yield createTestFolderInfo(); + folder_info.index = bmStartIndex; + folder_info.annotations = [ANNO]; + ensureUndoState(); + let txn = PT.NewFolder(folder_info); + folder_info.GUID = yield PT.transact(txn); + let ensureDo = function* (aRedo = false) { + ensureUndoState([[txn]], 0); + yield ensureItemsAdded(folder_info); + ensureAnnotationsSet(folder_info.GUID, [ANNO]); + if (aRedo) + ensureTimestampsUpdated(folder_info.GUID, true); + observer.reset(); + }; + + let ensureUndo = () => { + ensureUndoState([[txn]], 1); + ensureItemsRemoved({ GUID: folder_info.GUID + , parentGUID: folder_info.parentGUID + , index: bmStartIndex }); + observer.reset(); + }; + + yield ensureDo(); + yield PT.undo(); + yield ensureUndo(); + yield PT.redo(); + yield ensureDo(true); + yield PT.undo(); + ensureUndo(); + yield PT.clearTransactionsHistory(); + ensureUndoState(); +}); + +add_task(function* test_new_bookmark() { + let bm_info = { parentGUID: yield PlacesUtils.promiseItemGUID(root) + , uri: NetUtil.newURI("http://test_create_item.com") + , index: bmStartIndex + , title: "Test creating an item" }; + + ensureUndoState(); + let txn = PT.NewBookmark(bm_info); + bm_info.GUID = yield PT.transact(txn); + + let ensureDo = function* (aRedo = false) { + ensureUndoState([[txn]], 0); + yield ensureItemsAdded(bm_info); + if (aRedo) + ensureTimestampsUpdated(bm_info.GUID, true); + observer.reset(); + }; + let ensureUndo = () => { + ensureUndoState([[txn]], 1); + ensureItemsRemoved({ GUID: bm_info.GUID + , parentGUID: bm_info.parentGUID + , index: bmStartIndex }); + observer.reset(); + }; + + yield ensureDo(); + yield PT.undo(); + ensureUndo(); + yield PT.redo(true); + yield ensureDo(); + yield PT.undo(); + ensureUndo(); + + yield PT.clearTransactionsHistory(); + ensureUndoState(); +}); + +add_task(function* test_merge_create_folder_and_item() { + let folder_info = yield createTestFolderInfo(); + let bm_info = { uri: NetUtil.newURI("http://test_create_item_to_folder.com") + , title: "Test Bookmark" + , index: bmStartIndex }; + + let { folderTxn, bkmTxn } = yield PT.transact( function* () { + let folderTxn = PT.NewFolder(folder_info); + folder_info.GUID = bm_info.parentGUID = yield folderTxn; + let bkmTxn = PT.NewBookmark(bm_info); + bm_info.GUID = yield bkmTxn;; + return { folderTxn: folderTxn, bkmTxn: bkmTxn}; + }); + + let ensureDo = function* () { + ensureUndoState([[bkmTxn, folderTxn]], 0); + yield ensureItemsAdded(folder_info, bm_info); + observer.reset(); + }; + + let ensureUndo = () => { + ensureUndoState([[bkmTxn, folderTxn]], 1); + ensureItemsRemoved(folder_info, bm_info); + observer.reset(); + }; + + yield ensureDo(); + yield PT.undo(); + ensureUndo(); + yield PT.redo(); + yield ensureDo(); + yield PT.undo(); + ensureUndo(); + + yield PT.clearTransactionsHistory(); + ensureUndoState(); +}); + +add_task(function* test_move_items_to_folder() { + let folder_a_info = yield createTestFolderInfo("Folder A"); + let bkm_a_info = { uri: NetUtil.newURI("http://test_move_items.com") + , title: "Bookmark A" }; + let bkm_b_info = { uri: NetUtil.newURI("http://test_move_items.com") + , title: "Bookmark B" }; + + // Test moving items within the same folder. + let [folder_a_txn, bkm_a_txn, bkm_b_txn] = yield PT.transact(function* () { + let folder_a_txn = PT.NewFolder(folder_a_info); + + folder_a_info.GUID = + bkm_a_info.parentGUID = bkm_b_info.parentGUID = yield folder_a_txn; + let bkm_a_txn = PT.NewBookmark(bkm_a_info); + bkm_a_info.GUID = yield bkm_a_txn; + let bkm_b_txn = PT.NewBookmark(bkm_b_info); + bkm_b_info.GUID = yield bkm_b_txn; + return [folder_a_txn, bkm_a_txn, bkm_b_txn]; + }); + + ensureUndoState([[bkm_b_txn, bkm_a_txn, folder_a_txn]], 0); + + let moveTxn = PT.MoveItem({ GUID: bkm_a_info.GUID + , newParentGUID: folder_a_info.GUID + , newIndex: bmsvc.DEFAULT_INDEX }); + yield PT.transact(moveTxn); + + let ensureDo = () => { + ensureUndoState([[moveTxn], [bkm_b_txn, bkm_a_txn, folder_a_txn]], 0); + ensureItemsMoved({ GUID: bkm_a_info.GUID + , oldParentGUID: folder_a_info.GUID + , newParentGUID: folder_a_info.GUID + , oldIndex: 0 + , newIndex: 1 }); + observer.reset(); + }; + let ensureUndo = () => { + ensureUndoState([[moveTxn], [bkm_b_txn, bkm_a_txn, folder_a_txn]], 1); + ensureItemsMoved({ GUID: bkm_a_info.GUID + , oldParentGUID: folder_a_info.GUID + , newParentGUID: folder_a_info.GUID + , oldIndex: 1 + , newIndex: 0 }); + observer.reset(); + }; + + ensureDo(); + yield PT.undo(); + ensureUndo(); + yield PT.redo(); + ensureDo(); + yield PT.undo(); + ensureUndo(); + + yield PT.clearTransactionsHistory(false, true); + ensureUndoState([[bkm_b_txn, bkm_a_txn, folder_a_txn]], 0); + + // Test moving items between folders. + let folder_b_info = yield createTestFolderInfo("Folder B"); + let folder_b_txn = PT.NewFolder(folder_b_info); + folder_b_info.GUID = yield PT.transact(folder_b_txn); + ensureUndoState([ [folder_b_txn] + , [bkm_b_txn, bkm_a_txn, folder_a_txn] ], 0); + + moveTxn = PT.MoveItem({ GUID: bkm_a_info.GUID + , newParentGUID: folder_b_info.GUID + , newIndex: bmsvc.DEFAULT_INDEX }); + yield PT.transact(moveTxn); + + ensureDo = () => { + ensureUndoState([ [moveTxn] + , [folder_b_txn] + , [bkm_b_txn, bkm_a_txn, folder_a_txn] ], 0); + ensureItemsMoved({ GUID: bkm_a_info.GUID + , oldParentGUID: folder_a_info.GUID + , newParentGUID: folder_b_info.GUID + , oldIndex: 0 + , newIndex: 0 }); + observer.reset(); + }; + let ensureUndo = () => { + ensureUndoState([ [moveTxn] + , [folder_b_txn] + , [bkm_b_txn, bkm_a_txn, folder_a_txn] ], 1); + ensureItemsMoved({ GUID: bkm_a_info.GUID + , oldParentGUID: folder_b_info.GUID + , newParentGUID: folder_a_info.GUID + , oldIndex: 0 + , newIndex: 0 }); + observer.reset(); + }; + + ensureDo(); + yield PT.undo(); + ensureUndo(); + yield PT.redo(); + ensureDo(); + yield PT.undo(); + ensureUndo(); + + // Clean up + yield PT.undo(); // folder_b_txn + yield PT.undo(); // folder_a_txn + the bookmarks; + do_check_eq(observer.itemsRemoved.size, 4); + ensureUndoState([ [moveTxn] + , [folder_b_txn] + , [bkm_b_txn, bkm_a_txn, folder_a_txn] ], 3); + yield PT.clearTransactionsHistory(); + ensureUndoState(); +}); + +add_task(function* test_remove_folder() { + let folder_level_1_info = yield createTestFolderInfo("Folder Level 1"); + let folder_level_2_info = { title: "Folder Level 2" }; + let [folder_level_1_txn, + folder_level_2_txn] = yield PT.transact(function* () { + let folder_level_1_txn = PT.NewFolder(folder_level_1_info); + folder_level_1_info.GUID = yield folder_level_1_txn; + folder_level_2_info.parentGUID = folder_level_1_info.GUID; + let folder_level_2_txn = PT.NewFolder(folder_level_2_info); + folder_level_2_info.GUID = yield folder_level_2_txn; + return [folder_level_1_txn, folder_level_2_txn]; + }); + + ensureUndoState([[folder_level_2_txn, folder_level_1_txn]]); + yield ensureItemsAdded(folder_level_1_info, folder_level_2_info); + observer.reset(); + + let remove_folder_2_txn = PT.RemoveItem(folder_level_2_info); + yield PT.transact(remove_folder_2_txn); + + ensureUndoState([ [remove_folder_2_txn] + , [folder_level_2_txn, folder_level_1_txn] ]); + yield ensureItemsRemoved(folder_level_2_info); + + // Undo RemoveItem "Folder Level 2" + yield PT.undo(); + ensureUndoState([ [remove_folder_2_txn] + , [folder_level_2_txn, folder_level_1_txn] ], 1); + yield ensureItemsAdded(folder_level_2_info); + ensureTimestampsUpdated(folder_level_2_info.GUID, true); + observer.reset(); + + // Redo RemoveItem "Folder Level 2" + yield PT.redo(); + ensureUndoState([ [remove_folder_2_txn] + , [folder_level_2_txn, folder_level_1_txn] ]); + yield ensureItemsRemoved(folder_level_2_info); + observer.reset(); + + // Undo it again + yield PT.undo(); + ensureUndoState([ [remove_folder_2_txn] + , [folder_level_2_txn, folder_level_1_txn] ], 1); + yield ensureItemsAdded(folder_level_2_info); + ensureTimestampsUpdated(folder_level_2_info.GUID, true); + observer.reset(); + + // Undo the creation of both folders + yield PT.undo(); + ensureUndoState([ [remove_folder_2_txn] + , [folder_level_2_txn, folder_level_1_txn] ], 2); + yield ensureItemsRemoved(folder_level_2_info, folder_level_1_info); + observer.reset(); + + // Redo the creation of both folders + yield PT.redo(); + ensureUndoState([ [remove_folder_2_txn] + , [folder_level_2_txn, folder_level_1_txn] ], 1); + yield ensureItemsAdded(folder_level_1_info, folder_level_2_info); + ensureTimestampsUpdated(folder_level_1_info.GUID, true); + ensureTimestampsUpdated(folder_level_2_info.GUID, true); + observer.reset(); + + // Redo RemoveItem "Folder Level 2" + yield PT.redo(); + ensureUndoState([ [remove_folder_2_txn] + , [folder_level_2_txn, folder_level_1_txn] ]); + yield ensureItemsRemoved(folder_level_2_info); + observer.reset(); + + // Undo everything one last time + yield PT.undo(); + ensureUndoState([ [remove_folder_2_txn] + , [folder_level_2_txn, folder_level_1_txn] ], 1); + yield ensureItemsAdded(folder_level_2_info); + observer.reset(); + + yield PT.undo(); + ensureUndoState([ [remove_folder_2_txn] + , [folder_level_2_txn, folder_level_1_txn] ], 2); + yield ensureItemsRemoved(folder_level_2_info, folder_level_2_info); + observer.reset(); + + yield PT.clearTransactionsHistory(); + ensureUndoState(); +}); + +add_task(function* test_add_and_remove_bookmarks_with_additional_info() { + const testURI = NetUtil.newURI("http://add.remove.tag") + , TAG_1 = "TestTag1", TAG_2 = "TestTag2" + , KEYWORD = "test_keyword" + , POST_DATA = "post_data" + , ANNO = { name: "TestAnno", value: "TestAnnoValue" }; + + let folder_info = yield createTestFolderInfo(); + folder_info.GUID = yield PT.transact(PT.NewFolder(folder_info)); + let ensureTags = ensureTagsForURI.bind(null, testURI); + + // Check that the NewBookmark transaction preserves tags. + observer.reset(); + let b1_info = { parentGUID: folder_info.GUID, uri: testURI, tags: [TAG_1] }; + b1_info.GUID = yield PT.transact(PT.NewBookmark(b1_info)); + ensureTags([TAG_1]); + yield PT.undo(); + ensureTags([]); + + observer.reset(); + yield PT.redo(); + ensureTimestampsUpdated(b1_info.GUID, true); + ensureTags([TAG_1]); + + // Check if the RemoveItem transaction removes and restores tags of children + // correctly. + yield PT.transact(PT.RemoveItem(folder_info.GUID)); + ensureTags([]); + + observer.reset(); + yield PT.undo(); + ensureTimestampsUpdated(b1_info.GUID, true); + ensureTags([TAG_1]); + + yield PT.redo(); + ensureTags([]); + + observer.reset(); + yield PT.undo(); + ensureTimestampsUpdated(b1_info.GUID, true); + ensureTags([TAG_1]); + + // * Check that no-op tagging (the uri is already tagged with TAG_1) is + // also a no-op on undo. + // * Test the "keyword" property of the NewBookmark transaction. + observer.reset(); + let b2_info = { parentGUID: folder_info.GUID + , uri: testURI, tags: [TAG_1, TAG_2] + , keyword: KEYWORD + , postData: POST_DATA + , annotations: [ANNO] }; + b2_info.GUID = yield PT.transact(PT.NewBookmark(b2_info)); + let b2_post_creation_changes = [ + { GUID: b2_info.GUID + , isAnnoProperty: true + , property: ANNO.name + , newValue: ANNO.value }, + { GUID: b2_info.GUID + , property: "keyword" + , newValue: KEYWORD }, + { GUID: b2_info.GUID + , isAnnoProperty: true + , property: PlacesUtils.POST_DATA_ANNO + , newValue: POST_DATA } ]; + ensureItemsChanged(...b2_post_creation_changes); + ensureTags([TAG_1, TAG_2]); + + observer.reset(); + yield PT.undo(); + yield ensureItemsRemoved(b2_info); + ensureTags([TAG_1]); + + // Check if RemoveItem correctly restores keywords, tags and annotations. + observer.reset(); + yield PT.redo(); + ensureItemsChanged(...b2_post_creation_changes); + ensureTags([TAG_1, TAG_2]); + + // Test RemoveItem for multiple items. + observer.reset(); + yield PT.transact(PT.RemoveItem(b1_info.GUID)); + yield PT.transact(PT.RemoveItem(b2_info.GUID)); + yield PT.transact(PT.RemoveItem(folder_info.GUID)); + yield ensureItemsRemoved(b1_info, b2_info, folder_info); + ensureTags([]); + + observer.reset(); + yield PT.undo(); + yield ensureItemsAdded(folder_info); + ensureTags([]); + + observer.reset(); + yield PT.undo(); + ensureItemsChanged(...b2_post_creation_changes); + ensureTags([TAG_1, TAG_2]); + + observer.reset(); + yield PT.undo(); + yield ensureItemsAdded(b1_info); + ensureTags([TAG_1, TAG_2]); + + // The redo calls below cleanup everything we did. + observer.reset(); + yield PT.redo(); + yield ensureItemsRemoved(b1_info); + ensureTags([TAG_1, TAG_2]); + + observer.reset(); + yield PT.redo(); + yield ensureItemsRemoved(b2_info); + ensureTags([]); + + observer.reset(); + yield PT.redo(); + yield ensureItemsRemoved(folder_info); + ensureTags([]); + + yield PT.clearTransactionsHistory(); + ensureUndoState(); +}); + +add_task(function* test_creating_and_removing_a_separator() { + let folder_info = yield createTestFolderInfo(); + let separator_info = {}; + let undoEntries = []; + + observer.reset(); + let create_txns = yield PT.transact(function* () { + let folder_txn = PT.NewFolder(folder_info); + folder_info.GUID = separator_info.parentGUID = yield folder_txn; + let separator_txn = PT.NewSeparator(separator_info); + separator_info.GUID = yield separator_txn; + return [separator_txn, folder_txn]; + }); + undoEntries.unshift(create_txns); + ensureUndoState(undoEntries, 0); + ensureItemsAdded(folder_info, separator_info); + + observer.reset(); + yield PT.undo(); + ensureUndoState(undoEntries, 1); + ensureItemsRemoved(folder_info, separator_info); + + observer.reset(); + yield PT.redo(); + ensureUndoState(undoEntries, 0); + ensureItemsAdded(folder_info, separator_info); + + observer.reset(); + let remove_sep_txn = PT.RemoveItem(separator_info); + yield PT.transact(remove_sep_txn); + undoEntries.unshift([remove_sep_txn]); + ensureUndoState(undoEntries, 0); + ensureItemsRemoved(separator_info); + + observer.reset(); + yield PT.undo(); + ensureUndoState(undoEntries, 1); + ensureItemsAdded(separator_info); + + observer.reset(); + yield PT.undo(); + ensureUndoState(undoEntries, 2); + ensureItemsRemoved(folder_info, separator_info); + + observer.reset(); + yield PT.redo(); + ensureUndoState(undoEntries, 1); + ensureItemsAdded(folder_info, separator_info); + + // Clear redo entries and check that |redo| does nothing + observer.reset(); + yield PT.clearTransactionsHistory(false, true); + undoEntries.shift(); + ensureUndoState(undoEntries, 0); + yield PT.redo(); + ensureItemsAdded(); + ensureItemsRemoved(); + + // Cleanup + observer.reset(); + yield PT.undo(); + ensureUndoState(undoEntries, 1); + ensureItemsRemoved(folder_info, separator_info); + yield PT.clearTransactionsHistory(); + ensureUndoState(); +}); + +add_task(function* test_edit_title() { + let bm_info = { parentGUID: yield PlacesUtils.promiseItemGUID(root) + , uri: NetUtil.newURI("http://test_create_item.com") + , title: "Original Title" }; + + function ensureTitleChange(aCurrentTitle) { + ensureItemsChanged({ GUID: bm_info.GUID + , property: "title" + , newValue: aCurrentTitle}); + } + + bm_info.GUID = yield PT.transact(PT.NewBookmark(bm_info)); + + observer.reset(); + yield PT.transact(PT.EditTitle({ GUID: bm_info.GUID, title: "New Title" })); + ensureTitleChange("New Title"); + + observer.reset(); + yield PT.undo(); + ensureTitleChange("Original Title"); + + observer.reset(); + yield PT.redo(); + ensureTitleChange("New Title"); + + // Cleanup + observer.reset(); + yield PT.undo(); + ensureTitleChange("Original Title"); + yield PT.undo(); + ensureItemsRemoved(bm_info); + + yield PT.clearTransactionsHistory(); + ensureUndoState(); +}); + +add_task(function* test_edit_url() { + let oldURI = NetUtil.newURI("http://old.test_editing_item_uri.com/"); + let newURI = NetUtil.newURI("http://new.test_editing_item_uri.com/"); + let bm_info = { parentGUID: yield PlacesUtils.promiseItemGUID(root) + , uri: oldURI + , tags: ["TestTag"]}; + + function ensureURIAndTags(aPreChangeURI, aPostChangeURI, aOLdURITagsPreserved) { + ensureItemsChanged({ GUID: bm_info.GUID + , property: "uri" + , newValue: aPostChangeURI.spec }); + ensureTagsForURI(aPostChangeURI, bm_info.tags); + ensureTagsForURI(aPreChangeURI, aOLdURITagsPreserved ? bm_info.tags : []); + } + + bm_info.GUID = yield PT.transact(PT.NewBookmark(bm_info)); + ensureTagsForURI(oldURI, bm_info.tags); + + // When there's a single bookmark for the same url, tags should be moved. + observer.reset(); + yield PT.transact(PT.EditURI({ GUID: bm_info.GUID, uri: newURI })); + ensureURIAndTags(oldURI, newURI, false); + + observer.reset(); + yield PT.undo(); + ensureURIAndTags(newURI, oldURI, false); + + observer.reset(); + yield PT.redo(); + ensureURIAndTags(oldURI, newURI, false); + + observer.reset(); + yield PT.undo(); + ensureURIAndTags(newURI, oldURI, false); + + // When there're multiple bookmarks for the same url, tags should be copied. + let bm2_info = Object.create(bm_info); + bm2_info.GUID = yield PT.transact(PT.NewBookmark(bm2_info)); + let bm3_info = Object.create(bm_info); + bm3_info.uri = newURI; + bm3_info.GUID = yield PT.transact(PT.NewBookmark(bm3_info)); + + observer.reset(); + yield PT.transact(PT.EditURI({ GUID: bm_info.GUID, uri: newURI })); + ensureURIAndTags(oldURI, newURI, true); + + observer.reset(); + yield PT.undo(); + ensureURIAndTags(newURI, oldURI, true); + + observer.reset(); + yield PT.redo(); + ensureURIAndTags(oldURI, newURI, true); + + // Cleanup + observer.reset(); + yield PT.undo(); + ensureURIAndTags(newURI, oldURI, true); + yield PT.undo(); + yield PT.undo(); + yield PT.undo(); + ensureItemsRemoved(bm3_info, bm2_info, bm_info); + + yield PT.clearTransactionsHistory(); + ensureUndoState(); +}); + +add_task(function* test_edit_keyword() { + let bm_info = { parentGUID: yield PlacesUtils.promiseItemGUID(root) + , uri: NetUtil.newURI("http://test.edit.keyword") }; + const KEYWORD = "test_keyword"; + bm_info.GUID = yield PT.transact(PT.NewBookmark(bm_info)); + function ensureKeywordChange(aCurrentKeyword = "") { + ensureItemsChanged({ GUID: bm_info.GUID + , property: "keyword" + , newValue: aCurrentKeyword }); + } + + bm_info.GUID = yield PT.transact(PT.NewBookmark(bm_info)); + + observer.reset(); + yield PT.transact(PT.EditKeyword({ GUID: bm_info.GUID, keyword: KEYWORD })); + ensureKeywordChange(KEYWORD); + + observer.reset(); + yield PT.undo(); + ensureKeywordChange(); + + observer.reset(); + yield PT.redo(); + ensureKeywordChange(KEYWORD); + + // Cleanup + observer.reset(); + yield PT.undo(); + ensureKeywordChange(); + yield PT.undo(); + ensureItemsRemoved(bm_info); + + yield PT.clearTransactionsHistory(); + ensureUndoState(); +}); + +add_task(function* test_tag_uri_unbookmarked_uri() { + let info = { uri: NetUtil.newURI("http://un.book.marked"), tags: ["MyTag"] }; + + function ensureDo() { + // A new bookmark should be created. + // (getMostRecentBookmarkForURI ignores tags) + do_check_neq(PlacesUtils.getMostRecentBookmarkForURI(info.uri), -1); + ensureTagsForURI(info.uri, info.tags); + } + function ensureUndo() { + do_check_eq(PlacesUtils.getMostRecentBookmarkForURI(info.uri), -1); + ensureTagsForURI(info.uri, []); + } + + yield PT.transact(PT.TagURI(info)); + ensureDo(); + yield PT.undo(); + ensureUndo(); + yield PT.redo(); + ensureDo(); + yield PT.undo(); + ensureUndo(); +}); + +add_task(function* test_tag_uri_bookmarked_uri() { + let bm_info = { uri: NetUtil.newURI("http://bookmarked.uri") + , parentGUID: yield PlacesUtils.promiseItemGUID(root) }; + bm_info.GUID = yield PT.transact(PT.NewBookmark(bm_info)); + + let tagging_info = { uri: bm_info.uri, tags: ["MyTag"] }; + yield PT.transact(PT.TagURI(tagging_info)); + ensureTagsForURI(tagging_info.uri, tagging_info.tags); + + yield PT.undo(); + ensureTagsForURI(tagging_info.uri, []); + yield PT.redo(); + ensureTagsForURI(tagging_info.uri, tagging_info.tags); + + // Cleanup + yield PT.undo(); + ensureTagsForURI(tagging_info.uri, []); + observer.reset(); + yield PT.undo(); + ensureItemsRemoved(bm_info); + + yield PT.clearTransactionsHistory(); + ensureUndoState(); +}); + +add_task(function* test_untag_uri() { + let bm_info = { uri: NetUtil.newURI("http://test.untag.uri") + , parentGUID: yield PlacesUtils.promiseItemGUID(root) + , tags: ["T"]}; + bm_info.GUID = yield PT.transact(PT.NewBookmark(bm_info)); + + yield PT.transact(PT.UntagURI(bm_info)); + ensureTagsForURI(bm_info.uri, []); + yield PT.undo(); + ensureTagsForURI(bm_info.uri, bm_info.tags); + yield PT.redo(); + ensureTagsForURI(bm_info.uri, []); + yield PT.undo(); + ensureTagsForURI(bm_info.uri, bm_info.tags); + + // Also test just passing the uri (should remove all tags) + yield PT.transact(PT.UntagURI(bm_info.uri)); + ensureTagsForURI(bm_info.uri, []); + yield PT.undo(); + ensureTagsForURI(bm_info.uri, bm_info.tags); + yield PT.redo(); + ensureTagsForURI(bm_info.uri, []); +}); + +add_task(function* test_set_item_annotation() { + let bm_info = { uri: NetUtil.newURI("http://test.item.annotation") + , parentGUID: yield PlacesUtils.promiseItemGUID(root) }; + let anno_info = { name: "TestAnno", value: "TestValue" }; + function ensureAnnoState(aSet) { + ensureAnnotationsSet(bm_info.GUID, + [{ name: anno_info.name + , value: aSet ? anno_info.value : null }]); + } + + bm_info.GUID = yield PT.transact(PT.NewBookmark(bm_info)); + + observer.reset(); + yield PT.transact(PT.SetItemAnnotation({ GUID: bm_info.GUID + , annotationObject: anno_info })); + ensureAnnoState(true); + + observer.reset(); + yield PT.undo(); + ensureAnnoState(false); + + observer.reset(); + yield PT.redo(); + ensureAnnoState(true); + + // Test removing the annotation by not passing the |value| property. + observer.reset(); + yield PT.transact( + PT.SetItemAnnotation({ GUID: bm_info.GUID + , annotationObject: { name: anno_info.name }})); + ensureAnnoState(false); + + observer.reset(); + yield PT.undo(); + ensureAnnoState(true); + + observer.reset(); + yield PT.redo(); + ensureAnnoState(false); +}); + +add_task(function* test_sort_folder_by_name() { + let folder_info = yield createTestFolderInfo(); + + let uri = NetUtil.newURI("http://sort.by.name/"); + let preSep = [{ title: i, uri: uri } for (i of ["3","2","1"])]; + let sep = {}; + let postSep = [{ title: l, uri: uri } for (l of ["c","b","a"])]; + let originalOrder = [...preSep, sep, ...postSep]; + let sortedOrder = [...preSep.slice(0).reverse(), + sep, + ...postSep.slice(0).reverse()]; + yield PT.transact(function* () { + folder_info.GUID = yield PT.NewFolder(folder_info); + for (let info of originalOrder) { + info.parentGUID = folder_info.GUID; + info.GUID = yield info == sep ? + PT.NewSeparator(info) : PT.NewBookmark(info); + } + }); + + let folderId = yield PlacesUtils.promiseItemId(folder_info.GUID); + let folderContainer = PlacesUtils.getFolderContents(folderId).root; + function ensureOrder(aOrder) { + for (let i = 0; i < folderContainer.childCount; i++) { + do_check_eq(folderContainer.getChild(i).bookmarkGuid, aOrder[i].GUID); + } + } + + ensureOrder(originalOrder); + yield PT.transact(PT.SortByName(folder_info.GUID)); + ensureOrder(sortedOrder); + yield PT.undo(); + ensureOrder(originalOrder); + yield PT.redo(); + ensureOrder(sortedOrder); + + // Cleanup + observer.reset(); + yield PT.undo(); + ensureOrder(originalOrder); + yield PT.undo(); + ensureItemsRemoved(...originalOrder, folder_info); +}); + +add_task(function* test_livemark_txns() { + let livemark_info = + { feedURI: NetUtil.newURI("http://test.feed.uri") + , parentGUID: yield PlacesUtils.promiseItemGUID(root) + , title: "Test Livemark" }; + function ensureLivemarkAdded() { + ensureItemsAdded({ GUID: livemark_info.GUID + , title: livemark_info.title + , parentGUID: livemark_info.parentGUID + , itemType: bmsvc.TYPE_FOLDER }); + let annos = [{ name: PlacesUtils.LMANNO_FEEDURI + , value: livemark_info.feedURI.spec }]; + if ("siteURI" in livemark_info) { + annos.push({ name: PlacesUtils.LMANNO_SITEURI + , value: livemark_info.siteURI.spec }); + } + ensureAnnotationsSet(livemark_info.GUID, annos); + } + function ensureLivemarkRemoved() { + ensureItemsRemoved({ GUID: livemark_info.GUID + , parentGUID: livemark_info.parentGUID }); + } + + function* _testDoUndoRedoUndo() { + observer.reset(); + livemark_info.GUID = yield PT.transact(PT.NewLivemark(livemark_info)); + ensureLivemarkAdded(); + + observer.reset(); + yield PT.undo(); + ensureLivemarkRemoved(); + + observer.reset(); + yield PT.redo(); + ensureLivemarkAdded(); + + yield PT.undo(); + ensureLivemarkRemoved(); + } + + yield* _testDoUndoRedoUndo() + livemark_info.siteURI = NetUtil.newURI("http://feed.site.uri"); + yield* _testDoUndoRedoUndo(); + + yield PT.clearTransactionsHistory(); +}); diff --git a/toolkit/components/places/tests/unit/test_bookmarks_html.js b/toolkit/components/places/tests/unit/test_bookmarks_html.js index 4562d2fffbb..cdfa276bef1 100644 --- a/toolkit/components/places/tests/unit/test_bookmarks_html.js +++ b/toolkit/components/places/tests/unit/test_bookmarks_html.js @@ -363,13 +363,9 @@ function checkItem(aExpected, aNode) do_check_eq((yield PlacesUtils.getCharsetForURI(testURI)), aExpected.charset); break; case "feedUrl": - yield PlacesUtils.livemarks.getLivemark( - { id: id }, - (aStatus, aLivemark) => { - do_check_true(Components.isSuccessCode(aStatus)); - do_check_eq(aLivemark.siteURI.spec, aExpected.url); - do_check_eq(aLivemark.feedURI.spec, aExpected.feedUrl); - }); + let livemark = yield PlacesUtils.livemarks.getLivemark({ id: id }); + do_check_eq(livemark.siteURI.spec, aExpected.url); + do_check_eq(livemark.feedURI.spec, aExpected.feedUrl); break; case "children": let folder = aNode.QueryInterface(Ci.nsINavHistoryContainerResultNode); diff --git a/toolkit/components/places/tests/unit/test_bookmarks_html_corrupt.js b/toolkit/components/places/tests/unit/test_bookmarks_html_corrupt.js index d7062f77b2b..ff1a6a94a28 100644 --- a/toolkit/components/places/tests/unit/test_bookmarks_html_corrupt.js +++ b/toolkit/components/places/tests/unit/test_bookmarks_html_corrupt.js @@ -154,19 +154,11 @@ function database_check() { // title do_check_eq("Latest Headlines", livemark.title); - let deferGetLivemark = Promise.defer(); - PlacesUtils.livemarks.getLivemark( - { id: livemark.itemId }, - function (aStatus, aLivemark) { - do_check_true(Components.isSuccessCode(aStatus)); - do_check_eq("http://en-us.fxfeeds.mozilla.com/en-US/firefox/livebookmarks/", - aLivemark.siteURI.spec); - do_check_eq("http://en-us.fxfeeds.mozilla.com/en-US/firefox/headlines.xml", - aLivemark.feedURI.spec); - deferGetLivemark.resolve(); - } - ); - yield deferGetLivemark.promise; + let foundLivemark = yield PlacesUtils.livemarks.getLivemark({ id: livemark.itemId }); + do_check_eq("http://en-us.fxfeeds.mozilla.com/en-US/firefox/livebookmarks/", + foundLivemark.siteURI.spec); + do_check_eq("http://en-us.fxfeeds.mozilla.com/en-US/firefox/headlines.xml", + foundLivemark.feedURI.spec); // cleanup toolbar.containerOpen = false; diff --git a/toolkit/components/places/tests/unit/test_bookmarks_json.js b/toolkit/components/places/tests/unit/test_bookmarks_json.js index 7362e3fe32f..a1299e61299 100644 --- a/toolkit/components/places/tests/unit/test_bookmarks_json.js +++ b/toolkit/components/places/tests/unit/test_bookmarks_json.js @@ -192,13 +192,9 @@ function checkItem(aExpected, aNode) { do_check_eq((yield PlacesUtils.getCharsetForURI(testURI)), aExpected.charset); break; case "feedUrl": - yield PlacesUtils.livemarks.getLivemark( - { id: id }, - (aStatus, aLivemark) => { - do_check_true(Components.isSuccessCode(aStatus)); - do_check_eq(aLivemark.siteURI.spec, aExpected.url); - do_check_eq(aLivemark.feedURI.spec, aExpected.feedUrl); - }); + let livemark = yield PlacesUtils.livemarks.getLivemark({ id: id }); + do_check_eq(livemark.siteURI.spec, aExpected.url); + do_check_eq(livemark.feedURI.spec, aExpected.feedUrl); break; case "children": let folder = aNode.QueryInterface(Ci.nsINavHistoryContainerResultNode); diff --git a/toolkit/components/places/tests/unit/test_bug636917_isLivemark.js b/toolkit/components/places/tests/unit/test_bug636917_isLivemark.js index bf655532e3c..62536ede97a 100644 --- a/toolkit/components/places/tests/unit/test_bug636917_isLivemark.js +++ b/toolkit/components/places/tests/unit/test_bug636917_isLivemark.js @@ -13,14 +13,11 @@ function run_test() { if (aAnnotationName == PlacesUtils.LMANNO_FEEDURI) { PlacesUtils.annotations.removeObserver(this); - PlacesUtils.livemarks.getLivemark( - { id: aItemId }, - function (aStatus, aLivemark) { - do_check_true(Components.isSuccessCode(aStatus)); + PlacesUtils.livemarks.getLivemark({ id: aItemId }) + .then(aLivemark => { PlacesUtils.bookmarks.removeItem(aItemId); do_test_finished(); - } - ); + }, do_throw); } }, @@ -39,5 +36,5 @@ function run_test() , siteURI: uri("http://example.com/") , feedURI: uri("http://example.com/rdf") } - ); + ).then(null, do_throw); } diff --git a/toolkit/components/places/tests/unit/test_mozIAsyncLivemarks.js b/toolkit/components/places/tests/unit/test_mozIAsyncLivemarks.js index 429c23bfa10..eeb6fa2b7ca 100644 --- a/toolkit/components/places/tests/unit/test_mozIAsyncLivemarks.js +++ b/toolkit/components/places/tests/unit/test_mozIAsyncLivemarks.js @@ -178,82 +178,52 @@ add_task(function test_addLivemark_noCallback_succeeds() }); -add_task(function test_addLivemark_noSiteURI_callback_succeeds() +add_task(function test_addLivemark_noSiteURI_succeeds() { - let checkLivemark = aLivemark => { - do_check_true(aLivemark.id > 0); - do_check_valid_places_guid(aLivemark.guid); - do_check_eq(aLivemark.title, "test"); - do_check_eq(aLivemark.parentId, PlacesUtils.unfiledBookmarksFolderId); - do_check_eq(aLivemark.index, PlacesUtils.bookmarks.getItemIndex(aLivemark.id)); - do_check_eq(aLivemark.lastModified, PlacesUtils.bookmarks.getItemLastModified(aLivemark.id)); - do_check_true(aLivemark.feedURI.equals(FEED_URI)); - do_check_eq(aLivemark.siteURI, null); - }; - - // The deprecated callback is called before resolving the promise. - let callbackCalled = false; let livemark = yield PlacesUtils.livemarks.addLivemark( { title: "test" , parentId: PlacesUtils.unfiledBookmarksFolderId , index: PlacesUtils.bookmarks.DEFAULT_INDEX , feedURI: FEED_URI - }, - (aStatus, aLivemark) => { - callbackCalled = true; - do_check_true(Components.isSuccessCode(aStatus)); - checkLivemark(aLivemark); - } ); - do_check_true(callbackCalled); - checkLivemark(livemark); + }); + do_check_true(livemark.id > 0); + do_check_valid_places_guid(livemark.guid); + do_check_eq(livemark.title, "test"); + do_check_eq(livemark.parentId, PlacesUtils.unfiledBookmarksFolderId); + do_check_eq(livemark.index, PlacesUtils.bookmarks.getItemIndex(livemark.id)); + do_check_eq(livemark.lastModified, PlacesUtils.bookmarks.getItemLastModified(livemark.id)); + do_check_true(livemark.feedURI.equals(FEED_URI)); + do_check_eq(livemark.siteURI, null); }); -add_task(function test_addLivemark_callback_succeeds() +add_task(function test_addLivemark_succeeds() { - let checkLivemark = aLivemark => { - do_check_true(aLivemark.id > 0); - do_check_valid_places_guid(aLivemark.guid); - do_check_eq(aLivemark.title, "test"); - do_check_eq(aLivemark.parentId, PlacesUtils.unfiledBookmarksFolderId); - do_check_eq(aLivemark.index, PlacesUtils.bookmarks.getItemIndex(aLivemark.id)); - do_check_eq(aLivemark.lastModified, PlacesUtils.bookmarks.getItemLastModified(aLivemark.id)); - do_check_true(aLivemark.feedURI.equals(FEED_URI)); - do_check_true(aLivemark.siteURI.equals(SITE_URI)); - do_check_true(PlacesUtils.annotations - .itemHasAnnotation(aLivemark.id, - PlacesUtils.LMANNO_FEEDURI)); - do_check_true(PlacesUtils.annotations - .itemHasAnnotation(aLivemark.id, - PlacesUtils.LMANNO_SITEURI)); - }; - - // The deprecated callback is called before resolving the promise. - let callbackCalled = false; let livemark = yield PlacesUtils.livemarks.addLivemark( { title: "test" , parentId: PlacesUtils.unfiledBookmarksFolderId , index: PlacesUtils.bookmarks.DEFAULT_INDEX , feedURI: FEED_URI , siteURI: SITE_URI - }, - (aStatus, aLivemark) => { - callbackCalled = true; - do_check_true(Components.isSuccessCode(aStatus)); - checkLivemark(aLivemark); - } ); - do_check_true(callbackCalled); - checkLivemark(livemark); + }); + + do_check_true(livemark.id > 0); + do_check_valid_places_guid(livemark.guid); + do_check_eq(livemark.title, "test"); + do_check_eq(livemark.parentId, PlacesUtils.unfiledBookmarksFolderId); + do_check_eq(livemark.index, PlacesUtils.bookmarks.getItemIndex(livemark.id)); + do_check_eq(livemark.lastModified, PlacesUtils.bookmarks.getItemLastModified(livemark.id)); + do_check_true(livemark.feedURI.equals(FEED_URI)); + do_check_true(livemark.siteURI.equals(SITE_URI)); + do_check_true(PlacesUtils.annotations + .itemHasAnnotation(livemark.id, + PlacesUtils.LMANNO_FEEDURI)); + do_check_true(PlacesUtils.annotations + .itemHasAnnotation(livemark.id, + PlacesUtils.LMANNO_SITEURI)); }); -add_task(function test_addLivemark_bogusid_callback_succeeds() +add_task(function test_addLivemark_bogusid_succeeds() { - let checkLivemark = aLivemark => { - do_check_true(aLivemark.id > 0); - do_check_neq(aLivemark.id, 100); - }; - - // The deprecated callback is called before resolving the promise. - let callbackCalled = false; let livemark = yield PlacesUtils.livemarks.addLivemark( { id: 100 // Should be ignored. , title: "test" @@ -261,118 +231,72 @@ add_task(function test_addLivemark_bogusid_callback_succeeds() , index: PlacesUtils.bookmarks.DEFAULT_INDEX , feedURI: FEED_URI , siteURI: SITE_URI - }, - (aStatus, aLivemark) => { - callbackCalled = true; - do_check_true(Components.isSuccessCode(aStatus)); - checkLivemark(aLivemark); - } ); - do_check_true(callbackCalled); - checkLivemark(livemark); + }); + do_check_true(livemark.id > 0); + do_check_neq(livemark.id, 100); }); -add_task(function test_addLivemark_bogusParent_callback_fails() +add_task(function test_addLivemark_bogusParent_fails() { - // The deprecated callback is called before resolving the promise. - let callbackCalled = false; try { yield PlacesUtils.livemarks.addLivemark( { title: "test" , parentId: 187 , index: PlacesUtils.bookmarks.DEFAULT_INDEX , feedURI: FEED_URI - }, - (aStatus, aLivemark) => { - callbackCalled = true; - do_check_false(Components.isSuccessCode(aStatus)); - do_check_eq(aLivemark, null); - } ); + }); do_throw("Adding a livemark with a bogus parent should fail"); - } - catch(ex) { - do_check_true(callbackCalled); - } + } catch(ex) {} }); -add_task(function test_addLivemark_intoLivemark_callback_fails() +add_task(function test_addLivemark_intoLivemark_fails() { - // The deprecated callback is called before resolving the promise. - let callbackCalled = false; let livemark = yield PlacesUtils.livemarks.addLivemark( { title: "test" , parentId: PlacesUtils.unfiledBookmarksFolderId , index: PlacesUtils.bookmarks.DEFAULT_INDEX , feedURI: FEED_URI - }, - (aStatus, aLivemark) => { - callbackCalled = true; - do_check_true(Components.isSuccessCode(aStatus)); - } ); - do_check_true(callbackCalled); + }); do_check_true(Boolean(livemark)); - callbackCalled = false; try { yield PlacesUtils.livemarks.addLivemark( { title: "test" , parentId: livemark.id , index: PlacesUtils.bookmarks.DEFAULT_INDEX , feedURI: FEED_URI - }, - (aStatus, aLivemark) => { - callbackCalled = true; - do_check_false(Components.isSuccessCode(aStatus)); - do_check_eq(aLivemark, null); - } ); + }); do_throw("Adding a livemark into a livemark should fail"); - } - catch(ex) { - do_check_true(callbackCalled); - } + } catch(ex) {} }); -add_task(function test_addLivemark_forceGuid_callback_succeeds() +add_task(function test_addLivemark_forceGuid_succeeds() { let checkLivemark = aLivemark => { do_check_eq(aLivemark.guid, "1234567890AB"); do_check_guid_for_bookmark(aLivemark.id, "1234567890AB"); }; - // The deprecated callback is called before resolving the promise. - let callbackCalled = false; let livemark = yield PlacesUtils.livemarks.addLivemark( { title: "test" , parentId: PlacesUtils.unfiledBookmarksFolderId , index: PlacesUtils.bookmarks.DEFAULT_INDEX , feedURI: FEED_URI , guid: "1234567890AB" - }, - (aStatus, aLivemark) => { - callbackCalled = true; - do_check_true(Components.isSuccessCode(aStatus)); - checkLivemark(aLivemark); - } ); - do_check_true(callbackCalled); + }); checkLivemark(livemark); }); -add_task(function test_addLivemark_lastModified_callback_succeeds() +add_task(function test_addLivemark_lastModified_succeeds() { let now = Date.now() * 1000; - let callbackCalled = false; let livemark = yield PlacesUtils.livemarks.addLivemark( { title: "test" , parentId: PlacesUtils.unfiledBookmarksFolderId , index: PlacesUtils.bookmarks.DEFAULT_INDEX , feedURI: FEED_URI , lastModified: now - }, - (aStatus, aLivemark) => { - callbackCalled = true; - do_check_true(Components.isSuccessCode(aStatus)); - do_check_eq(aLivemark.lastModified, now); - } ); - do_check_true(callbackCalled); + }); do_check_eq(livemark.lastModified, now); }); @@ -398,19 +322,11 @@ add_task(function test_removeLivemark_noValidId_throws() add_task(function test_removeLivemark_nonExistent_fails() { - let callbackCalled = false; try { - yield PlacesUtils.livemarks.removeLivemark( - { id: 1337 }, - (aStatus, aLivemark) => { - callbackCalled = true; - do_check_false(Components.isSuccessCode(aStatus)); - do_check_eq(aLivemark, null); - } ); + yield PlacesUtils.livemarks.removeLivemark({ id: 1337 }); do_throw("Removing a non-existent livemark should fail"); } catch(ex) { - do_check_true(callbackCalled); } }); @@ -469,36 +385,20 @@ add_task(function test_getLivemark_noValidId_throws() add_task(function test_getLivemark_nonExistentId_fails() { - let callbackCalled = false; try { - yield PlacesUtils.livemarks.getLivemark({ id: 1234 }, - (aStatus, aLivemark) => { - callbackCalled = true; - do_check_false(Components.isSuccessCode(aStatus)); - do_check_eq(aLivemark, null); - } ); + yield PlacesUtils.livemarks.getLivemark({ id: 1234 }); do_throw("getLivemark for a non existent id should fail"); } - catch(ex) { - do_check_true(callbackCalled); - } + catch(ex) {} }); add_task(function test_getLivemark_nonExistentGUID_fails() { - let callbackCalled = false; try { - yield PlacesUtils.livemarks.getLivemark({ guid: "34567890ABCD" }, - (aStatus, aLivemark) => { - callbackCalled = true; - do_check_false(Components.isSuccessCode(aStatus)); - do_check_eq(aLivemark, null); - } ); + yield PlacesUtils.livemarks.getLivemark({ guid: "34567890ABCD" }); do_throw("getLivemark for a non-existent guid should fail"); } - catch(ex) { - do_check_true(callbackCalled); - } + catch(ex) {} }); add_task(function test_getLivemark_guid_succeeds() @@ -510,26 +410,16 @@ add_task(function test_getLivemark_guid_succeeds() , feedURI: FEED_URI , guid: "34567890ABCD" }); - let checkLivemark = aLivemark => { - do_check_eq(aLivemark.title, "test"); - do_check_eq(aLivemark.parentId, PlacesUtils.unfiledBookmarksFolderId); - do_check_eq(aLivemark.index, PlacesUtils.bookmarks.getItemIndex(aLivemark.id)); - do_check_true(aLivemark.feedURI.equals(FEED_URI)); - do_check_eq(aLivemark.siteURI, null); - do_check_eq(aLivemark.guid, "34567890ABCD"); - }; - // invalid id to check the guid wins. let livemark = - yield PlacesUtils.livemarks.getLivemark({ id: 789, guid: "34567890ABCD" }, - (aStatus, aLivemark) => { - callbackCalled = true; - do_check_true(Components.isSuccessCode(aStatus)); - checkLivemark(aLivemark) - } ); + yield PlacesUtils.livemarks.getLivemark({ id: 789, guid: "34567890ABCD" }); - do_check_true(callbackCalled); - checkLivemark(livemark); + do_check_eq(livemark.title, "test"); + do_check_eq(livemark.parentId, PlacesUtils.unfiledBookmarksFolderId); + do_check_eq(livemark.index, PlacesUtils.bookmarks.getItemIndex(livemark.id)); + do_check_true(livemark.feedURI.equals(FEED_URI)); + do_check_eq(livemark.siteURI, null); + do_check_eq(livemark.guid, "34567890ABCD"); }); add_task(function test_getLivemark_id_succeeds() @@ -541,26 +431,14 @@ add_task(function test_getLivemark_id_succeeds() , feedURI: FEED_URI }); - let checkLivemark = aLivemark => { - do_check_eq(aLivemark.title, "test"); - do_check_eq(aLivemark.parentId, PlacesUtils.unfiledBookmarksFolderId); - do_check_eq(aLivemark.index, PlacesUtils.bookmarks.getItemIndex(aLivemark.id)); - do_check_true(aLivemark.feedURI.equals(FEED_URI)); - do_check_eq(aLivemark.siteURI, null); - do_check_guid_for_bookmark(aLivemark.id, aLivemark.guid); - }; + livemark = yield PlacesUtils.livemarks.getLivemark({ id: livemark.id }); - let callbackCalled = false; - livemark = yield PlacesUtils.livemarks.getLivemark( - { id: livemark.id }, - (aStatus, aLivemark) => { - callbackCalled = true; - do_check_true(Components.isSuccessCode(aStatus)); - checkLivemark(aLivemark); - } ); - - do_check_true(callbackCalled); - checkLivemark(livemark); + do_check_eq(livemark.title, "test"); + do_check_eq(livemark.parentId, PlacesUtils.unfiledBookmarksFolderId); + do_check_eq(livemark.index, PlacesUtils.bookmarks.getItemIndex(livemark.id)); + do_check_true(livemark.feedURI.equals(FEED_URI)); + do_check_eq(livemark.siteURI, null); + do_check_guid_for_bookmark(livemark.id, livemark.guid); }); add_task(function test_getLivemark_removeItem_contention() @@ -579,26 +457,14 @@ add_task(function test_getLivemark_removeItem_contention() let id = PlacesUtils.bookmarks.getIdForItemAt(PlacesUtils.unfiledBookmarksFolderId, PlacesUtils.bookmarks.DEFAULT_INDEX); - let checkLivemark = (aLivemark) => { - do_check_eq(aLivemark.title, "test"); - do_check_eq(aLivemark.parentId, PlacesUtils.unfiledBookmarksFolderId); - do_check_eq(aLivemark.index, PlacesUtils.bookmarks.getItemIndex(aLivemark.id)); - do_check_true(aLivemark.feedURI.equals(FEED_URI)); - do_check_eq(aLivemark.siteURI, null); - do_check_guid_for_bookmark(aLivemark.id, aLivemark.guid); - }; + let livemark = yield PlacesUtils.livemarks.getLivemark({ id: id }); - let callbackCalled = false; - let livemark = yield PlacesUtils.livemarks.getLivemark( - { id: id }, - (aStatus, aLivemark) => { - callbackCalled = true; - do_check_true(Components.isSuccessCode(aStatus)); - checkLivemark(aLivemark); - } ); - - do_check_true(callbackCalled); - checkLivemark(livemark); + do_check_eq(livemark.title, "test"); + do_check_eq(livemark.parentId, PlacesUtils.unfiledBookmarksFolderId); + do_check_eq(livemark.index, PlacesUtils.bookmarks.getItemIndex(livemark.id)); + do_check_true(livemark.feedURI.equals(FEED_URI)); + do_check_eq(livemark.siteURI, null); + do_check_guid_for_bookmark(livemark.id, livemark.guid); }); add_task(function test_title_change() diff --git a/toolkit/components/places/tests/unit/xpcshell.ini b/toolkit/components/places/tests/unit/xpcshell.ini index 0c0d2e87c11..b033699bf52 100644 --- a/toolkit/components/places/tests/unit/xpcshell.ini +++ b/toolkit/components/places/tests/unit/xpcshell.ini @@ -138,3 +138,4 @@ skip-if = os == "android" [test_telemetry.js] [test_getPlacesInfo.js] [test_pageGuid_bookmarkGuid.js] +[test_async_transactions.js]