Bug 680727 - Part 1: IHistory::VisitURI should accept a reloaded URI. r=mak
authorOonishi Atsushi <torisugari@gmail.com>
Wed, 28 Mar 2012 21:50:59 +0200
changeset 90556 bc7c335062cf39b5c7e62fe06d0836bb6f85ca51
parent 90555 ee3a013ecc898c79656d262e7b6c719d0a05c6c4
child 90557 9f6159650e8071a10dfa4bb95b96785616fbe81b
push idunknown
push userunknown
push dateunknown
reviewersmak
bugs680727
milestone14.0a1
Bug 680727 - Part 1: IHistory::VisitURI should accept a reloaded URI. r=mak
toolkit/components/places/History.cpp
toolkit/components/places/History.h
toolkit/components/places/tests/browser/Makefile.in
toolkit/components/places/tests/browser/browser_bug680727.js
toolkit/components/places/tests/browser/head.js
--- a/toolkit/components/places/History.cpp
+++ b/toolkit/components/places/History.cpp
@@ -500,16 +500,17 @@ public:
     if (obsService) {
       DebugOnly<nsresult> rv =
         obsService->NotifyObservers(uri, URI_VISIT_SAVED, nsnull);
       NS_WARN_IF_FALSE(NS_SUCCEEDED(rv), "Could not notify observers");
     }
 
     History* history = History::GetService();
     NS_ENSURE_STATE(history);
+    history->AppendToRecentlyVisitedURIs(uri);
     history->NotifyVisited(uri);
 
     return NS_OK;
   }
 private:
   VisitData mPlace;
   VisitData mReferrer;
   nsRefPtr<History> mHistory;
