Bug 737836 - Don't rely on past visits for frecency calcuations for redirects. r=mak
authorMark Banner <standard8@mozilla.com>
Wed, 18 Jan 2017 14:09:09 +0000
changeset 375316 9c0c8b1a1250b7c0314368f8cba9d8380ba4fc82
parent 375315 7ac654e1c42652c8a37c2919c870a32b61894597
child 375317 05848bf39248f1199d6877efd49c1fcbb0da4585
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs737836
milestone53.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 737836 - Don't rely on past visits for frecency calcuations for redirects. r=mak MozReview-Commit-ID: GfgZSInN9Lv
browser/app/profile/firefox.js
toolkit/components/places/History.cpp
toolkit/components/places/SQLFunctions.cpp
toolkit/components/places/SQLFunctions.h
toolkit/components/places/nsNavHistory.cpp
toolkit/components/places/nsNavHistory.h
toolkit/components/places/tests/browser/browser_redirect.js
--- a/browser/app/profile/firefox.js
+++ b/browser/app/profile/firefox.js
@@ -850,17 +850,24 @@ pref("places.frecency.thirdBucketWeight"
 pref("places.frecency.fourthBucketWeight", 30);
 pref("places.frecency.defaultBucketWeight", 10);
 
 // bonus (in percent) for visit transition types for frecency calculations
 pref("places.frecency.embedVisitBonus", 0);
 pref("places.frecency.framedLinkVisitBonus", 0);
 pref("places.frecency.linkVisitBonus", 100);
 pref("places.frecency.typedVisitBonus", 2000);
+// The bookmarks bonus is always added on top of any other bonus, including
+// the redirect source and the typed ones.
 pref("places.frecency.bookmarkVisitBonus", 75);
+// The redirect source bonus overwrites any transition bonus.
+// 0 would hide these pages, instead we want them low ranked.  Thus we use
+// linkVisitBonus - bookmarkVisitBonus, so that a bookmarked source is in par
+// with a common link.
+pref("places.frecency.redirectSourceVisitBonus", 25);
 pref("places.frecency.downloadVisitBonus", 0);
 pref("places.frecency.permRedirectVisitBonus", 0);
 pref("places.frecency.tempRedirectVisitBonus", 0);
 pref("places.frecency.reloadVisitBonus", 0);
 pref("places.frecency.defaultVisitBonus", 0);
 
 // bonus (in percent) for place types for frecency calculations
 pref("places.frecency.unvisitedBookmarkBonus", 140);
@@ -1569,9 +1576,9 @@ pref("services.sync.validation.enabled",
 
 // Preferences for the form autofill system extension
 pref("browser.formautofill.experimental", false);
 
 // Enable safebrowsing v4 tables (suffixed by "-proto") update.
 #ifdef NIGHTLY_BUILD
 pref("urlclassifier.malwareTable", "goog-malware-shavar,goog-unwanted-shavar,goog-malware-proto,goog-unwanted-proto,test-malware-simple,test-unwanted-simple");
 pref("urlclassifier.phishTable", "goog-phish-shavar,goog-phish-proto,test-phish-simple");
-#endif
\ No newline at end of file
+#endif
--- a/toolkit/components/places/History.cpp
+++ b/toolkit/components/places/History.cpp
@@ -79,16 +79,17 @@ struct VisitData {
   , visitTime(0)
   , frecency(-1)
   , lastVisitId(0)
   , lastVisitTime(0)
   , visitCount(0)
   , referrerVisitId(0)
   , titleChanged(false)
   , shouldUpdateFrecency(true)
+  , redirect(false)
   {
     guid.SetIsVoid(true);
     title.SetIsVoid(true);
   }
 
   explicit VisitData(nsIURI* aURI,
                      nsIURI* aReferrer = nullptr)
   : placeId(0)
@@ -100,16 +101,17 @@ struct VisitData {
   , visitTime(0)
   , frecency(-1)
   , lastVisitId(0)
   , lastVisitTime(0)
   , visitCount(0)
   , referrerVisitId(0)
   , titleChanged(false)
   , shouldUpdateFrecency(true)
+  , redirect(false)
   {
     MOZ_ASSERT(aURI);
     if (aURI) {
       (void)aURI->GetSpec(spec);
       (void)GetReversedHostname(aURI, revHost);
     }
     if (aReferrer) {
       (void)aReferrer->GetSpec(referrerSpec);
@@ -157,16 +159,19 @@ struct VisitData {
   nsCString referrerSpec;
   int64_t referrerVisitId;
 
   // TODO bug 626836 hook up hidden and typed change tracking too!
   bool titleChanged;
 
   // Indicates whether frecency should be updated for this visit.
   bool shouldUpdateFrecency;
+
+  // Whether this is a redirect source.
+  bool redirect;
 };
 
 ////////////////////////////////////////////////////////////////////////////////
 //// RemoveVisitsFilter
 
 /**
  * Used to store visit filters for RemoveVisits.
  */
@@ -1159,27 +1164,30 @@ private:
     MOZ_ASSERT(aPlace.placeId > 0);
 
     nsresult rv;
     { // First, set our frecency to the proper value.
       nsCOMPtr<mozIStorageStatement> stmt;
       stmt = mHistory->GetStatement(
         "UPDATE moz_places "
         "SET frecency = NOTIFY_FRECENCY("
-          "CALCULATE_FRECENCY(:page_id), "
+          "CALCULATE_FRECENCY(:page_id, :redirect), "
           "url, guid, hidden, last_visit_date"
         ") "
         "WHERE id = :page_id"
       );
       NS_ENSURE_STATE(stmt);
       mozStorageStatementScoper scoper(stmt);
 
       rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("page_id"), aPlace.placeId);
       NS_ENSURE_SUCCESS(rv, rv);
 
+      rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("redirect"), aPlace.redirect);
+      NS_ENSURE_SUCCESS(rv, rv);
+
       rv = stmt->Execute();
       NS_ENSURE_SUCCESS(rv, rv);
     }
 
     if (!aPlace.hidden && aPlace.shouldUpdateHidden) {
       // Mark the page as not hidden if the frecency is now nonzero.
       nsCOMPtr<mozIStorageStatement> stmt;
       stmt = mHistory->GetStatement(
@@ -2436,18 +2444,18 @@ History::VisitURI(nsIURI* aURI,
     transitionType = nsINavHistoryService::TRANSITION_BOOKMARK;
   }
   else if (!(aFlags & IHistory::TOP_LEVEL) && isFollowedLink) {
     // User activated a link in a frame.
     transitionType = nsINavHistoryService::TRANSITION_FRAMED_LINK;
   }
 
   place.SetTransitionType(transitionType);
-  place.hidden = GetHiddenState(aFlags & IHistory::REDIRECT_SOURCE,
-                                transitionType);
+  place.redirect = aFlags & IHistory::REDIRECT_SOURCE;
+  place.hidden = GetHiddenState(place.redirect, place.transitionType);
 
   // Error pages should never be autocompleted.
   if (aFlags & IHistory::UNRECOVERABLE_ERROR) {
     place.shouldUpdateFrecency = false;
   }
 
   // EMBED visits are session-persistent and should not go through the database.
   // They exist only to keep track of isVisited status during the session.
@@ -2568,22 +2576,22 @@ History::SetURITitle(nsIURI* aURI, const
   if (mShuttingDown) {
     return NS_OK;
   }
 
   if (XRE_IsContentProcess()) {
     URIParams uri;
     SerializeURI(aURI, uri);
 
-    mozilla::dom::ContentChild * cpc = 
+    mozilla::dom::ContentChild * cpc =
       mozilla::dom::ContentChild::GetSingleton();
     NS_ASSERTION(cpc, "Content Protocol is NULL!");
     (void)cpc->SendSetURITitle(uri, PromiseFlatString(aTitle));
     return NS_OK;
-  } 
+  }
 
   nsNavHistory* navHistory = nsNavHistory::GetHistoryService();
 
   // At first, it seems like nav history should always be available here, no
   // matter what.
   //
   // nsNavHistory fails to register as a service if there is no profile in
   // place (for instance, if user is choosing a profile).
--- a/toolkit/components/places/SQLFunctions.cpp
+++ b/toolkit/components/places/SQLFunctions.cpp
@@ -446,17 +446,17 @@ namespace places {
   /* static */
   nsresult
   CalculateFrecencyFunction::create(mozIStorageConnection *aDBConn)
   {
     RefPtr<CalculateFrecencyFunction> function =
       new CalculateFrecencyFunction();
 
     nsresult rv = aDBConn->CreateFunction(
-      NS_LITERAL_CSTRING("calculate_frecency"), 1, function
+      NS_LITERAL_CSTRING("calculate_frecency"), -1, function
     );
     NS_ENSURE_SUCCESS(rv, rv);
 
     return NS_OK;
   }
 
   NS_IMPL_ISUPPORTS(
     CalculateFrecencyFunction,
@@ -466,25 +466,37 @@ namespace places {
   NS_IMETHODIMP
   CalculateFrecencyFunction::OnFunctionCall(mozIStorageValueArray *aArguments,
                                             nsIVariant **_result)
   {
     // Fetch arguments.  Use default values if they were omitted.
     uint32_t numEntries;
     nsresult rv = aArguments->GetNumEntries(&numEntries);
     NS_ENSURE_SUCCESS(rv, rv);
-    MOZ_ASSERT(numEntries == 1, "unexpected number of arguments");
+    MOZ_ASSERT(numEntries <= 2, "unexpected number of arguments");
 
     int64_t pageId = aArguments->AsInt64(0);
     MOZ_ASSERT(pageId > 0, "Should always pass a valid page id");
     if (pageId <= 0) {
       NS_ADDREF(*_result = new IntegerVariant(0));
       return NS_OK;
     }
 
+    enum RedirectState {
+      eRedirectUnknown,
+      eIsRedirect,
+      eIsNotRedirect
+    };
+
+    RedirectState isRedirect = eRedirectUnknown;
+
+    if (numEntries > 1) {
+      isRedirect = aArguments->AsInt32(1) ? eIsRedirect : eIsNotRedirect;
+    }
+
     int32_t typed = 0;
     int32_t visitCount = 0;
     bool hasBookmark = false;
     int32_t isQuery = 0;
     float pointsForSampledVisits = 0.0;
     int32_t numSampledVisits = 0;
     int32_t bonus = 0;
 
@@ -524,28 +536,33 @@ namespace places {
       rv = getPageInfo->GetInt32(3, &isQuery);
       NS_ENSURE_SUCCESS(rv, rv);
     }
 
     if (visitCount > 0) {
       // Get a sample of the last visits to the page, to calculate its weight.
       // In case of a temporary or permanent redirect, calculate the frecency
       // as if the original page was visited.
+      nsCString redirectsTransitionFragment =
+        nsPrintfCString("%d AND %d ", nsINavHistoryService::TRANSITION_REDIRECT_PERMANENT,
+                                      nsINavHistoryService::TRANSITION_REDIRECT_TEMPORARY);
       nsCOMPtr<mozIStorageStatement> getVisits = DB->GetStatement(
         NS_LITERAL_CSTRING(
           "/* do not warn (bug 659740 - SQLite may ignore index if few visits exist) */"
           "SELECT "
             "ROUND((strftime('%s','now','localtime','utc') - v.visit_date/1000000)/86400), "
-            "IFNULL(r.visit_type, v.visit_type), "
-            "v.visit_date "
+            "IFNULL(origin.visit_type, v.visit_type), "
+            "target.id NOTNULL "
           "FROM moz_historyvisits v "
-          "LEFT JOIN moz_historyvisits r ON r.id = v.from_visit AND v.visit_type BETWEEN "
-        ) + nsPrintfCString("%d AND %d ", nsINavHistoryService::TRANSITION_REDIRECT_PERMANENT,
-                                          nsINavHistoryService::TRANSITION_REDIRECT_TEMPORARY) +
-        NS_LITERAL_CSTRING(
+          "LEFT JOIN moz_historyvisits origin ON origin.id = v.from_visit "
+                                            "AND v.visit_type BETWEEN "
+            ) + redirectsTransitionFragment + NS_LITERAL_CSTRING(
+          "LEFT JOIN moz_historyvisits target ON v.id = target.from_visit "
+                                            "AND target.visit_type BETWEEN "
+            ) + redirectsTransitionFragment + NS_LITERAL_CSTRING(
           "WHERE v.place_id = :page_id "
           "ORDER BY v.visit_date DESC "
         )
       );
       NS_ENSURE_STATE(getVisits);
       mozStorageStatementScoper visitsScoper(getVisits);
       rv = getVisits->BindInt64ByName(NS_LITERAL_CSTRING("page_id"), pageId);
       NS_ENSURE_SUCCESS(rv, rv);
@@ -554,17 +571,30 @@ namespace places {
       bool hasResult = false;
       for (int32_t maxVisits = history->GetNumVisitsForFrecency();
            numSampledVisits < maxVisits &&
            NS_SUCCEEDED(getVisits->ExecuteStep(&hasResult)) && hasResult;
            numSampledVisits++) {
         int32_t visitType;
         rv = getVisits->GetInt32(1, &visitType);
         NS_ENSURE_SUCCESS(rv, rv);
-        bonus = history->GetFrecencyTransitionBonus(visitType, true);
+
+        RedirectState visitIsRedirect = isRedirect;
+
+        // If we don't know if this is a redirect or not, or this is not the
+        // most recent visit that we're looking at, then we use the redirect
+        // value from the database.
+        if (visitIsRedirect == eRedirectUnknown || numSampledVisits >= 1) {
+          int32_t redirect;
+          rv = getVisits->GetInt32(2, &redirect);
+          NS_ENSURE_SUCCESS(rv, rv);
+          visitIsRedirect = !!redirect ? eIsRedirect : eIsNotRedirect;
+        }
+
+        bonus = history->GetFrecencyTransitionBonus(visitType, true, visitIsRedirect == eIsRedirect);
 
         // Add the bookmark visit bonus.
         if (hasBookmark) {
           bonus += history->GetFrecencyTransitionBonus(nsINavHistoryService::TRANSITION_BOOKMARK, true);
         }
 
         // If bonus was zero, we can skip the work to determine the weight.
         if (bonus) {
--- a/toolkit/components/places/SQLFunctions.h
+++ b/toolkit/components/places/SQLFunctions.h
@@ -182,24 +182,20 @@ private:
 /**
  * This function is used to calculate frecency for a page.
  *
  * In SQL, you'd use it in when setting frecency like:
  * SET frecency = CALCULATE_FRECENCY(place_id).
  * Optional parameters must be passed in if the page is not yet in the database,
  * otherwise they will be fetched from it automatically.
  *
- * @param pageId
+ * @param {int64_t} pageId
  *        The id of the page.  Pass -1 if the page is being added right now.
- * @param [optional] typed
- *        Whether the page has been typed in.  Default is false.
- * @param [optional] fullVisitCount
- *        Count of all the visits (All types).  Default is 0.
- * @param [optional] isBookmarked
- *        Whether the page is bookmarked. Default is false.
+ * @param [optional] {int32_t} redirect
+ *        Whether the page visit is a redirect.  Default is false.
  */
 class CalculateFrecencyFunction final : public mozIStorageFunction
 {
 public:
   NS_DECL_THREADSAFE_ISUPPORTS
   NS_DECL_MOZISTORAGEFUNCTION
 
   /**
--- a/toolkit/components/places/nsNavHistory.cpp
+++ b/toolkit/components/places/nsNavHistory.cpp
@@ -88,16 +88,18 @@ using namespace mozilla::places;
 #define PREF_FREC_BOOKMARK_VISIT_BONUS          "places.frecency.bookmarkVisitBonus"
 #define PREF_FREC_BOOKMARK_VISIT_BONUS_DEF      75
 #define PREF_FREC_DOWNLOAD_VISIT_BONUS          "places.frecency.downloadVisitBonus"
 #define PREF_FREC_DOWNLOAD_VISIT_BONUS_DEF      0
 #define PREF_FREC_PERM_REDIRECT_VISIT_BONUS     "places.frecency.permRedirectVisitBonus"
 #define PREF_FREC_PERM_REDIRECT_VISIT_BONUS_DEF 0
 #define PREF_FREC_TEMP_REDIRECT_VISIT_BONUS     "places.frecency.tempRedirectVisitBonus"
 #define PREF_FREC_TEMP_REDIRECT_VISIT_BONUS_DEF 0
+#define PREF_FREC_REDIR_SOURCE_VISIT_BONUS      "places.frecency.redirectSourceVisitBonus"
+#define PREF_FREC_REDIR_SOURCE_VISIT_BONUS_DEF  25
 #define PREF_FREC_DEFAULT_VISIT_BONUS           "places.frecency.defaultVisitBonus"
 #define PREF_FREC_DEFAULT_VISIT_BONUS_DEF       0
 #define PREF_FREC_UNVISITED_BOOKMARK_BONUS      "places.frecency.unvisitedBookmarkBonus"
 #define PREF_FREC_UNVISITED_BOOKMARK_BONUS_DEF  140
 #define PREF_FREC_UNVISITED_TYPED_BONUS         "places.frecency.unvisitedTypedBonus"
 #define PREF_FREC_UNVISITED_TYPED_BONUS_DEF     200
 #define PREF_FREC_RELOAD_VISIT_BONUS            "places.frecency.reloadVisitBonus"
 #define PREF_FREC_RELOAD_VISIT_BONUS_DEF        0
@@ -150,16 +152,17 @@ static const char* kObservedPrefs[] = {
 , PREF_FREC_EMBED_VISIT_BONUS
 , PREF_FREC_FRAMED_LINK_VISIT_BONUS
 , PREF_FREC_LINK_VISIT_BONUS
 , PREF_FREC_TYPED_VISIT_BONUS
 , PREF_FREC_BOOKMARK_VISIT_BONUS
 , PREF_FREC_DOWNLOAD_VISIT_BONUS
 , PREF_FREC_PERM_REDIRECT_VISIT_BONUS
 , PREF_FREC_TEMP_REDIRECT_VISIT_BONUS
+, PREF_FREC_REDIR_SOURCE_VISIT_BONUS
 , PREF_FREC_DEFAULT_VISIT_BONUS
 , PREF_FREC_UNVISITED_BOOKMARK_BONUS
 , PREF_FREC_UNVISITED_TYPED_BONUS
 , nullptr
 };
 
 NS_IMPL_ADDREF(nsNavHistory)
 NS_IMPL_RELEASE(nsNavHistory)
@@ -464,16 +467,17 @@ nsNavHistory::LoadPrefs()
   FRECENCY_PREF(mEmbedVisitBonus,          PREF_FREC_EMBED_VISIT_BONUS);
   FRECENCY_PREF(mFramedLinkVisitBonus,     PREF_FREC_FRAMED_LINK_VISIT_BONUS);
   FRECENCY_PREF(mLinkVisitBonus,           PREF_FREC_LINK_VISIT_BONUS);
   FRECENCY_PREF(mTypedVisitBonus,          PREF_FREC_TYPED_VISIT_BONUS);
   FRECENCY_PREF(mBookmarkVisitBonus,       PREF_FREC_BOOKMARK_VISIT_BONUS);
   FRECENCY_PREF(mDownloadVisitBonus,       PREF_FREC_DOWNLOAD_VISIT_BONUS);
   FRECENCY_PREF(mPermRedirectVisitBonus,   PREF_FREC_PERM_REDIRECT_VISIT_BONUS);
   FRECENCY_PREF(mTempRedirectVisitBonus,   PREF_FREC_TEMP_REDIRECT_VISIT_BONUS);
+  FRECENCY_PREF(mRedirectSourceVisitBonus, PREF_FREC_REDIR_SOURCE_VISIT_BONUS);
   FRECENCY_PREF(mDefaultVisitBonus,        PREF_FREC_DEFAULT_VISIT_BONUS);
   FRECENCY_PREF(mUnvisitedBookmarkBonus,   PREF_FREC_UNVISITED_BOOKMARK_BONUS);
   FRECENCY_PREF(mUnvisitedTypedBonus,      PREF_FREC_UNVISITED_TYPED_BONUS);
   FRECENCY_PREF(mReloadVisitBonus,         PREF_FREC_RELOAD_VISIT_BONUS);
   FRECENCY_PREF(mFirstBucketWeight,        PREF_FREC_FIRST_BUCKET_WEIGHT);
   FRECENCY_PREF(mSecondBucketWeight,       PREF_FREC_SECOND_BUCKET_WEIGHT);
   FRECENCY_PREF(mThirdBucketWeight,        PREF_FREC_THIRD_BUCKET_WEIGHT);
   FRECENCY_PREF(mFourthBucketWeight,       PREF_FREC_FOURTH_BUCKET_WEIGHT);
--- a/toolkit/components/places/nsNavHistory.h
+++ b/toolkit/components/places/nsNavHistory.h
@@ -389,18 +389,23 @@ public:
       case 4:
         return mFourthBucketWeight;
       default:
         return mDefaultWeight;
     }
   }
 
   int32_t GetFrecencyTransitionBonus(int32_t aTransitionType,
-                                     bool aVisited) const
+                                     bool aVisited,
+                                     bool aRedirect = false) const
   {
+    if (aRedirect) {
+      return mRedirectSourceVisitBonus;
+    }
+
     switch (aTransitionType) {
       case nsINavHistoryService::TRANSITION_EMBED:
         return mEmbedVisitBonus;
       case nsINavHistoryService::TRANSITION_FRAMED_LINK:
         return mFramedLinkVisitBonus;
       case nsINavHistoryService::TRANSITION_LINK:
         return mLinkVisitBonus;
       case nsINavHistoryService::TRANSITION_TYPED:
@@ -614,16 +619,17 @@ protected:
   int32_t mEmbedVisitBonus;
   int32_t mFramedLinkVisitBonus;
   int32_t mLinkVisitBonus;
   int32_t mTypedVisitBonus;
   int32_t mBookmarkVisitBonus;
   int32_t mDownloadVisitBonus;
   int32_t mPermRedirectVisitBonus;
   int32_t mTempRedirectVisitBonus;
+  int32_t mRedirectSourceVisitBonus;
   int32_t mDefaultVisitBonus;
   int32_t mUnvisitedBookmarkBonus;
   int32_t mUnvisitedTypedBonus;
   int32_t mReloadVisitBonus;
 
   // in nsNavHistoryQuery.cpp
   nsresult TokensToQueries(const nsTArray<QueryKeyValuePair>& aTokens,
                            nsCOMArray<nsNavHistoryQuery>* aQueries,
--- a/toolkit/components/places/tests/browser/browser_redirect.js
+++ b/toolkit/components/places/tests/browser/browser_redirect.js
@@ -1,61 +1,117 @@
 /* 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/. */
 
-add_task(function* () {
-  const REDIRECT_URI = NetUtil.newURI("http://mochi.test:8888/tests/toolkit/components/places/tests/browser/redirect.sjs");
-  const TARGET_URI = NetUtil.newURI("http://mochi.test:8888/tests/toolkit/components/places/tests/browser/redirect-target.html");
+const REDIRECT_URI = NetUtil.newURI("http://mochi.test:8888/tests/toolkit/components/places/tests/browser/redirect.sjs");
+const TARGET_URI = NetUtil.newURI("http://mochi.test:8888/tests/toolkit/components/places/tests/browser/redirect-target.html");
 
+const REDIRECT_SOURCE_VISIT_BONUS =
+  Services.prefs.getIntPref("places.frecency.redirectSourceVisitBonus");
+const LINK_VISIT_BONUS =
+  Services.prefs.getIntPref("places.frecency.linkVisitBonus");
+const TYPED_VISIT_BONUS =
+  Services.prefs.getIntPref("places.frecency.typedVisitBonus");
+
+registerCleanupFunction(function*() {
+  yield PlacesTestUtils.clearHistory();
+});
+
+function promiseVisitedWithFrecency(expectedRedirectFrecency, expectedTargetFrecency) {
   // Create and add history observer.
-  let visitedPromise = new Promise(resolve => {
+  return new Promise(resolve => {
     let historyObserver = {
       _redirectNotified: false,
       onVisit(aURI, aVisitID, aTime, aSessionID, aReferringID,
-                        aTransitionType) {
-        info("Received onVisit: " + aURI.spec);
+                       aTransitionType) {
+       info("Received onVisit: " + aURI.spec);
 
-        if (aURI.equals(REDIRECT_URI)) {
-          this._redirectNotified = true;
-          // Wait for the target page notification.
-          return;
-        }
+       if (aURI.equals(REDIRECT_URI)) {
+         this._redirectNotified = true;
+         // Wait for the target page notification.
+         return;
+       }
 
-        PlacesUtils.history.removeObserver(historyObserver);
+       PlacesUtils.history.removeObserver(historyObserver);
 
-        ok(this._redirectNotified, "The redirect should have been notified");
+       ok(this._redirectNotified, "The redirect should have been notified");
 
-        fieldForUrl(REDIRECT_URI, "frecency", function(aFrecency) {
-          ok(aFrecency != 0, "Frecency or the redirecting page should not be 0");
+       fieldForUrl(REDIRECT_URI, "frecency", function(aFrecency) {
+         is(aFrecency, expectedRedirectFrecency,
+            "Frecency of the redirecting page is the expected one");
 
-          fieldForUrl(REDIRECT_URI, "hidden", function(aHidden) {
-            is(aHidden, 1, "The redirecting page should be hidden");
+         fieldForUrl(REDIRECT_URI, "hidden", function(aHidden) {
+           is(aHidden, 1, "The redirecting page should be hidden");
 
-            fieldForUrl(TARGET_URI, "frecency", function(aFrecency2) {
-              ok(aFrecency2 != 0, "Frecency of the target page should not be 0");
+           fieldForUrl(TARGET_URI, "frecency", function(aFrecency2) {
+             is(aFrecency2, expectedTargetFrecency,
+                "Frecency of the target page is the expected one");
 
-              fieldForUrl(TARGET_URI, "hidden", function(aHidden2) {
-                is(aHidden2, 0, "The target page should not be hidden");
-                resolve();
-              });
-            });
-          });
-        });
+             fieldForUrl(TARGET_URI, "hidden", function(aHidden2) {
+               is(aHidden2, 0, "The target page should not be hidden");
+               resolve();
+             });
+           });
+         });
+       });
       },
       onBeginUpdateBatch() {},
       onEndUpdateBatch() {},
       onTitleChanged() {},
       onDeleteURI() {},
       onClearHistory() {},
       onPageChanged() {},
       onDeleteVisits() {},
       QueryInterface: XPCOMUtils.generateQI([Ci.nsINavHistoryObserver])
     };
     PlacesUtils.history.addObserver(historyObserver, false);
   });
+}
+
+let expectedRedirectSourceFrecency = 0;
+let expectedTypedVisitBonus = 0;
+
+add_task(function* redirect_check_new_typed_visit() {
+  // Used to verify the redirect bonus overrides the typed bonus.
+  PlacesUtils.history.markPageAsTyped(REDIRECT_URI);
+
+  expectedRedirectSourceFrecency += REDIRECT_SOURCE_VISIT_BONUS;
+  expectedTypedVisitBonus += TYPED_VISIT_BONUS;
+
+  let visitedPromise = promiseVisitedWithFrecency(expectedRedirectSourceFrecency,
+                                                  expectedTypedVisitBonus);
 
   let newTabPromise = BrowserTestUtils.openNewForegroundTab(gBrowser, REDIRECT_URI.spec);
   yield Promise.all([visitedPromise, newTabPromise]);
 
-  yield PlacesTestUtils.clearHistory();
   gBrowser.removeCurrentTab();
 });
+
+add_task(function* redirect_check_second_typed_visit() {
+  // A second visit with a typed url.
+  PlacesUtils.history.markPageAsTyped(REDIRECT_URI);
+
+  expectedRedirectSourceFrecency += REDIRECT_SOURCE_VISIT_BONUS;
+  expectedTypedVisitBonus += TYPED_VISIT_BONUS;
+
+  let visitedPromise = promiseVisitedWithFrecency(expectedRedirectSourceFrecency,
+                                                  expectedTypedVisitBonus);
+
+  let newTabPromise = BrowserTestUtils.openNewForegroundTab(gBrowser, REDIRECT_URI.spec);
+  yield Promise.all([visitedPromise, newTabPromise]);
+
+  gBrowser.removeCurrentTab();
+});
+
+add_task(function* redirect_check_subsequent_link_visit() {
+  // Another visit, but this time as a visited url.
+  expectedRedirectSourceFrecency += REDIRECT_SOURCE_VISIT_BONUS;
+  expectedTypedVisitBonus += LINK_VISIT_BONUS;
+
+  let visitedPromise = promiseVisitedWithFrecency(expectedRedirectSourceFrecency,
+                                                  expectedTypedVisitBonus);
+
+  let newTabPromise = BrowserTestUtils.openNewForegroundTab(gBrowser, REDIRECT_URI.spec);
+  yield Promise.all([visitedPromise, newTabPromise]);
+
+  gBrowser.removeCurrentTab();
+});