Bug 419749 - Add/Update/Delete for items in query set do not seem to close transaction (r=dietrich)

toolkit/components/places/public/nsINavHistoryService.idl

@@ -1045,7 +1045,7 @@ interface nsINavHistoryQueryOptions : nsISupports
   nsINavHistoryQueryOptions clone();
 };
 
-[scriptable, uuid(a9bea2db-0a3e-4895-9ccc-24bac41a0674)]
+[scriptable, uuid(1972ac40-d270-40ee-995a-c97d02849596)]
 interface nsINavHistoryService : nsISupports
 {
   /**
@@ -1123,25 +1123,6 @@ interface nsINavHistoryService : nsISupports
    */
   boolean canAddURI(in nsIURI aURI);
 
-  /**
-   * Call to set the full information for a given page. If the page does not
-   * exist, it will be added to the database. If it does, the existing values
-   * WILL BE OVERWRITTEN. This is an updated version of addPageWithDetails
-   * for backup/restore type operations.
-   *
-   * @param aURI        Page to add/change.
-   * @param aTitle      Title as specified by the page.
-   * @param aVisitCount Number of times this page has been visited. Setting this
-   *                    to 0 may make the page invisible in some views.
-   * @param aHidden     Whether the page is hidden. If the page has only
-   *                    TRANSITION_EMBED visits, this will be true, otherwise
-   *                    false.
-   * @param aTyped      True if this URI has ever been typed.
-   */
-  void setPageDetails(in nsIURI aURI, in AString aTitle, 
-                      in unsigned long aVisitCount, in boolean aHidden,
-                      in boolean aTyped);
-
   /**
    * Call to manually add a visit for a specific page. This will probably not
    * be commonly used other than for backup/restore type operations. If the URI

toolkit/components/places/src/nsNavHistory.cpp

@@ -2342,56 +2342,6 @@ nsNavHistory::CanAddURI(nsIURI* aURI, PRBool* canAdd)
   return NS_OK;
 }
 
-
-// nsNavHistory::SetPageDetails
-
-NS_IMETHODIMP
-nsNavHistory::SetPageDetails(nsIURI* aURI, const nsAString& aTitle,
-                             PRUint32 aVisitCount, PRBool aHidden,
-                             PRBool aTyped)
-{
-  // look up the page ID, creating a new one if necessary
-  PRInt64 pageID;
-  nsresult rv = GetUrlIdFor(aURI, &pageID, PR_TRUE);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  nsCOMPtr<mozIStorageStatement> statement;
-  rv = mDBConn->CreateStatement(NS_LITERAL_CSTRING(
-      "UPDATE moz_places "
-      "SET title = ?2, "
-          "visit_count = ?4, "
-          "hidden = ?5, "
-          "typed = ?6 "
-       "WHERE id = ?1"),
-    getter_AddRefs(statement));
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = statement->BindInt64Parameter(0, pageID);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  // XXX should we be calculating / setting frecency here?
-
-  // for the titles, be careful to interpret isVoid as NULL SQL command so that
-  // we can tell the difference between "set to empty" and "unset"
-  if (aTitle.IsVoid())
-    rv = statement->BindNullParameter(1);
-  else
-    rv = statement->BindStringParameter(1, StringHead(aTitle, HISTORY_TITLE_LENGTH_MAX));
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  rv = statement->BindInt32Parameter(3, aVisitCount);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = statement->BindInt32Parameter(4, aHidden ? 1 : 0);
-  NS_ENSURE_SUCCESS(rv, rv);
-  rv = statement->BindInt32Parameter(5, aTyped ? 1 : 0);
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  rv = statement->Execute();
-  NS_ENSURE_SUCCESS(rv, rv);
-
-  return NS_OK;
-}
-
-
 // nsNavHistory::AddVisit
 //
 //    Adds or updates a page with the given URI. The ID of the new visit will

toolkit/components/places/src/nsNavHistory.h

@@ -343,7 +343,7 @@ public:
                                    nsCOMArray<nsNavHistoryQuery>* aQueries,
                                    nsNavHistoryQueryOptions** aOptions);
 
-  // Import-friendly version of SetPageDetails + AddVisit.
+  // Import-friendly version of AddVisit.
   // This method adds a page to history along with a single last visit.
   // It is an error to call this method if aURI might already be in history.
   // The given aVisitCount should include the given last-visit date.

toolkit/components/places/src/nsNavHistoryResult.cpp

@@ -2528,7 +2528,7 @@ nsNavHistoryQueryResultNode::OnVisit(nsIURI* aURI, PRInt64 aVisitId,
 
       PRBool hasDomain;
       queryHost->GetHasDomain(&hasDomain);
-      if (!hasDomain || !queryHost->DomainIsHost())
+      if (!hasDomain)
         return NS_OK;
 
       nsCAutoString host;

toolkit/components/places/tests/unit/test_000_frecency.js

@@ -51,12 +51,19 @@ Autocomplete Frecency Tests
 
 */
 
-var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"].
-              getService(Ci.nsINavHistoryService);
-var bmsvc = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].
-            getService(Ci.nsINavBookmarksService);
-var prefs = Cc["@mozilla.org/preferences-service;1"].
-            getService(Ci.nsIPrefBranch);
+try {
+  var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"].
+                getService(Ci.nsINavHistoryService);
+  var bhist = histsvc.QueryInterface(Ci.nsIBrowserHistory);
+  var ghist = Cc["@mozilla.org/browser/global-history;2"].
+              getService(Ci.nsIGlobalHistory2);
+  var bmsvc = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].
+              getService(Ci.nsINavBookmarksService);
+  var prefs = Cc["@mozilla.org/preferences-service;1"].
+              getService(Ci.nsIPrefBranch);
+} catch(ex) {
+  do_throw("Could not get services\n");
+}
 
 function add_visit(aURI, aVisitDate, aVisitType) {
   var isRedirect = aVisitType == histsvc.TRANSITION_REDIRECT_PERMANENT ||
@@ -132,7 +139,8 @@ bucketPrefs.every(function(bucket) {
         }
         else {
           matchTitle = searchTerm + "UnvisitedTyped";
-          histsvc.setPageDetails(calculatedURI, matchTitle, 1, false, true);
+          ghist.setPageTitle(calculatedURI, matchTitle);
+          bhist.markPageAsTyped(calculatedURI);
         }
       }
     }

