From 93af15cb716bd6e13f04ee037795cd3ab6f473f8 Mon Sep 17 00:00:00 2001 From: Shawn Wilsher Date: Fri, 10 Apr 2009 13:51:40 -0400 Subject: [PATCH] Bug 487511 - nsINavHistoryObserver has no "onBeforeDeleteURI" callback r=dietrich --- browser/components/feeds/src/FeedWriter.js | 1 + .../downloads/src/nsDownloadManager.cpp | 6 + .../places/public/nsINavHistoryService.idl | 11 +- .../components/places/src/nsNavBookmarks.cpp | 6 + .../components/places/src/nsNavHistory.cpp | 7 +- .../places/src/nsNavHistoryResult.cpp | 14 ++ .../places/src/nsNavHistoryResult.h | 1 + .../browser/browser_bug413985-history.go-0.js | 1 + .../browser/browser_bug413985-httprefresh.js | 1 + .../browser_bug413985-location.reload.js | 1 + .../browser_bug413985-location.replace.js | 1 + .../browser_bug413985-window.location.href.js | 1 + .../browser_bug413985-window.location.js | 1 + .../places/tests/unit/test_399606.js | 1 + .../places/tests/unit/test_expiration.js | 2 + .../tests/unit/test_history_removeAllPages.js | 2 + .../places/tests/unit/test_markpageas.js | 2 + .../unit/test_onBeforeDeleteURI_observer.js | 125 ++++++++++++++++++ 18 files changed, 182 insertions(+), 2 deletions(-) create mode 100644 toolkit/components/places/tests/unit/test_onBeforeDeleteURI_observer.js diff --git a/browser/components/feeds/src/FeedWriter.js b/browser/components/feeds/src/FeedWriter.js index 26aadd49eda..769e7696a7c 100644 --- a/browser/components/feeds/src/FeedWriter.js +++ b/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() { }, diff --git a/toolkit/components/downloads/src/nsDownloadManager.cpp b/toolkit/components/downloads/src/nsDownloadManager.cpp index 7d73ad01c65..55828140475 100644 --- a/toolkit/components/downloads/src/nsDownloadManager.cpp +++ b/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) { diff --git a/toolkit/components/places/public/nsINavHistoryService.idl b/toolkit/components/places/public/nsINavHistoryService.idl index 7b0498ac220..e4975230b0a 100644 --- a/toolkit/components/places/public/nsINavHistoryService.idl +++ b/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. diff --git a/toolkit/components/places/src/nsNavBookmarks.cpp b/toolkit/components/places/src/nsNavBookmarks.cpp index 12bf7b1c50b..b98d56611a9 100644 --- a/toolkit/components/places/src/nsNavBookmarks.cpp +++ b/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) { diff --git a/toolkit/components/places/src/nsNavHistory.cpp b/toolkit/components/places/src/nsNavHistory.cpp index ce77dca1032..6bbc0308b47 100644 --- a/toolkit/components/places/src/nsNavHistory.cpp +++ b/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; } diff --git a/toolkit/components/places/src/nsNavHistoryResult.cpp b/toolkit/components/places/src/nsNavHistoryResult.cpp index 6d44c3baa36..dea08acd59a 100644 --- a/toolkit/components/places/src/nsNavHistoryResult.cpp +++ b/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 diff --git a/toolkit/components/places/src/nsNavHistoryResult.h b/toolkit/components/places/src/nsNavHistoryResult.h index 55ca8fad403..13159f36e8f 100644 --- a/toolkit/components/places/src/nsNavHistoryResult.h +++ b/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, \ diff --git a/toolkit/components/places/tests/browser/browser_bug413985-history.go-0.js b/toolkit/components/places/tests/browser/browser_bug413985-history.go-0.js index 3fe7e747aba..c417a160b96 100644 --- a/toolkit/components/places/tests/browser/browser_bug413985-history.go-0.js +++ b/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) {}, diff --git a/toolkit/components/places/tests/browser/browser_bug413985-httprefresh.js b/toolkit/components/places/tests/browser/browser_bug413985-httprefresh.js index 4d5b3c091a9..2b7ed63eff3 100644 --- a/toolkit/components/places/tests/browser/browser_bug413985-httprefresh.js +++ b/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) {}, diff --git a/toolkit/components/places/tests/browser/browser_bug413985-location.reload.js b/toolkit/components/places/tests/browser/browser_bug413985-location.reload.js index afd4228fcdb..5208940cb70 100644 --- a/toolkit/components/places/tests/browser/browser_bug413985-location.reload.js +++ b/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) {}, diff --git a/toolkit/components/places/tests/browser/browser_bug413985-location.replace.js b/toolkit/components/places/tests/browser/browser_bug413985-location.replace.js index 632670ff411..58997762a1f 100644 --- a/toolkit/components/places/tests/browser/browser_bug413985-location.replace.js +++ b/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) {}, diff --git a/toolkit/components/places/tests/browser/browser_bug413985-window.location.href.js b/toolkit/components/places/tests/browser/browser_bug413985-window.location.href.js index 7d753e00662..2f7b8a137ad 100644 --- a/toolkit/components/places/tests/browser/browser_bug413985-window.location.href.js +++ b/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) {}, diff --git a/toolkit/components/places/tests/browser/browser_bug413985-window.location.js b/toolkit/components/places/tests/browser/browser_bug413985-window.location.js index de5584417b2..dd311f7486c 100644 --- a/toolkit/components/places/tests/browser/browser_bug413985-window.location.js +++ b/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) {}, diff --git a/toolkit/components/places/tests/unit/test_399606.js b/toolkit/components/places/tests/unit/test_399606.js index a8a3488b369..b9644d79859 100644 --- a/toolkit/components/places/tests/unit/test_399606.js +++ b/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) {}, diff --git a/toolkit/components/places/tests/unit/test_expiration.js b/toolkit/components/places/tests/unit/test_expiration.js index 2a8be6a3059..3657f040aab 100644 --- a/toolkit/components/places/tests/unit/test_expiration.js +++ b/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() { diff --git a/toolkit/components/places/tests/unit/test_history_removeAllPages.js b/toolkit/components/places/tests/unit/test_history_removeAllPages.js index 8916acfc6f0..4ee28d5a0b5 100644 --- a/toolkit/components/places/tests/unit/test_history_removeAllPages.js +++ b/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) { }, diff --git a/toolkit/components/places/tests/unit/test_markpageas.js b/toolkit/components/places/tests/unit/test_markpageas.js index d22b414c5cb..7ed204607c8 100644 --- a/toolkit/components/places/tests/unit/test_markpageas.js +++ b/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() { diff --git a/toolkit/components/places/tests/unit/test_onBeforeDeleteURI_observer.js b/toolkit/components/places/tests/unit/test_onBeforeDeleteURI_observer.js new file mode 100644 index 00000000000..eb764920b77 --- /dev/null +++ b/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 (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()); +}