@@ -1448,16 +1449,17 @@ NS_MEMORY_REPORTER_IMPLEMENT(HistoryServ
 ////////////////////////////////////////////////////////////////////////////////
 //// History
 
 History* History::gService = NULL;
 
 History::History()
   : mShuttingDown(false)
   , mShutdownMutex("History::mShutdownMutex")
+  , mRecentlyVisitedURIsNextIndex(0)
 {
   NS_ASSERTION(!gService, "Ruh-roh!  This service has already been created!");
   gService = this;
 
   nsCOMPtr<nsIObserverService> os = services::GetObserverService();
   NS_WARN_IF_FALSE(os, "Observer service was not found!");
   if (os) {
     (void)os->AddObserver(this, TOPIC_PLACES_SHUTDOWN, false);
@@ -1777,16 +1779,39 @@ History::Shutdown()
   if (mReadOnlyDBConn) {
     if (mIsVisitedStatement) {
       (void)mIsVisitedStatement->Finalize();
     }
     (void)mReadOnlyDBConn->AsyncClose(nsnull);
   }
 }
 
+void
+History::AppendToRecentlyVisitedURIs(nsIURI* aURI) {
+  if (mRecentlyVisitedURIs.Length() < RECENTLY_VISITED_URI_SIZE) {
+    // Append a new element while the array is not full.
+    mRecentlyVisitedURIs.AppendElement(aURI);
+  } else {
+    // Otherwise, replace the oldest member.
+    mRecentlyVisitedURIsNextIndex %= RECENTLY_VISITED_URI_SIZE;
+    mRecentlyVisitedURIs.ElementAt(mRecentlyVisitedURIsNextIndex) = aURI;
+    mRecentlyVisitedURIsNextIndex++;
+  }
+}
+
+inline bool
+History::IsRecentlyVisitedURI(nsIURI* aURI) {
+  bool equals = false;
+  RecentlyVisitedArray::index_type i;
+  for (i = 0; i < mRecentlyVisitedURIs.Length() && !equals; ++i) {
+    aURI->Equals(mRecentlyVisitedURIs.ElementAt(i), &equals);
+  }
+  return equals;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 //// IHistory
 
 NS_IMETHODIMP
 History::VisitURI(nsIURI* aURI,
                   nsIURI* aLastVisitedURI,
                   PRUint32 aFlags)
 {
@@ -1813,18 +1838,18 @@ History::VisitURI(nsIURI* aURI,
   if (!canAdd) {
     return NS_OK;
   }
 
   if (aLastVisitedURI) {
     bool same;
     rv = aURI->Equals(aLastVisitedURI, &same);
     NS_ENSURE_SUCCESS(rv, rv);
-    if (same) {
-      // Do not save refresh-page visits.
+    if (same && IsRecentlyVisitedURI(aURI)) {
+      // Do not save refresh visits if we have visited this URI recently.
       return NS_OK;
     }
   }
 
   nsTArray<VisitData> placeArray(1);
   NS_ENSURE_TRUE(placeArray.AppendElement(VisitData(aURI, aLastVisitedURI)),
                  NS_ERROR_OUT_OF_MEMORY);
   VisitData& place = placeArray.ElementAt(0);
--- a/toolkit/components/places/History.h
+++ b/toolkit/components/places/History.h
@@ -58,16 +58,19 @@
 namespace mozilla {
 namespace places {
 
 struct VisitData;
 
 #define NS_HISTORYSERVICE_CID \
   {0x0937a705, 0x91a6, 0x417a, {0x82, 0x92, 0xb2, 0x2e, 0xb1, 0x0d, 0xa8, 0x6c}}
 
+// Max size of History::mRecentlyVisitedURIs
+#define RECENTLY_VISITED_URI_SIZE 8
+
 class History : public IHistory
               , public nsIDownloadHistory
               , public mozIAsyncHistory
               , public nsIObserver
 {
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_IHISTORY
@@ -143,16 +146,22 @@ public:
 
   bool IsShuttingDown() const {
     return mShuttingDown;
   }
   Mutex& GetShutdownMutex() {
     return mShutdownMutex;
   }
 
+  /**
+   * Helper function to append a new URI to mRecentlyVisitedURIs. See
+   * mRecentlyVisitedURIs.
+   */
+  void AppendToRecentlyVisitedURIs(nsIURI* aURI);
+
 private:
   virtual ~History();
 
   /**
    * Obtains a read-write database connection.
    */
   mozIStorageConnection* GetDBConn();
 
@@ -214,14 +223,25 @@ private:
    * Helper function for nsTHashtable::SizeOfExcludingThis call in
    * SizeOfIncludingThis().
    */
   static size_t SizeOfEntryExcludingThis(KeyClass* aEntry,
                                          nsMallocSizeOfFun aMallocSizeOf,
                                          void*);
 
   nsTHashtable<KeyClass> mObservers;
+
+  /**
+   * mRecentlyVisitedURIs remembers URIs which are recently added to the DB,
+   * to avoid saving these locations repeatedly in a short period.
+   */
+  typedef nsAutoTArray<nsCOMPtr<nsIURI>, RECENTLY_VISITED_URI_SIZE>
+          RecentlyVisitedArray;
+  RecentlyVisitedArray mRecentlyVisitedURIs;
+  RecentlyVisitedArray::index_type mRecentlyVisitedURIsNextIndex;
+
+  bool IsRecentlyVisitedURI(nsIURI* aURI);
 };
 
 } // namespace places
 } // namespace mozilla
 
 #endif // mozilla_places_History_h_
--- a/toolkit/components/places/tests/browser/Makefile.in
+++ b/toolkit/components/places/tests/browser/Makefile.in
@@ -48,16 +48,17 @@ include $(topsrcdir)/config/rules.mk
 _BROWSER_FILES = \
 	head.js \
 	browser_bug399606.js \
 	browser_visituri.js \
 	browser_visituri_nohistory.js \
 	browser_visituri_privatebrowsing.js \
 	browser_settitle.js \
 	browser_bug646422.js \
+	browser_bug680727.js \
 	$(NULL)
 
 # These are files that need to be loaded via the HTTP proxy server
 # Access them through http://example.com/
 _HTTP_FILES = \
 	bug_399606/399606-httprefresh.html \
 	bug_399606/399606-location.reload.html \
 	bug_399606/399606-location.replace.html \
new file mode 100644
--- /dev/null
+++ b/toolkit/components/places/tests/browser/browser_bug680727.js
@@ -0,0 +1,100 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/* Ensure that clicking the button in the Offline mode neterror page updates
+   global history. See bug 680727. */
+/* TEST_PATH=toolkit/components/places/tests/browser/browser_bug680727.js make -C $(OBJDIR) mochitest-browser-chrome */
+
+
+const kUniqueURI = Services.io.newURI("http://mochi.test:8888/#bug_680727",
+                                      null, null);
+var gAsyncHistory = 
+  Cc["@mozilla.org/browser/history;1"].getService(Ci.mozIAsyncHistory);
+
+function test() {
+  waitForExplicitFinish();
+
+  gBrowser.selectedTab = gBrowser.addTab();
+
+  // Clear network cache.
+  Components.classes["@mozilla.org/network/cache-service;1"]
+            .getService(Components.interfaces.nsICacheService)
+            .evictEntries(Components.interfaces.nsICache.STORE_ANYWHERE);
+
+  // Go offline, expecting the error page.
+  Services.io.offline = true;
+  window.addEventListener("DOMContentLoaded", errorListener, false);
+  content.location = kUniqueURI.spec;
+}
+
+//------------------------------------------------------------------------------
+// listen to loading the neterror page. (offline mode)
+function errorListener() {
+  if(content.location == "about:blank") {
+    info("got about:blank, which is expected once, so return");
+    return;
+  }
+
+  window.removeEventListener("DOMContentLoaded", errorListener, false);
+  ok(Services.io.offline, "Services.io.offline is true.");
+
+  // This is an error page.
+  is(gBrowser.contentDocument.documentURI.substring(0, 27),
+     "about:neterror?e=netOffline",
+     "Document URI is the error page.");
+
+  // But location bar should show the original request.
+  is(content.location.href, kUniqueURI.spec,
+     "Docshell URI is the original URI.");
+
+  // Global history does not record URI of a failed request.
+  waitForAsyncUpdates(function() {
+    gAsyncHistory.isURIVisited(kUniqueURI, errorAsyncListener);
+  });
+}
+
+function errorAsyncListener(aURI, aIsVisited) {
+  ok(kUniqueURI.equals(aURI) && !aIsVisited,
+     "The neterror page is not listed in global history.");
+
+  // Now press the "Try Again" button, with offline mode off.
+  Services.io.offline = false;
+
+  window.addEventListener("DOMContentLoaded", reloadListener, false);
+
+  ok(gBrowser.contentDocument.getElementById("errorTryAgain"),
+     "The error page has got a #errorTryAgain element");
+  gBrowser.contentDocument.getElementById("errorTryAgain").click();
+}
+
+//------------------------------------------------------------------------------
+// listen to reload of neterror.
+function reloadListener() {
+  window.removeEventListener("DOMContentLoaded", reloadListener, false);
+
+  // This listener catches "DOMContentLoaded" on being called
+  // nsIWPL::onLocationChange(...). That is right *AFTER*
+  // IHistory::VisitURI(...) is called.
+  ok(!Services.io.offline, "Services.io.offline is false.");
+
+  // This is not an error page.
+  is(gBrowser.contentDocument.documentURI, kUniqueURI.spec, 
+     "Document URI is not the offline-error page, but the original URI.");
+
+  // Check if global history remembers the successfully-requested URI.
+  waitForAsyncUpdates(function() {
+    gAsyncHistory.isURIVisited(kUniqueURI, reloadAsyncListener);
+  });
+}
+
+function reloadAsyncListener(aURI, aIsVisited) {
+  ok(kUniqueURI.equals(aURI) && aIsVisited, "We have visited the URI.");
+  waitForClearHistory(finish);
+}
+
+registerCleanupFunction(function() {
+  Services.io.offline = false;
+  window.removeEventListener("DOMContentLoaded", errorListener, false);
+  window.removeEventListener("DOMContentLoaded", reloadListener, false);
+  gBrowser.removeCurrentTab();
+});
--- a/toolkit/components/places/tests/browser/head.js
+++ b/toolkit/components/places/tests/browser/head.js
@@ -5,8 +5,30 @@ Components.utils.import("resource://gre/
 
 function waitForClearHistory(aCallback) {
   Services.obs.addObserver(function observeCH(aSubject, aTopic, aData) {
     Services.obs.removeObserver(observeCH, PlacesUtils.TOPIC_EXPIRATION_FINISHED);
     aCallback();
   }, PlacesUtils.TOPIC_EXPIRATION_FINISHED, false);
   PlacesUtils.bhistory.removeAllPages();
 }
+
+function waitForAsyncUpdates(aCallback, aScope, aArguments)
+{
+  let scope = aScope || this;
+  let args = aArguments || [];
+  let db = PlacesUtils.history.QueryInterface(Ci.nsPIPlacesDatabase)
+                              .DBConnection;
+  let begin = db.createAsyncStatement("BEGIN EXCLUSIVE");
+  begin.executeAsync();
+  begin.finalize();
+
+  let commit = db.createAsyncStatement("COMMIT");
+  commit.executeAsync({
+    handleResult: function() {},
+    handleError: function() {},
+    handleCompletion: function(aReason)
+    {
+      aCallback.apply(scope, args);
+    }
+  });
+  commit.finalize();
+}