toolkit/components/places/tests/unit/test_416211.js

@@ -54,6 +54,7 @@ Test showing bookmark title when matching the tag for bug 416211
 try {
   var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"].
                 getService(Ci.nsINavHistoryService);
+  var bhist = histsvc.QueryInterface(Ci.nsIBrowserHistory);
   var bmsvc = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].
               getService(Ci.nsINavBookmarksService);
   var tagssvc = Cc["@mozilla.org/browser/tagging-service;1"].
@@ -75,8 +76,8 @@ function add_visit(aURI, aVisitDate, aVisitType) {
 var theTag = "superTag";
 var url = uri("http://www.foobar.com/");
 var title = "Cool Title";
-histsvc.setPageDetails(url, theTag, 1, false, true);
-add_visit(url, Date.now(), Ci.nsINavHistoryService.TRANSITION_LINK);
+bhist.addPageWithDetails(url, theTag, Date.now());
+
 tagssvc.tagURI(url, [theTag]);
 bmsvc.insertBookmark(bmsvc.unfiledBookmarksFolder, url, bmsvc.DEFAULT_INDEX, title);
 

toolkit/components/places/tests/unit/test_417798.js

@@ -163,10 +163,13 @@ function ensure_results(aSearch, aExpected)
 
 // Get history services
 try {
-  var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"].getService(Ci.nsINavHistoryService);
+  var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"].
+                getService(Ci.nsINavHistoryService);
   var bhist = histsvc.QueryInterface(Ci.nsIBrowserHistory);
-  var bmsvc = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].getService(Ci.nsINavBookmarksService);
-  var iosvc = Cc["@mozilla.org/network/io-service;1"].getService(Ci.nsIIOService);
+  var bmsvc = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].
+              getService(Ci.nsINavBookmarksService);
+  var iosvc = Cc["@mozilla.org/network/io-service;1"].
+              getService(Ci.nsIIOService);
 } catch(ex) {
   do_throw("Could not get services\n");
 }
@@ -181,8 +184,7 @@ function addPageBook(aURI, aTitle, aBook)
 
   print("Adding page/book: " + [aURI, aTitle, aBook, kURIs[aURI], title].join(", "));
   // Add the page and a visit for good measure
