Bug 520165 - Part8: Change onPageExpired to onDeleteVisits, r=mano, sr=mconnor

2010-01-15 17:40:17 +01:00 · 2010-01-15 17:40:17 +01:00 · 04fa14d307
--- a/browser/components/feeds/src/FeedWriter.js
+++ b/browser/components/feeds/src/FeedWriter.js
@ -1415,7 +1415,7 @@ FeedWriter.prototype = {
   onBeforeDeleteURI: function() { },
   onDeleteURI: function() { },
   onClearHistory: function() { },
-   onPageExpired: function() { },
+   onDeleteVisits: function() { },

  // nsIClassInfo
  getInterfaces: function FW_getInterfaces(countRef) {
--- a/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_placestitle.js
+++ b/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_placestitle.js
@ -109,7 +109,7 @@ function test() {
    },
    onPageChanged: function(aURI, aWhat, aValue) {
    },
-    onPageExpired: function(aURI, aVisitTime, aWholeEntry) {
+    onDeleteVisits: function() {
    },
    QueryInterface: function(iid) {
      if (iid.equals(Ci.nsINavHistoryObserver) ||
--- a/toolkit/components/downloads/src/nsDownloadManager.cpp
+++ b/toolkit/components/downloads/src/nsDownloadManager.cpp
@ -1851,14 +1851,10 @@ nsDownloadManager::OnPageChanged(nsIURI *aURI, PRUint32 aWhat,
 }

 NS_IMETHODIMP
-nsDownloadManager::OnPageExpired(nsIURI *aURI, PRTime aVisitTime,
-                                 PRBool aWholeEntry)
+nsDownloadManager::OnDeleteVisits(nsIURI *aURI, PRTime aVisitTime)
 {
-  // Don't bother removing downloads if there are still recent visits
-  if (!aWholeEntry)
-    return NS_OK;
-
-  return RemoveDownloadsForURI(aURI);
+  // Don't bother removing downloads until the page is removed.
+  return NS_OK;
 }

 ////////////////////////////////////////////////////////////////////////////////
--- a/toolkit/components/places/public/nsINavHistoryService.idl
+++ b/toolkit/components/places/public/nsINavHistoryService.idl
@ -726,7 +726,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(14065711-8a91-4d96-ba32-59512f5401b6)]
+[scriptable, uuid(0a5ce210-c803-11de-8a39-0800200c9a66)]
 interface nsINavHistoryObserver : nsISupports
 {
  /**
@ -814,18 +814,20 @@ interface nsINavHistoryObserver : nsISupports
  void onPageChanged(in nsIURI aURI, in unsigned long aWhat, in AString aValue);

  /**
-   * Called when a history entry expires. You will receive notifications that
-   * a specific visit has expired with the time of that visit. When the last
-   * visit for a history entry expires, the history entry itself is deleted
-   * and aWholeEntry is set. (If your observer only cares about URLs and not
-   * specific visits, it needs only to listen for aWholeEntry notifications).
+   * Called when some visits of an history entry are expired.
   *
-   * It is possible for a history entry to be deleted that has no visits if
-   * something is out of sync or after a bookmark is deleted that has no
-   * visits (thus freeing the history entry). In these cases, aVisitTime will
-   * be 0.
+   * @param aURI
+   *        The page whose visits have been expired.
+   * @param aVisitTime
+   *        The largest visit time in microseconds that has been expired.  We
+   *        guarantee that we don't have any visit older than this date.
+   *
+   * @note: when all visits for a page are expired and also the full page entry
+   *        is expired, you will only get an onDeleteURI notification.  If a
+   *        page entry is removed, then you can be sure that we don't have
+   *        anymore visits for it.
   */
-  void onPageExpired(in nsIURI aURI, in PRTime aVisitTime, in boolean aWholeEntry);
+  void onDeleteVisits(in nsIURI aURI, in PRTime aVisitTime);
 };


--- a/toolkit/components/places/public/nsPIPlacesHistoryListenersNotifier.idl
+++ b/toolkit/components/places/public/nsPIPlacesHistoryListenersNotifier.idl
@ -52,7 +52,8 @@ interface nsIURI;
 interface nsPIPlacesHistoryListenersNotifier : nsISupports
 {
  /**
-   * Calls onPageExpired on registered listeners with the history service.
+   * Calls onDeleteVisits and onDeleteURI notifications on registered listeners
+   * with the history service.
   *
   * @param aURI
   *        The nsIURI object representing the URI of the page being expired.
--- a/toolkit/components/places/src/nsNavBookmarks.cpp
+++ b/toolkit/components/places/src/nsNavBookmarks.cpp
@ -3181,8 +3181,7 @@ nsNavBookmarks::OnPageChanged(nsIURI *aURI, PRUint32 aWhat,
 }

 NS_IMETHODIMP
-nsNavBookmarks::OnPageExpired(nsIURI* aURI, PRTime aVisitTime,
-                              PRBool aWholeEntry)
+nsNavBookmarks::OnDeleteVisits(nsIURI* aURI, PRTime aVisitTime)
 {
  // pages that are bookmarks shouldn't expire, so we don't need to handle it
  return NS_OK;
--- a/toolkit/components/places/src/nsNavHistory.cpp
+++ b/toolkit/components/places/src/nsNavHistory.cpp
@ -5493,13 +5493,17 @@ NS_IMETHODIMP
 nsNavHistory::NotifyOnPageExpired(nsIURI *aURI, PRTime aVisitTime,
                                  PRBool aWholeEntry)
 {
-  ENUMERATE_OBSERVERS(mCanNotify, mCacheObservers, mObservers, nsINavHistoryObserver,
-                      OnPageExpired(aURI, aVisitTime, aWholeEntry));
  if (aWholeEntry) {
-    // Notify our observers that the URI has been removed.
+    // Notify our observers that the page has been removed.
    ENUMERATE_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
                        nsINavHistoryObserver, OnDeleteURI(aURI));
  }
+  else {
+    // Notify our observers that some visits for the page have been removed.
+    ENUMERATE_OBSERVERS(mCanNotify, mCacheObservers, mObservers,
+                        nsINavHistoryObserver,
+                        OnDeleteVisits(aURI, aVisitTime));
+  }

  return NS_OK;
 }
--- a/toolkit/components/places/src/nsNavHistoryExpire.cpp
+++ b/toolkit/components/places/src/nsNavHistoryExpire.cpp
@ -336,8 +336,7 @@ nsNavHistoryExpire::ExpireItems(PRUint32 aNumToExpire)

    ENUMERATE_OBSERVERS(mHistory->canNotify(), mHistory->mCacheObservers,
                        mHistory->mObservers, nsINavHistoryObserver,
-                        OnPageExpired(uri, expiredVisits[i].visitDate,
-                                      expiredVisits[i].erased));
+                        OnDeleteVisits(uri, expiredVisits[i].visitDate));
  }

  // Don't worry about errors here, it doesn't affect our ability to continue.
--- a/toolkit/components/places/src/nsNavHistoryResult.cpp
+++ b/toolkit/components/places/src/nsNavHistoryResult.cpp
@ -3000,14 +3000,13 @@ nsNavHistoryQueryResultNode::OnPageChanged(nsIURI *aURI, PRUint32 aWhat,
 }


-// nsNavHistoryQueryResultNode::OnPageExpired
+// nsNavHistoryQueryResultNode::OnDeleteVisits
 //
 //    Do nothing. Perhaps we want to handle this case. If so, add the call to
 //    the result to enumerate the history observers.

 NS_IMETHODIMP
-nsNavHistoryQueryResultNode::OnPageExpired(nsIURI* aURI, PRTime aVisitTime,
-                                           PRBool aWholeEntry)
+nsNavHistoryQueryResultNode::OnDeleteVisits(nsIURI* aURI, PRTime aVisitTime)
 {
  return NS_OK;
 }
@ -4635,14 +4634,12 @@ nsNavHistoryResult::OnPageChanged(nsIURI *aURI,
 }


-// nsNavHistoryResult;:OnPageExpired (nsINavHistoryObserver)
+// nsNavHistoryResult::OnDeleteVisits (nsINavHistoryObserver)
 //
-//    Don't do anything when pages expire. Perhaps we want to find the item
-//    to delete it.
+//    Don't do anything when visits expire.

 NS_IMETHODIMP
-nsNavHistoryResult::OnPageExpired(nsIURI* aURI, PRTime aVisitTime,
-                                  PRBool aWholeEntry)
+nsNavHistoryResult::OnDeleteVisits(nsIURI* aURI, PRTime aVisitTime)
 {
  return NS_OK;
 }
--- a/toolkit/components/places/src/nsNavHistoryResult.h
+++ b/toolkit/components/places/src/nsNavHistoryResult.h
@ -103,8 +103,7 @@ private:
  NS_IMETHOD OnClearHistory();                                          \
  NS_IMETHOD OnPageChanged(nsIURI *aURI, PRUint32 aWhat,                \
                           const nsAString &aValue);                    \
-  NS_IMETHOD OnPageExpired(nsIURI* aURI, PRTime aVisitTime,             \
-                           PRBool aWholeEntry);
+  NS_IMETHOD OnDeleteVisits(nsIURI* aURI, PRTime aVisitTime);

 // nsNavHistoryResult
 //
--- a/toolkit/components/places/src/nsPlacesDBFlush.js
+++ b/toolkit/components/places/src/nsPlacesDBFlush.js
@ -268,13 +268,13 @@ nsPlacesDBFlush.prototype = {
  // the implementations can be found above.
  //onBeginUpdateBatch: function() { },
  //onEndUpdateBatch: function() { },
-  onVisit: function(aURI, aVisitID, aTime, aSessionID, aReferringID, aTransitionType) { },
-  onTitleChanged: function(aURI, aPageTitle) { },
-  onBeforeDeleteURI: function(aURI) { },
-  onDeleteURI: function(aURI) { },
+  onVisit: function() { },
+  onTitleChanged: function() { },
+  onBeforeDeleteURI: function() { },
+  onDeleteURI: function() { },
  onClearHistory: function() { },
-  onPageChanged: function(aURI, aWhat, aValue) { },
-  onPageExpired: function(aURI, aVisitTime, aWholeEntry) { },
+  onPageChanged: function() { },
+  onDeleteVisits: function() { },

  //////////////////////////////////////////////////////////////////////////////
  //// nsITimerCallback
--- a/toolkit/components/places/tests/browser/browser_bug399606.js
+++ b/toolkit/components/places/tests/browser/browser_bug399606.js
@ -70,7 +70,7 @@ function test() {
    onDeleteURI: function(aURI) {},
    onClearHistory: function() {},
    onPageChanged: function(aURI, aWhat, aValue) {},
-    onPageExpired: function(aURI, aVisitTime, aWholeEntry) {},
+    onDeleteVisits: function() {},
    QueryInterface: XPCOMUtils.generateQI([Ci.nsINavHistoryObserver])
  };
  hs.addObserver(historyObserver, false);
--- a/toolkit/components/places/tests/sync/test_expire_on_timed_flush.js
+++ b/toolkit/components/places/tests/sync/test_expire_on_timed_flush.js
@ -85,19 +85,13 @@ os.addObserver(observer, kSyncFinished, false);
 var historyObserver = {
  visitTime: -1,
  _runCount: 0,
-  onPageExpired: function(aURI, aVisitTime, aWholeEntry)
+  onDeleteVisits: function(aURI, aVisitTime)
  {
    do_check_true(aURI.equals(uri(TEST_URI)));

    // We should have the visit time that was set in the addVisit call.
    do_check_eq(this.visitTime, aVisitTime);

-    // This was the only visit for this uri, so ensure that aWholeEntry is true.
-    if (++this._runCount == 1)
-      do_check_false(aWholeEntry);
-    else
-      do_check_true(aWholeEntry);
-
    observer.notificationReceived = true;
    hs.removeObserver(this, false);
  }
--- a/toolkit/components/places/tests/sync/test_expire_on_timed_flush_not_whole_entry.js
+++ b/toolkit/components/places/tests/sync/test_expire_on_timed_flush_not_whole_entry.js
@ -87,16 +87,13 @@ os.addObserver(observer, kSyncFinished, false);
 // Used to ensure that we did in fact get notified about our expiration.
 var historyObserver = {
  visitTime: -1,
-  onPageExpired: function(aURI, aVisitTime, aWholeEntry)
+  onDeleteVisits: function(aURI, aVisitTime)
  {
    do_check_true(aURI.equals(uri(TEST_URI)));

    // We should have the visit time that was set in the addVisit call.
    do_check_eq(this.visitTime, aVisitTime);

-    // This was not the only visit, so aWholeEntry better be false!
-    do_check_false(aWholeEntry);
-
    // We should only be called once.
    do_check_false(observer.notificationReceived);
    observer.notificationReceived = true;
--- a/toolkit/components/places/tests/sync/test_expire_on_timed_flush_obeys_date.js
+++ b/toolkit/components/places/tests/sync/test_expire_on_timed_flush_obeys_date.js
@ -85,7 +85,7 @@ os.addObserver(observer, kSyncFinished, false);

 // Used to ensure that we do not expire anything.
 var historyObserver = {
-  onPageExpired: function(aURI, aVisitTime, aWholeEntry)
+  onDeleteVisits: function(aURI, aVisitTime)
  {
    do_throw("How did we get called?!");
    observer.notificationReceived = true;
--- a/toolkit/components/places/tests/unit/nsDummyObserver.js
+++ b/toolkit/components/places/tests/unit/nsDummyObserver.js
@ -60,7 +60,7 @@ DummyObserver.prototype = {
  onDeleteURI: function(aURI) {},
  onClearHistory: function() {},
  onPageChanged: function(aURI, aWhat, aValue) {},
-  onPageExpired: function(aURI, aVisitTime, aWholeEntry) {},
+  onDeleteVisits: function(aURI, aVisitTime) {},

  // bookmark observer
  //onBeginUpdateBatch: function() {},
--- a/toolkit/components/places/tests/unit/test_399606.js
+++ b/toolkit/components/places/tests/unit/test_399606.js
@ -66,7 +66,7 @@ var observer = {
  onDeleteURI: function(aURI) {},
  onClearHistory: function() {},
  onPageChanged: function(aURI, aWhat, aValue) {},
-  onPageExpired: function(aURI, aVisitTime, aWholeEntry) {},
+  onDeleteVisits: function() {},
  QueryInterface: function(iid) {
    if (iid.equals(Ci.nsINavHistoryObserver) ||
        iid.equals(Ci.nsISupports)) {
--- a/toolkit/components/places/tests/unit/test_doSetAndLoadFaviconForPage.js
+++ b/toolkit/components/places/tests/unit/test_doSetAndLoadFaviconForPage.js
@ -184,7 +184,7 @@ var historyObserver = {
  onBeforeDeleteURI: function() {},
  onDeleteURI: function() {},
  onClearHistory: function() {},
-  onPageExpired: function() {},
+  onDeleteVisits: function() {},

  onPageChanged: function historyObserver_onPageChanged(pageURI, what, value) {
    if (what != Ci.nsINavHistoryObserver.ATTRIBUTE_FAVICON)
--- a/toolkit/components/places/tests/unit/test_expiration.js
+++ b/toolkit/components/places/tests/unit/test_expiration.js
@ -72,7 +72,7 @@ var observer = {
  onPageChanged: function(aURI, aWhat, aValue) {
  },
  expiredURI: null,
-  onPageExpired: function(aURI, aVisitTime, aWholeEntry) {
+  onDeleteVisits: function(aURI, aVisitTime) {
    this.expiredURI = aURI.spec;
  },
  QueryInterface: function(iid) {
@ -502,7 +502,7 @@ steps:
  - kick off incremental expiration

 confirmation:
-  - check onPageExpired, confirm nothing was expired
+  - check onDeleteVisits, confirm nothing was expired
  - query for the visit, confirm it's there

 */
@ -561,7 +561,7 @@ steps:
  - kick off incremental expiration

 confirmation:
-  - check onPageExpired, confirm that the expirable uri was expired
+  - check onDeleteVisits, confirm that the expirable uri was expired
  - query for the visit, confirm it's there
 */
 function startExpireDaysOnly() {
@ -624,7 +624,7 @@ steps:
  - kick off incremental expiration

 confirmation:
-  - check onPageExpired, confirm our oldest visit was expired
+  - check onDeleteVisits, confirm our oldest visit was expired
  - query for the oldest visit, confirm it's not there
 */
 function startExpireBoth() {
@ -690,7 +690,7 @@ steps:
  - kick off incremental expiration

 confirmation:
-  - check onPageExpired, confirm nothing was expired
+  - check onDeleteVisits, confirm nothing was expired
  - query for the visit, confirm it's there

 */
@ -746,7 +746,7 @@ steps:
  - kick off incremental expiration

 confirmation:
-  - check onPageExpired, confirm visit was expired
+  - check onDeleteVisits, confirm visit was expired
  - query for the visit, confirm it's not there

 */
@ -795,7 +795,7 @@ steps:
  - kick off incremental expiration

 confirmation:
-  - check onPageExpired, confirm nothing was expired
+  - check onDeleteVisits, confirm nothing was expired
  - query for the visit, confirm it's there

 */
--- a/toolkit/components/places/tests/unit/test_history_removeAllPages.js
+++ b/toolkit/components/places/tests/unit/test_history_removeAllPages.js
@ -125,7 +125,7 @@ let observer = {

  onPageChanged: function(aURI, aWhat, aValue) {
  },
-  onPageExpired: function(aURI, aVisitTime, aWholeEntry) {
+  onDeleteVisits: function() {
  },

  QueryInterface: function(iid) {
--- a/toolkit/components/places/tests/unit/test_markpageas.js
+++ b/toolkit/components/places/tests/unit/test_markpageas.js
@ -110,7 +110,7 @@ var observer = {
  },
  onPageChanged: function(aURI, aWhat, aValue) {
  },
-  onPageExpired: function(aURI, aVisitTime, aWholeEntry) {
+  onDeleteVisits: function() {
  },
  QueryInterface: function(iid) {
    if (iid.equals(Ci.nsINavHistoryObserver) ||
--- a/toolkit/components/places/tests/unit/test_onBeforeDeleteURI_observer.js
+++ b/toolkit/components/places/tests/unit/test_onBeforeDeleteURI_observer.js
@ -81,7 +81,7 @@ Observer.prototype =
  onPageChanged: function(aURI, aWhat, aValue)
  {
  },
-  onPageExpired: function(aURI, aVisitTime, aWholeEntry)
+  onDeleteVisits: function()
  {
  },
  QueryInterface: function(iid) {