Bug 1368754 - fix consumers which wait for `NodeHistoryDetailsChanged`/`NodeURIChanged` to be called to use node for new values, r=mak
authormilindl <i.milind.luthra@gmail.com>
Tue, 30 May 2017 23:51:09 +0530
changeset 362535 aa36ffe32ddc2afb9d80f232694c65ec86923644
parent 362534 9cab45e72b30ee0d2a67e768ed08b1b28e56a03c
child 362536 c27161891c66b52f2771e483f683290ff058710c
push id44196
push usermak77@bonardo.net
push dateTue, 06 Jun 2017 16:24:51 +0000
treeherderautoland@aa36ffe32ddc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1368754
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1368754 - fix consumers which wait for `NodeHistoryDetailsChanged`/`NodeURIChanged` to be called to use node for new values, r=mak Updated time, access count and uri can be accessed using the node passed to the method. There is no need to access the other arguments, which contain the old values of the quantities changed. MozReview-Commit-ID: 3WEwAs8gQ0w
browser/components/places/content/browserPlacesViews.js
browser/components/places/content/treeView.js
toolkit/components/places/tests/unit/test_nsINavHistoryViewer.js
--- a/browser/components/places/content/browserPlacesViews.js
+++ b/browser/components/places/content/browserPlacesViews.js
@@ -491,17 +491,17 @@ PlacesViewBase.prototype = {
 
   nodeURIChanged: function PVB_nodeURIChanged(aPlacesNode, aURIString) {
     let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
 
     // Here we need the <menu>.
     if (elt.localName == "menupopup")
       elt = elt.parentNode;
 
-    elt.setAttribute("scheme", PlacesUIUtils.guessUrlSchemeForUI(aURIString));
+    elt.setAttribute("scheme", PlacesUIUtils.guessUrlSchemeForUI(aPlacesNode.uri));
   },
 
   nodeIconChanged: function PVB_nodeIconChanged(aPlacesNode) {
     let elt = this._getDOMNodeForPlacesNode(aPlacesNode);
 
     // There's no UI representation for the root node, thus there's nothing to
     // be done when the icon changes.
     if (elt == this._rootElt)
@@ -591,17 +591,17 @@ PlacesViewBase.prototype = {
     if (aPlacesNode.parent &&
         this.controller.hasCachedLivemarkInfo(aPlacesNode.parent)) {
       // Find the node in the parent.
       let popup = this._getDOMNodeForPlacesNode(aPlacesNode.parent);
       for (let child = popup._startMarker.nextSibling;
            child != popup._endMarker;
            child = child.nextSibling) {
         if (child._placesNode && child._placesNode.uri == aPlacesNode.uri) {
-          if (aCount)
+          if (aPlacesNode.accessCount)
             child.setAttribute("visited", "true");
           else
             child.removeAttribute("visited");
           break;
         }
       }
     }
   },
--- a/browser/components/places/content/treeView.js
+++ b/browser/components/places/content/treeView.js
@@ -811,27 +811,27 @@ PlacesTreeView.prototype = {
         }
       }, Components.utils.reportError);
   },
 
   nodeTitleChanged: function PTV_nodeTitleChanged(aNode, aNewTitle) {
     this._invalidateCellValue(aNode, this.COLUMN_TYPE_TITLE);
   },
 
-  nodeURIChanged: function PTV_nodeURIChanged(aNode, aNewURI) {
+  nodeURIChanged: function PTV_nodeURIChanged(aNode, aOldURI) {
     this._invalidateCellValue(aNode, this.COLUMN_TYPE_URI);
   },
 
   nodeIconChanged: function PTV_nodeIconChanged(aNode) {
     this._invalidateCellValue(aNode, this.COLUMN_TYPE_TITLE);
   },
 
   nodeHistoryDetailsChanged:
-  function PTV_nodeHistoryDetailsChanged(aNode, aUpdatedVisitDate,
-                                         aUpdatedVisitCount) {
+  function PTV_nodeHistoryDetailsChanged(aNode, aOldVisitDate,
+                                         aOldVisitCount) {
     if (aNode.parent && this._controller.hasCachedLivemarkInfo(aNode.parent)) {
       // Find the node in the parent.
       let parentRow = this._flatList ? 0 : this._getRowForNode(aNode.parent);
       for (let i = parentRow; i < this._rows.length; i++) {
         let child = this.nodeForTreeIndex(i);
         if (child.uri == aNode.uri) {
           this._cellProperties.delete(child);
           this._invalidateCellValue(child, this.COLUMN_TYPE_TITLE);
--- a/toolkit/components/places/tests/unit/test_nsINavHistoryViewer.js
+++ b/toolkit/components/places/tests/unit/test_nsINavHistoryViewer.js
@@ -22,21 +22,21 @@ var resultObserver = {
     this.nodeChangedByTitle = node;
     this.newTitle = newTitle;
   },
 
   newAccessCount: 0,
   newTime: 0,
   nodeChangedByHistoryDetails: null,
   nodeHistoryDetailsChanged(node,
-                                         updatedVisitDate,
-                                         updatedVisitCount) {
-    this.nodeChangedByHistoryDetails = node
-    this.newTime = updatedVisitDate;
-    this.newAccessCount = updatedVisitCount;
+                            oldVisitDate,
+                            oldVisitCount) {
+    this.nodeChangedByHistoryDetails = node;
+    this.newTime = node.time;
+    this.newAccessCount = node.accessCount;
   },
 
   movedNode: null,
   nodeMoved(node, oldParent, oldIndex, newParent, newIndex) {
     this.movedNode = node;
   },
   openedContainer: null,
   closedContainer: null,
@@ -119,17 +119,17 @@ add_test(function check_history_query() 
 
         // nsINavHistoryResultObserver.sortingChanged
         resultObserver.invalidatedContainer = null;
         result.sortingMode = options.SORT_BY_TITLE_ASCENDING;
         do_check_eq(resultObserver.sortingMode, options.SORT_BY_TITLE_ASCENDING);
         do_check_eq(resultObserver.invalidatedContainer, result.root);
 
         // nsINavHistoryResultObserver.invalidateContainer
-        PlacesTestUtils.clearHistoryEnabled().then(() => {
+        PlacesTestUtils.clearHistory().then(() => {
           do_check_eq(root.uri, resultObserver.invalidatedContainer.uri);
 
           // nsINavHistoryResultObserver.batching
           do_check_false(resultObserver.inBatchMode);
           PlacesUtils.history.runInBatchMode({
             runBatched(aUserData) {
               do_check_true(resultObserver.inBatchMode);
             }