Bug 487511 - nsINavHistoryObserver has no "onBeforeDeleteURI" callback

r=dietrich

browser/components/feeds/src/FeedWriter.js

@@ -1423,6 +1423,7 @@ FeedWriter.prototype = {
    onEndUpdateBatch: function() { },
    onVisit: function() { },
    onTitleChanged: function() { },
+   onBeforeDeleteURI: function() { },
    onDeleteURI: function() { },
    onClearHistory: function() { },
    onPageExpired: function() { },

toolkit/components/downloads/src/nsDownloadManager.cpp

@@ -1820,6 +1820,12 @@ nsDownloadManager::OnTitleChanged(nsIURI *aURI, const nsAString &aPageTitle)
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsDownloadManager::OnBeforeDeleteURI(nsIURI *aURI)
+{
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsDownloadManager::OnDeleteURI(nsIURI *aURI)
 {

toolkit/components/places/public/nsINavHistoryService.idl

@@ -625,7 +625,7 @@ interface nsINavHistoryResult : nsISupports
  * DANGER! If you are in the middle of a batch transaction, there may be a
  * database transaction active. You can still access the DB, but be careful.
  */
-[scriptable, uuid(eacb76eb-3eeb-419b-a963-9b3a9d65f356)]
+[scriptable, uuid(14065711-8a91-4d96-ba32-59512f5401b6)]
 interface nsINavHistoryObserver : nsISupports
 {
   /**
@@ -679,6 +679,15 @@ interface nsINavHistoryObserver : nsISupports
    */
   void onTitleChanged(in nsIURI aURI, in AString aPageTitle);
 
+  /**
+   * This page and all of its visits are about to be deleted.  Note: the page
+   * may not necessarily have actually existed for this function to be called.
+   *
+   * @param aURI
+   *        The URI being deleted.
+   */
+  void onBeforeDeleteURI(in nsIURI aURI);
+
   /**
    * This page and all of its visits are being deleted. Note: the page may not
    * necessarily have actually existed for this function to be called.

toolkit/components/places/src/nsNavBookmarks.cpp

@@ -3037,6 +3037,12 @@ nsNavBookmarks::OnVisit(nsIURI *aURI, PRInt64 aVisitID, PRTime aTime,
   return NS_OK;
 }
 
+NS_IMETHODIMP
+nsNavBookmarks::OnBeforeDeleteURI(nsIURI *aURI)
+{
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 nsNavBookmarks::OnDeleteURI(nsIURI *aURI)
 {

toolkit/components/places/src/nsNavHistory.cpp

@@ -4576,10 +4576,15 @@ nsNavHistory::RemovePage(nsIURI *aURI)
 {
   NS_ASSERTION(NS_IsMainThread(), "This can only be called on the main thread");
 
+  // Before we remove, we have to notify our observers!
+  ENUMERATE_WEAKARRAY(mObservers, nsINavHistoryObserver,
+                      OnBeforeDeleteURI(aURI))
+
   nsIURI** URIs = &aURI;
   nsresult rv = RemovePages(URIs, 1, PR_FALSE);
   NS_ENSURE_SUCCESS(rv, rv);
-  // call observers here for the removed url
+
+  // Notify our observers that the URI has been removed.
   ENUMERATE_WEAKARRAY(mObservers, nsINavHistoryObserver, OnDeleteURI(aURI))
   return NS_OK;
 }

toolkit/components/places/src/nsNavHistoryResult.cpp

@@ -2770,6 +2770,12 @@ nsNavHistoryQueryResultNode::OnTitleChanged(nsIURI* aURI,
 }
 
 
+NS_IMETHODIMP
+nsNavHistoryQueryResultNode::OnBeforeDeleteURI(nsIURI *aURI)
+{
+  return NS_OK;
+}
+
 // nsNavHistoryQueryResultNode::OnDeleteURI
 //
 //    Here, we can always live update by just deleting all occurrences of
@@ -4345,6 +4351,14 @@ nsNavHistoryResult::OnTitleChanged(nsIURI* aURI, const nsAString& aPageTitle)
 }
 
 
+// nsNavHistoryResult::OnBeforeDeleteURI (nsINavHistoryObserver)
+NS_IMETHODIMP
+nsNavHistoryResult::OnBeforeDeleteURI(nsIURI *aURI)
+{
+  return NS_OK;
+}
+
+
 // nsNavHistoryResult::OnDeleteURI (nsINavHistoryObserver)
 
 NS_IMETHODIMP

toolkit/components/places/src/nsNavHistoryResult.h

@@ -100,6 +100,7 @@ private:
                      PRInt64 aSessionId, PRInt64 aReferringId,          \
                      PRUint32 aTransitionType, PRUint32* aAdded);      \
   NS_IMETHOD OnTitleChanged(nsIURI* aURI, const nsAString& aPageTitle); \
+  NS_IMETHOD OnBeforeDeleteURI(nsIURI *aURI);                           \
   NS_IMETHOD OnDeleteURI(nsIURI *aURI);                                 \
   NS_IMETHOD OnClearHistory();                                          \
   NS_IMETHOD OnPageChanged(nsIURI *aURI, PRUint32 aWhat,                \

toolkit/components/places/tests/browser/browser_bug413985-history.go-0.js

@@ -56,6 +56,7 @@ function test()
       finish();
     },
     onTitleChanged: function(aURI, aPageTitle) {},
+    onBeforeDeleteURI: function(aURI) {},
     onDeleteURI: function(aURI) {},
     onClearHistory: function() {},
     onPageChanged: function(aURI, aWhat, aValue) {},

toolkit/components/places/tests/browser/browser_bug413985-httprefresh.js

@@ -56,6 +56,7 @@ function test()
       finish();
     },
     onTitleChanged: function(aURI, aPageTitle) {},
+    onBeforeDeleteURI: function(aURI) {},
     onDeleteURI: function(aURI) {},
     onClearHistory: function() {},
     onPageChanged: function(aURI, aWhat, aValue) {},

toolkit/components/places/tests/browser/browser_bug413985-location.reload.js

@@ -56,6 +56,7 @@ function test()
       finish();
     },
     onTitleChanged: function(aURI, aPageTitle) {},
+    onBeforeDeleteURI: function(aURI) {},
     onDeleteURI: function(aURI) {},
     onClearHistory: function() {},
     onPageChanged: function(aURI, aWhat, aValue) {},

toolkit/components/places/tests/browser/browser_bug413985-location.replace.js

@@ -56,6 +56,7 @@ function test()
       finish();
     },
     onTitleChanged: function(aURI, aPageTitle) {},
+    onBeforeDeleteURI: function(aURI) {},
     onDeleteURI: function(aURI) {},
     onClearHistory: function() {},
     onPageChanged: function(aURI, aWhat, aValue) {},

toolkit/components/places/tests/browser/browser_bug413985-window.location.href.js

@@ -56,6 +56,7 @@ function test()
       finish();
     },
     onTitleChanged: function(aURI, aPageTitle) {},
+    onBeforeDeleteURI: function(aURI) {},
     onDeleteURI: function(aURI) {},
     onClearHistory: function() {},
     onPageChanged: function(aURI, aWhat, aValue) {},

toolkit/components/places/tests/browser/browser_bug413985-window.location.js

@@ -56,6 +56,7 @@ function test()
       finish();
     },
     onTitleChanged: function(aURI, aPageTitle) {},
+    onBeforeDeleteURI: function(aURI) {},
     onDeleteURI: function(aURI) {},
     onClearHistory: function() {},
     onPageChanged: function(aURI, aWhat, aValue) {},

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

@@ -62,6 +62,7 @@ var observer = {
     confirm_results();
   },
   onTitleChanged: function(aURI, aPageTitle) {},
+  onBeforeDeleteURI: function(aURI) {},
   onDeleteURI: function(aURI) {},
   onClearHistory: function() {},
   onPageChanged: function(aURI, aWhat, aValue) {},

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

@@ -65,6 +65,8 @@ var observer = {
   },
   onTitleChanged: function(aURI, aPageTitle) {
   },
+  onBeforeDeleteURI: function(aURI) {
+  },
   onDeleteURI: function(aURI) {
   },
   onClearHistory: function() {

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

@@ -82,6 +82,8 @@ let observer = {
   },
   onTitleChanged: function(aURI, aPageTitle) {
   },
+  onBeforeDeleteURI: function(aURI) {
+  },
   onDeleteURI: function(aURI) {
   },
 

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

@@ -102,6 +102,8 @@ var observer = {
   },
   onTitleChanged: function(aURI, aPageTitle) {
   },
+  onBeforeDeleteURI: function(aURI) {
+  },
   onDeleteURI: function(aURI) {
   },
   onClearHistory: function() {

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

@@ -0,0 +1,125 @@
+/* -*- Mode: Java; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim:set ts=2 sw=2 sts=2 et: */
+/* ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is mozilla.org code.
+ *
+ * The Initial Developer of the Original Code is
+ * Mozilla Corporation.
+ * Portions created by the Initial Developer are Copyright (C) 2009
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Contributor(s):
+ *   Shawn Wilsher <me@shawnwilsher.com> (Original Author)
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+/**
+ * Added with bug 487511 to test that onBeforeDeleteURI is dispatched before
+ * onDeleteURI and always with the right uri.
+ */
+
+////////////////////////////////////////////////////////////////////////////////
+//// Globals and Constants
+
+let hs = Cc["@mozilla.org/browser/nav-history-service;1"].
+         getService(Ci.nsINavHistoryService);
+
+////////////////////////////////////////////////////////////////////////////////
+//// Observer
+
+function Observer()
+{
+}
+Observer.prototype =
+{
+  checked: false,
+  onBeginUpdateBatch: function() {
+  },
+  onEndUpdateBatch: function() {
+  },
+  onVisit: function(aURI, aVisitID, aTime, aSessionId, aReferringId,
+                    aTransitionType, _added)
+  {
+  },
+  onTitleChanged: function(aURI, aPageTable)
+  {
+  },
+  onBeforeDeleteURI: function(aURI)
+  {
+    this.removedURI = aURI;
+  },
+  onDeleteURI: function(aURI)
+  {
+    do_check_false(this.checked);
+    do_check_true(this.removedURI.equals(aURI));
+    this.checked = true;
+  },
+  onPageChanged: function(aURI, aWhat, aValue)
+  {
+  },
+  onPageExpired: function(aURI, aVisitTime, aWholeEntry)
+  {
+  },
+  QueryInterface: function(iid) {
+    if (iid.equals(Ci.nsINavHistoryObserver) ||
+        iid.equals(Ci.nsISupports)) {
+      return this;
+    }
+    throw Cr.NS_ERROR_NO_INTERFACE;
+  }
+};
+
+////////////////////////////////////////////////////////////////////////////////
+//// Test Functions
+
+function test_removePage()
+{
+  // First we add the URI to history that we are going to remove.
+  let testURI = uri("http://mozilla.org");
+  hs.addVisit(testURI, Date.now() * 1000, null,
+              Ci.nsINavHistoryService.TRANSITION_LINK, false, 0);
+
+  // Add our observer, and remove it.
+  let observer = new Observer();
+  hs.addObserver(observer, false);
+  hs.removePage(testURI);
+
+  // Make sure we were notified!
+  do_check_true(observer.checked);
+  hs.removeObserver(observer);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+//// Test Runner
+
+let tests = [
+  test_removePage,
+];
+function run_test()
+{
+  tests.forEach(function(test) test());
+}