Bug 949104 - Don't write history visits during HTTP redirection r=rnewman
authorMark Finkle <mfinkle@mozilla.com>
Wed, 19 Nov 2014 09:06:16 -0500
changeset 240854 0ed3e26cbc2be9ccc96c32ee83d67e229a731e8e
parent 240853 90307470c3478f06261807b771e359ad750fa319
child 240855 d1adecc7adad0beee45eeea0d7f774adbfe03dee
child 240867 ef7bfca8a59f7cf2d6d259b058b0ad20c9b58ea9
push id4311
push userraliiev@mozilla.com
push dateMon, 12 Jan 2015 19:37:41 +0000
treeherdermozilla-beta@150c9fed433b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersrnewman
bugs949104
milestone36.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 949104 - Don't write history visits during HTTP redirection r=rnewman
mobile/android/components/build/nsAndroidHistory.cpp
mobile/android/components/build/nsAndroidHistory.h
--- a/mobile/android/components/build/nsAndroidHistory.cpp
+++ b/mobile/android/components/build/nsAndroidHistory.cpp
@@ -1,32 +1,36 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsThreadUtils.h"
 #include "nsAndroidHistory.h"
+#include "nsComponentManagerUtils.h"
 #include "AndroidBridge.h"
 #include "Link.h"
 #include "nsIURI.h"
-#include "mozilla/Services.h"
 #include "nsIObserverService.h"
 
+#include "mozilla/Services.h"
 #include "mozilla/Preferences.h"
 
 #define NS_LINK_VISITED_EVENT_TOPIC "link-visited"
 
 // We copy Places here.
 // Note that we don't yet observe this pref at runtime.
 #define PREF_HISTORY_ENABLED "places.history.enabled"
 
+// Time we wait to see if a pending visit is really a redirect
+#define PENDING_REDIRECT_TIMEOUT 3000
+
 using namespace mozilla;
 using mozilla::dom::Link;
 