-  histsvc.setPageDetails(uri, title, 1, false, true);
-  histsvc.addVisit(uri, gDate, null, histsvc.TRANSITION_TYPED, false, 0);
+  bhist.addPageWithDetails(uri, title, gDate);
 
   // Add a bookmark if we need to
   if (aBook != undefined) {

toolkit/components/places/tests/unit/test_adaptive.js

@@ -141,18 +141,17 @@ function ensure_results(uris, searchTerm)
 
 // Get history service
 try {
-  var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"].getService(Ci.nsINavHistoryService);
+  var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"].
+                getService(Ci.nsINavHistoryService);
   var bhist = histsvc.QueryInterface(Ci.nsIBrowserHistory);
-  var obs = Cc["@mozilla.org/observer-service;1"].getService(Ci.nsIObserverService);
+  var obs = Cc["@mozilla.org/observer-service;1"].
+            getService(Ci.nsIObserverService);
 } catch(ex) {
   do_throw("Could not get history service\n");
 } 
 
 function setCountRank(aURI, aCount, aRank, aSearch)
 {
-  // Set the visit count and date for a uri
-  histsvc.setPageDetails(aURI, aURI, aCount, false, true);
-
   // Bump up the visit count for the uri
   for (let i = 0; i < aCount; i++)
     histsvc.addVisit(aURI, d1, null, histsvc.TRANSITION_TYPED, false, 0);

toolkit/components/places/tests/unit/test_frecency.js

@@ -143,7 +143,8 @@ function ensure_results(uris, searchTerm)
 
 // Get history service
 try {
-  var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"].getService(Ci.nsINavHistoryService);
+  var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"].
+                getService(Ci.nsINavHistoryService);
   var bhist = histsvc.QueryInterface(Ci.nsIBrowserHistory);
   var tagssvc = Cc["@mozilla.org/browser/tagging-service;1"].
                 getService(Ci.nsITaggingService);
@@ -153,8 +154,6 @@ try {
 
 function setCountDate(aURI, aCount, aDate)
 {
-  // Set the visit count and date for a uri
-  histsvc.setPageDetails(aURI, aURI, aCount, false, true);
   // We need visits so that frecency can be computed over multiple visits
   for (let i = 0; i < aCount; i++)
     histsvc.addVisit(aURI, aDate, null, histsvc.TRANSITION_TYPED, false, 0);

toolkit/components/places/tests/unit/test_multi_word_search.js

@@ -169,10 +169,13 @@ function ensure_results(aSearch, aExpected)
 
 // Get history services
 try {
-  var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"].getService(Ci.nsINavHistoryService);
+  var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"].
+                getService(Ci.nsINavHistoryService);
   var bhist = histsvc.QueryInterface(Ci.nsIBrowserHistory);
-  var bmsvc = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].getService(Ci.nsINavBookmarksService);
-  var iosvc = Cc["@mozilla.org/network/io-service;1"].getService(Ci.nsIIOService);
+  var bmsvc = Cc["@mozilla.org/browser/nav-bookmarks-service;1"].
+              getService(Ci.nsINavBookmarksService);
+  var iosvc = Cc["@mozilla.org/network/io-service;1"].
+              getService(Ci.nsIIOService);
 } catch(ex) {
   do_throw("Could not get services\n");
 } 
@@ -207,9 +210,8 @@ function addPageBook(aURI, aTitle, aBook)
 
   print("Adding page/book: " + [aURI, aTitle, aBook, kURIs[aURI], title].join(", "));
   // Add the page and a visit for good measure
-  histsvc.setPageDetails(uri, title, 1, false, true);
-  histsvc.addVisit(uri, gDate, null, histsvc.TRANSITION_TYPED, false, 0);
-  
+  bhist.addPageWithDetails(uri, title, gDate);
+
   // Add a bookmark if we need to
   if (aBook != undefined) {
     let book = kTitles[aBook];

toolkit/components/places/tests/unit/test_resolveNullBookmarkTitles.js

@@ -42,8 +42,9 @@
 try {
   var histsvc = Cc["@mozilla.org/browser/nav-history-service;1"].
                 getService(Ci.nsINavHistoryService);
+  var bhist = histsvc.QueryInterface(Ci.nsIBrowserHistory);
 } catch(ex) {
-  do_throw("Could not get history service\n");
+  do_throw("Could not get history services\n");
 }
 
 // Get bookmark service
@@ -71,10 +72,9 @@ function run_test() {
   tagssvc.tagURI(uri1, ["tag 1"]);
   tagssvc.tagURI(uri2, ["tag 2"]);
 
-  histsvc.addVisit(uri1, Date.now() * 1000, null, histsvc.TRANSITION_TYPED, false, 0);
-  histsvc.setPageDetails(uri1, "foo title", 0, false, true);
-  histsvc.addVisit(uri2, Date.now() * 1000, null, histsvc.TRANSITION_TYPED, false, 0);
-  histsvc.setPageDetails(uri2, "bar title", 0, false, true);
+  bhist.addPageWithDetails(uri1, "foo title", Date.now() * 1000);
+
+  bhist.addPageWithDetails(uri2, "bar title", Date.now() * 1000);
 
   var options = histsvc.getNewQueryOptions();
   options.queryType = Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS;
@@ -84,7 +84,7 @@ function run_test() {
   options.queryType = Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS;
   options.maxResults = 2;
 
-  query = histsvc.getNewQuery();
+  var query = histsvc.getNewQuery();
   query.setFolders([bmsvc.tagsFolder], 1);
   var result = histsvc.executeQuery(query, options);
   var root = result.root;