Bug 1490158 - Minimize cancelability in nsISHistoryListener. r=nika

nsISHistoryListener can cancel several operations, but the functionality is
only ever used for OnHistoryReload(). So this patch removes it for the other
operations.

--HG--
extra : rebase_source : 433422e9160f7d645570baaaff4779c4bcc3ec04

browser/components/sessionstore/ContentRestore.jsm

@@ -381,8 +381,8 @@ HistoryListener.prototype = {
     }
   },
 
-  OnHistoryGotoIndex(index, gotoURI) { return true; },
-  OnHistoryPurge(numEntries) { return true; },
+  OnHistoryGotoIndex(index, gotoURI) {},
+  OnHistoryPurge(numEntries) {},
   OnHistoryReplaceEntry(index) {},
 
   // This will be called for a pending tab when loadURI(uri) is called where

browser/components/sessionstore/ContentSessionStore.jsm

@@ -301,12 +301,10 @@ class SessionHistoryListener extends Handler {
   OnHistoryGotoIndex(index, gotoURI) {
     // We ought to collect the previously current entry as well, see bug 1350567.
     this.collectFrom(kLastIndex);
-    return true;
   }
 
   OnHistoryPurge(numEntries) {
     this.collect();
-    return true;
   }
 
   OnHistoryReload(reloadURI, reloadFlags) {

browser/components/sessionstore/test/content.js

@@ -19,12 +19,10 @@ var historyListener = {
 
   OnHistoryGotoIndex() {
     sendAsyncMessage("ss-test:OnHistoryGotoIndex");
-    return true;
   },
 
   OnHistoryPurge() {
     sendAsyncMessage("ss-test:OnHistoryPurge");
-    return true;
   },
 
   OnHistoryReload() {

docshell/shistory/nsISHistoryListener.idl

@@ -10,19 +10,18 @@ interface nsIURI;
 
 /**
  * nsISHistoryListener defines the interface one can implement to receive
- * notifications about activities in session history and to be able to
- * cancel them.
+ * notifications about activities in session history and (for reloads) to be
+ * able to cancel them.
  *
  * A session history listener will be notified when pages are added, removed
- * and loaded from session history. It can prevent any action (except adding
- * a new session history entry) from happening by returning false from the
- * corresponding callback method.
+ * and loaded from session history. In the case of reloads, it can prevent them
+ * from happening by returning false from the corresponding callback method.
  *
  * A session history listener can be registered on a particular nsISHistory
  * instance via the nsISHistory::addSHistoryListener() method.
  */
 [scriptable, uuid(125c0833-746a-400e-9b89-d2d18545c08a)]
-interface nsISHistoryListener : nsISupports 
+interface nsISHistoryListener : nsISupports
 {
   /**
    * Called when a new document is added to session history. New documents are
@@ -30,71 +29,68 @@ interface nsISHistoryListener : nsISupports
    * or content area, for example via nsIWebNavigation::loadURI()
    *
    * @param aNewURI     The URI of the document to be added to session history.
-   * @param aOldIndex   The index of the current history item before the operation.
+   * @param aOldIndex   The index of the current history item before the
+   *                    operation.
    */
-   void OnHistoryNewEntry(in nsIURI aNewURI, in long aOldIndex);
+  void OnHistoryNewEntry(in nsIURI aNewURI, in long aOldIndex);
 
-  /** 
-   * Called when the current document is reloaded, for example due to a
+  /**
+   * Called before the current document is reloaded, for example due to a
    * nsIWebNavigation::reload() call.
    *
    * @param aReloadURI    The URI of the document to be reloaded.
-   * @param aReloadFlags  Flags that indicate how the document is to be 
+   * @param aReloadFlags  Flags that indicate how the document is to be
    *                      refreshed. See constants on the nsIWebNavigation
    *                      interface.
    * @return              Whether the operation can proceed.
    *
    * @see  nsIWebNavigation
    */
-   boolean OnHistoryReload(in nsIURI aReloadURI, in unsigned long aReloadFlags);
+  boolean OnHistoryReload(in nsIURI aReloadURI, in unsigned long aReloadFlags);
 
   /**
-   * Called when navigating to a session history entry by index, for example,
+   * Called before navigating to a session history entry by index, for example,
    * when nsIWebNavigation::gotoIndex() is called.
    *
-   * @param aIndex        The index in session history of the entry to be loaded.
+   * @param aIndex        The index in session history of the entry to be
+   *                      loaded.
    * @param aGotoURI      The URI of the session history entry to be loaded.
    *                      It could be null in case of a grouped session history
    *                      navigation since we have no URI information of entries
    *                      existing in other partial histories.
-   * @return              Whether the operation can proceed.
    */
-   boolean OnHistoryGotoIndex(in long aIndex, in nsIURI aGotoURI);
+  void OnHistoryGotoIndex(in long aIndex, in nsIURI aGotoURI);
 
   /**
-   * Called when entries are removed from session history. Entries can be
+   * Called before entries are removed from session history. Entries can be
    * removed from session history for various reasons, for example to control
    * the memory usage of the browser, to prevent users from loading documents
    * from history, to erase evidence of prior page loads, etc.
    *
-   * To purge documents from session history call nsISHistory::PurgeHistory()
+   * To purge documents from session history call nsISHistory::PurgeHistory().
    *
-   * @param aNumEntries   The number of entries to be removed from session history.
-   * @return              Whether the operation can proceed.
+   * @param aNumEntries   The number of entries to be removed from session
+   *                      history.
    */
-   boolean OnHistoryPurge(in long aNumEntries);
+  void OnHistoryPurge(in long aNumEntries);
 
   /**
-   * Called when an entry is replaced in the session history. Entries are
+   * Called before an entry is replaced in the session history. Entries are
    * replaced when navigating away from non-persistent history entries (such as
    * about pages) and when history.replaceState is called.
    *
    * @param aIndex        The index in session history of the entry being
-  *                       replaced
+   *                      replaced.
    */
-   void OnHistoryReplaceEntry(in long aIndex);
+  void OnHistoryReplaceEntry(in long aIndex);
 
    /**
-    * Called when nsISHistory::count has been updated. Unlike OnHistoryNewEntry
-    * and OnHistoryPurge which happen before the modifications are actually done
-    * and maybe cancellable, this function is called after these modifications.
+    * Called after nsISHistory::count has been updated.
     */
-   void OnLengthChanged(in long aCount);
+  void OnLengthChanged(in long aCount);
 
   /**
-   * Called when nsISHistory::index has been updated. Unlike the other methods
-   * on this interface, which happen before the modifications are actually done
-   * and maybe cancellable, this function is called after these modifications.
+   * Called after nsISHistory::index has been updated.
    */
-   void OnIndexChanged(in long aIndex);
+  void OnIndexChanged(in long aIndex);
 };

docshell/shistory/nsSHistory.cpp

@@ -796,14 +796,7 @@ nsSHistory::PurgeHistory(int32_t aNumEntries)
 
   aNumEntries = std::min(aNumEntries, Length());
 
-  bool purgeHistory = true;
-  NOTIFY_LISTENERS_CANCELABLE(OnHistoryPurge, purgeHistory,
-                              (aNumEntries, &purgeHistory));
-
-  if (!purgeHistory) {
-    // Listener asked us not to purge
-    return NS_SUCCESS_LOSS_OF_INSIGNIFICANT_DATA;
-  }
+  NOTIFY_LISTENERS(OnHistoryPurge, (aNumEntries));
 
   // Remove the first `aNumEntries` entries.
   mEntries.RemoveElementsAt(0, aNumEntries);
@@ -952,14 +945,9 @@ NS_IMETHODIMP
 nsSHistory::ReloadCurrentEntry()
 {
   // Notify listeners
-  bool canNavigate = true;
   nsCOMPtr<nsIURI> currentURI;
   GetCurrentURI(getter_AddRefs(currentURI));
-  NOTIFY_LISTENERS_CANCELABLE(OnHistoryGotoIndex, canNavigate,
-                              (mIndex, currentURI, &canNavigate));
-  if (!canNavigate) {
-    return NS_OK;
-  }
+  NOTIFY_LISTENERS(OnHistoryGotoIndex, (mIndex, currentURI));
 
   return LoadEntry(mIndex, LOAD_HISTORY, HIST_CMD_RELOAD);
 }
@@ -1529,18 +1517,9 @@ nsSHistory::LoadEntry(int32_t aIndex, long aLoadType, uint32_t aHistCmd)
   MOZ_ASSERT((prevEntry && nextEntry && nextURI), "prevEntry, nextEntry and nextURI can't be null");
 
   // Send appropriate listener notifications.
-  bool canNavigate = true;
   if (aHistCmd == HIST_CMD_GOTOINDEX) {
     // We are going somewhere else. This is not reload either
-    NOTIFY_LISTENERS_CANCELABLE(OnHistoryGotoIndex, canNavigate,
-                                (aIndex, nextURI, &canNavigate));
-  }
-
-  if (!canNavigate) {
-    // If the listener asked us not to proceed with
-    // the operation, simply return.
-    mRequestedIndex = -1;
-    return NS_OK;  // XXX Maybe I can return some other error code?
+    NOTIFY_LISTENERS(OnHistoryGotoIndex, (aIndex, nextURI));
   }
 
   if (mRequestedIndex == mIndex) {

docshell/test/browser/browser_bug670318.js

@@ -34,19 +34,16 @@ add_task(async function test() {
             delete content._testListener;
             content.setTimeout(() => { content.location.reload(); }, 0);
           }
-
-          return true;
         },
 
         OnHistoryReload: () => true,
-        OnHistoryGotoIndex: () => true,
-        OnHistoryPurge: () => true,
+        OnHistoryGotoIndex: () => {},
+        OnHistoryPurge: () => {},
         OnHistoryReplaceEntry: () => {
           // The initial load of about:blank causes a transient entry to be
           // created, so our first navigation to a real page is a replace
           // instead of a new entry.
           ++count;
-          return true;
         },
 
         QueryInterface: ChromeUtils.generateQI([Ci.nsISHistoryListener,

docshell/test/browser/file_bug422543_script.js

@@ -13,12 +13,10 @@ SHistoryListener.prototype = {
 
   OnHistoryGotoIndex: function (aIndex, aGotoURI) {
     this.last = "gotoindex";
-    return this.retval;
   },
 
   OnHistoryPurge: function (aNumEntries) {
     this.last = "purge";
-    return this.retval;
   },
 
   OnHistoryReload: function (aReloadURI, aReloadFlags) {

mobile/android/chrome/content/browser.js

@@ -4677,12 +4677,10 @@ Tab.prototype = {
 
   OnHistoryGotoIndex: function(index, gotoURI) {
     Services.obs.notifyObservers(this.browser, "Content:HistoryChange");
-    return true;
   },
 
   OnHistoryPurge: function(numEntries) {
     Services.obs.notifyObservers(this.browser, "Content:HistoryChange");
-    return true;
   },
 
   OnHistoryReplaceEntry: function(index) {