-NS_IMPL_ISUPPORTS(nsAndroidHistory, IHistory, nsIRunnable)
+NS_IMPL_ISUPPORTS(nsAndroidHistory, IHistory, nsIRunnable, nsITimerCallback)
 
 nsAndroidHistory* nsAndroidHistory::sHistory = nullptr;
 
 /*static*/
 nsAndroidHistory*
 nsAndroidHistory::GetSingleton()
 {
   if (!sHistory) {
@@ -37,16 +41,18 @@ nsAndroidHistory::GetSingleton()
   NS_ADDREF(sHistory);
   return sHistory;
 }
 
 nsAndroidHistory::nsAndroidHistory()
   : mHistoryEnabled(true)
 {
   LoadPrefs();
+
+  mTimer = do_CreateInstance(NS_TIMER_CONTRACTID);
 }
 
 NS_IMETHODIMP
 nsAndroidHistory::RegisterVisitedCallback(nsIURI *aURI, Link *aContent)
 {
   if (!aContent || !aURI)
     return NS_OK;
 
@@ -153,21 +159,86 @@ nsAndroidHistory::IsEmbedURI(nsIURI* aUR
   EmbedArray::index_type i;
   EmbedArray::size_type length = mEmbedURIs.Length();
   for (i = 0; i < length && !equals; ++i) {
     aURI->Equals(mEmbedURIs.ElementAt(i), &equals);
   }
   return equals;
 }
 
+inline bool
+nsAndroidHistory::RemovePendingVisitURI(nsIURI* aURI) {
+  // Remove the first pending URI that matches. Return a boolean to
+  // let the caller know if we removed a URI or not.
+  bool equals = false;
+  PendingVisitArray::index_type i;
+  for (i = 0; i < mPendingVisitURIs.Length(); ++i) {
+    aURI->Equals(mPendingVisitURIs.ElementAt(i), &equals);
+    if (equals) {
+      mPendingVisitURIs.RemoveElementAt(i);
+      return true;
+    }
+  }
+  return false;
+}
+
+NS_IMETHODIMP
+nsAndroidHistory::Notify(nsITimer *timer)
+{
+  // Any pending visits left in the queue have exceeded our threshold for
+  // redirects, so save them
+  PendingVisitArray::index_type i;
+  for (i = 0; i < mPendingVisitURIs.Length(); ++i) {
+    SaveVisitURI(mPendingVisitURIs.ElementAt(i));
+  }
+  mPendingVisitURIs.Clear();
+
+  return NS_OK;
+}
+
+void
+nsAndroidHistory::SaveVisitURI(nsIURI* aURI) {
+  // Add the URI to our cache so we can take a fast path later
+  AppendToRecentlyVisitedURIs(aURI);
+
+  if (AndroidBridge::HasEnv()) {
+    // Save this URI in our history
+    nsAutoCString spec;
+    (void)aURI->GetSpec(spec);
+    mozilla::widget::android::GeckoAppShell::MarkURIVisited(NS_ConvertUTF8toUTF16(spec));
+  }
+
+  // Finally, notify that we've been visited.
+  nsCOMPtr<nsIObserverService> obsService = mozilla::services::GetObserverService();
+  if (obsService) {
+    obsService->NotifyObservers(aURI, NS_LINK_VISITED_EVENT_TOPIC, nullptr);
+  }
+}
+
 NS_IMETHODIMP
 nsAndroidHistory::VisitURI(nsIURI *aURI, nsIURI *aLastVisitedURI, uint32_t aFlags)
 {
-  if (!aURI)
+  if (!aURI) {
+    return NS_OK;
+  }
+
+  if (!(aFlags & VisitFlags::TOP_LEVEL)) {
     return NS_OK;
+  }
+
+  if (aFlags & VisitFlags::UNRECOVERABLE_ERROR) {
+    return NS_OK;
+  }
+
+  if (aFlags & VisitFlags::REDIRECT_SOURCE || aFlags & VisitFlags::REDIRECT_PERMANENT || aFlags & VisitFlags::REDIRECT_TEMPORARY) {
+    // aLastVisitedURI redirected to aURI. We want to ignore aLastVisitedURI,
+    // so remove the pending visit. We want to give aURI a chance to be saved,
+    // so don't return early.
+    RemovePendingVisitURI(aLastVisitedURI);
+  }
 
   // Silently return if URI is something we shouldn't add to DB.
   bool canAdd;
   nsresult rv = CanAddURI(aURI, &canAdd);
   NS_ENSURE_SUCCESS(rv, rv);
   if (!canAdd) {
     return NS_OK;
   }
@@ -175,45 +246,27 @@ nsAndroidHistory::VisitURI(nsIURI *aURI,
   if (aLastVisitedURI) {
     bool same;
     rv = aURI->Equals(aLastVisitedURI, &same);
     NS_ENSURE_SUCCESS(rv, rv);
     if (same && IsRecentlyVisitedURI(aURI)) {
       // Do not save refresh visits if we have visited this URI recently.
       return NS_OK;
     }
-  }
 
-  if (!(aFlags & VisitFlags::TOP_LEVEL)) {
-    AppendToEmbedURIs(aURI);
-    return NS_OK;
+    // Since we have a last visited URI and we were not redirected, it is
+    // safe to save the visit if it's still pending.
+    if (RemovePendingVisitURI(aLastVisitedURI)) {
+      SaveVisitURI(aLastVisitedURI);
+    }
   }
 
-  if (aFlags & VisitFlags::REDIRECT_SOURCE)
-    return NS_OK;
-
-  if (aFlags & VisitFlags::UNRECOVERABLE_ERROR)
-    return NS_OK;
-
-  if (AndroidBridge::HasEnv()) {
-    nsAutoCString uri;
-    rv = aURI->GetSpec(uri);
-    if (NS_FAILED(rv)) return rv;
-    NS_ConvertUTF8toUTF16 uriString(uri);
-    mozilla::widget::android::GeckoAppShell::MarkURIVisited(uriString);
-  }
-
-  AppendToRecentlyVisitedURIs(aURI);
-
-  // Finally, notify that we've been visited.
-  nsCOMPtr<nsIObserverService> obsService =
-    mozilla::services::GetObserverService();
-  if (obsService) {
-    obsService->NotifyObservers(aURI, NS_LINK_VISITED_EVENT_TOPIC, nullptr);
-  }
+  // Let's wait and see if this visit is not a redirect.
+  mPendingVisitURIs.AppendElement(aURI);
+  mTimer->InitWithCallback(this, PENDING_REDIRECT_TIMEOUT, nsITimer::TYPE_ONE_SHOT);
 
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsAndroidHistory::SetURITitle(nsIURI *aURI, const nsAString& aTitle)
 {
   // Silently return if URI is something we shouldn't add to DB.
@@ -239,27 +292,27 @@ nsAndroidHistory::SetURITitle(nsIURI *aU
 }
 
 NS_IMETHODIMP
 nsAndroidHistory::NotifyVisited(nsIURI *aURI)
 {
   if (aURI && sHistory) {
     nsAutoCString spec;
     (void)aURI->GetSpec(spec);
-    sHistory->mPendingURIs.Push(NS_ConvertUTF8toUTF16(spec));
+    sHistory->mPendingLinkURIs.Push(NS_ConvertUTF8toUTF16(spec));
     NS_DispatchToMainThread(sHistory);
   }
   return NS_OK;
 }
 
 NS_IMETHODIMP
 nsAndroidHistory::Run()
 {
-  while (! mPendingURIs.IsEmpty()) {
-    nsString uriString = mPendingURIs.Pop();
+  while (! mPendingLinkURIs.IsEmpty()) {
+    nsString uriString = mPendingLinkURIs.Pop();
     nsTArray<Link*>* list = sHistory->mListeners.Get(uriString);
     if (list) {
       for (unsigned int i = 0; i < list->Length(); i++) {
         list->ElementAt(i)->SetLinkState(eLinkState_Visited);
       }
       // as per the IHistory interface contract, remove the
       // Link pointers once they have been notified
       mListeners.Remove(uriString);
--- a/mobile/android/components/build/nsAndroidHistory.h
+++ b/mobile/android/components/build/nsAndroidHistory.h
@@ -6,32 +6,37 @@
 #ifndef NS_ANDROIDHISTORY_H
 #define NS_ANDROIDHISTORY_H
 
 #include "IHistory.h"
 #include "nsDataHashtable.h"
 #include "nsTPriorityQueue.h"
 #include "nsIRunnable.h"
 #include "nsIURI.h"
+#include "nsITimer.h"
+
 
 #define NS_ANDROIDHISTORY_CID \
     {0xCCAA4880, 0x44DD, 0x40A7, {0xA1, 0x3F, 0x61, 0x56, 0xFC, 0x88, 0x2C, 0x0B}}
 
 // Max size of History::mRecentlyVisitedURIs
 #define RECENTLY_VISITED_URI_SIZE 8
 
 // Max size of History::mEmbedURIs
 #define EMBED_URI_SIZE 128
 
-class nsAndroidHistory MOZ_FINAL : public mozilla::IHistory, public nsIRunnable
+class nsAndroidHistory MOZ_FINAL : public mozilla::IHistory,
+                                   public nsIRunnable,
+                                   public nsITimerCallback
 {
 public:
   NS_DECL_ISUPPORTS
   NS_DECL_IHISTORY
   NS_DECL_NSIRUNNABLE
+  NS_DECL_NSITIMERCALLBACK
 
   /**
    * Obtains a pointer that has had AddRef called on it.  Used by the service
    * manager only.
    */
   static nsAndroidHistory* GetSingleton();
 
   nsAndroidHistory();
@@ -39,29 +44,43 @@ public:
 private:
   ~nsAndroidHistory() {}
 
   static nsAndroidHistory* sHistory;
 
   // Will mimic the value of the places.history.enabled preference.
   bool mHistoryEnabled;
 
-  nsDataHashtable<nsStringHashKey, nsTArray<mozilla::dom::Link *> *> mListeners;
-  nsTPriorityQueue<nsString> mPendingURIs;
-
   void LoadPrefs();
   bool ShouldRecordHistory();
   nsresult CanAddURI(nsIURI* aURI, bool* canAdd);
 
   /**
+   * We need to manage data used to determine a:visited status.
+   */
+  nsDataHashtable<nsStringHashKey, nsTArray<mozilla::dom::Link *> *> mListeners;
+  nsTPriorityQueue<nsString> mPendingLinkURIs;
+
+  /**
+   * Redirection (temporary and permanent) flags are sent with the redirected
+   * URI, not the original URI. Since we want to ignore the original URI, we
+   * need to cache the pending visit and make sure it doesn't redirect.
+   */
+  nsRefPtr<nsITimer> mTimer;
+  typedef nsAutoTArray<nsCOMPtr<nsIURI>, RECENTLY_VISITED_URI_SIZE> PendingVisitArray;
+  PendingVisitArray mPendingVisitURIs;
+
+  bool RemovePendingVisitURI(nsIURI* aURI);
+  void SaveVisitURI(nsIURI* aURI);
+
+  /**
    * 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;
+  typedef nsAutoTArray<nsCOMPtr<nsIURI>, RECENTLY_VISITED_URI_SIZE> RecentlyVisitedArray;
   RecentlyVisitedArray mRecentlyVisitedURIs;
   RecentlyVisitedArray::index_type mRecentlyVisitedURIsNextIndex;
 
   void AppendToRecentlyVisitedURIs(nsIURI* aURI);
   bool IsRecentlyVisitedURI(nsIURI* aURI);
 
   /**
    * mEmbedURIs remembers URIs which are explicitly not added to the DB,