mozilla
/
gecko-dev
зеркало из https://github.com/mozilla/gecko-dev.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Bug 1463132 - New autofill threshold doesn't work well with redirects. r=adw 

MozReview-Commit-ID: 9DqCWA2nGnz

--HG--
extra : rebase_source : 17e4cba78a34a7541d1ccc2345bff31e2d36fab5
This commit is contained in:
Marco Bonardo 2018-05-23 16:49:06 +02:00
Родитель fc99ada428
Коммит 4e62a21508
6 изменённых файлов: 152 добавлений и 135 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

14
docshell/base/IHistory.h
Просмотреть файл

@ -71,22 +71,28 @@ public:
*/
TOP_LEVEL = 1 << 0,
/**
* Indicates whether the URI was loaded as part of a permanent redirect.
* Indicates whether the URI is the target of a permanent redirect.
*/
REDIRECT_PERMANENT = 1 << 1,
/**
* Indicates whether the URI was loaded as part of a temporary redirect.
* Indicates whether the URI is the target of a temporary redirect.
*/
REDIRECT_TEMPORARY = 1 << 2,
/**
* Indicates the URI is redirecting (Response code 3xx).
* Indicates the URI will redirect (Response code 3xx).
*/
REDIRECT_SOURCE = 1 << 3,
/**
* Indicates the URI caused an error that is unlikely fixable by a
* retry, like a not found or unfetchable page.
*/
UNRECOVERABLE_ERROR = 1 << 4
UNRECOVERABLE_ERROR = 1 << 4,
/**
* If REDIRECT_SOURCE is set, this indicates that the redirect is permanent.
* Note this differs from REDIRECT_PERMANENT because that one refers to how
* we reached the URI, while this is used when the URI itself redirects.
*/
REDIRECT_SOURCE_PERMANENT = 1 << 5
};
/**

3
docshell/base/nsDocShell.cpp
Просмотреть файл

@ -12610,6 +12610,9 @@ nsDocShell::AddURIVisit(nsIURI* aURI,
if (aResponseStatus >= 300 && aResponseStatus < 400) {
visitURIFlags |= IHistory::REDIRECT_SOURCE;
if (aResponseStatus == 301 || aResponseStatus == 308) {
visitURIFlags |= IHistory::REDIRECT_SOURCE_PERMANENT;
}
}
// Errors 400-501 and 505 are considered unrecoverable, in the sense a
// simple retry attempt by the user is unlikely to solve them.

21
toolkit/components/places/History.cpp
Просмотреть файл

@ -83,7 +83,7 @@ struct VisitData {
, referrerVisitId(0)
, titleChanged(false)
, shouldUpdateFrecency(true)
, redirect(false)
, useFrecencyRedirectBonus(false)
{
guid.SetIsVoid(true);
title.SetIsVoid(true);
@ -105,7 +105,7 @@ struct VisitData {
, referrerVisitId(0)
, titleChanged(false)
, shouldUpdateFrecency(true)
, redirect(false)
, useFrecencyRedirectBonus(false)
{
MOZ_ASSERT(aURI);
if (aURI) {
@ -164,8 +164,9 @@ struct VisitData {
// Indicates whether frecency should be updated for this visit.
bool shouldUpdateFrecency;
// Whether this is a redirect source.
bool redirect;
// Whether to override the visit type bonus with a redirect bonus when
// calculating frecency on the most recent visit.
bool useFrecencyRedirectBonus;
};
////////////////////////////////////////////////////////////////////////////////
@ -1489,8 +1490,7 @@ private:
rv = stmt->BindInt64ByName(NS_LITERAL_CSTRING("page_id"), aPlace.placeId);
NS_ENSURE_SUCCESS(rv, rv);
rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("redirect"), aPlace.redirect);
rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("redirect"), aPlace.useFrecencyRedirectBonus);
NS_ENSURE_SUCCESS(rv, rv);
rv = stmt->Execute();
@ -2840,8 +2840,13 @@ History::VisitURI(nsIURI* aURI,
}
place.SetTransitionType(transitionType);
place.redirect = aFlags & IHistory::REDIRECT_SOURCE;
place.hidden = GetHiddenState(place.redirect, place.transitionType);
bool isRedirect = aFlags & IHistory::REDIRECT_SOURCE;
if (isRedirect) {
place.useFrecencyRedirectBonus =
(aFlags & IHistory::REDIRECT_SOURCE_PERMANENT) ||
transitionType != nsINavHistoryService::TRANSITION_TYPED;
}
place.hidden = GetHiddenState(isRedirect, place.transitionType);
// Error pages should never be autocompleted.
if (aFlags & IHistory::UNRECOVERABLE_ERROR) {

64
toolkit/components/places/SQLFunctions.cpp
Просмотреть файл

@ -676,16 +676,16 @@ namespace places {
return NS_OK;
}
enum RedirectState {
eRedirectUnknown,
eIsRedirect,
eIsNotRedirect
enum RedirectBonus {
eUnknown,
eRedirect,
eNormal
};
RedirectState isRedirect = eRedirectUnknown;
RedirectBonus mostRecentVisitBonus = eUnknown;
if (numEntries > 1) {
isRedirect = aArguments->AsInt32(1) ? eIsRedirect : eIsNotRedirect;
mostRecentVisitBonus = aArguments->AsInt32(1) ? eRedirect : eNormal;
}
int32_t typed = 0;
@ -735,8 +735,9 @@ namespace places {
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
// In case the visit is a redirect target, calculate the frecency
// as if the original page was visited.
// If it's a redirect source, we may want to use a lower bonus.
nsCString redirectsTransitionFragment =
nsPrintfCString("%d AND %d ", nsINavHistoryService::TRANSITION_REDIRECT_PERMANENT,
nsINavHistoryService::TRANSITION_REDIRECT_TEMPORARY);
@ -744,10 +745,9 @@ namespace places {
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), "
"origin.visit_type, "
"v.visit_type, "
"target.id NOTNULL "
"IFNULL(origin.visit_type, v.visit_type) AS visit_type, "
"target.visit_type AS target_visit_type, "
"ROUND((strftime('%s','now','localtime','utc') - v.visit_date/1000000)/86400) AS age_in_days "
"FROM moz_historyvisits v "
"LEFT JOIN moz_historyvisits origin ON origin.id = v.from_visit "
"AND v.visit_type BETWEEN "
@ -770,35 +770,23 @@ namespace places {
numSampledVisits < maxVisits &&
NS_SUCCEEDED(getVisits->ExecuteStep(&hasResult)) && hasResult;
numSampledVisits++) {
// If this is a redirect target, we'll use the visitType of the source,
// otherwise the actual visitType.
int32_t visitType = getVisits->AsInt32(0);
int32_t visitType;
bool isNull = false;
rv = getVisits->GetIsNull(1, &isNull);
NS_ENSURE_SUCCESS(rv, rv);
if (isRedirect == eIsRedirect || isNull) {
// Use the main visit_type.
rv = getVisits->GetInt32(2, &visitType);
NS_ENSURE_SUCCESS(rv, rv);
} else {
// This is a redirect target, so use the origin visit_type.
rv = getVisits->GetInt32(1, &visitType);
NS_ENSURE_SUCCESS(rv, rv);
// When adding a new visit, we should haved passed-in whether we should
// use the redirect bonus. We can't fetch this information from the
// database, because we only store redirect targets.
// For older visits we extract the value from the database.
bool useRedirectBonus = mostRecentVisitBonus == eRedirect;
if (mostRecentVisitBonus == eUnknown || numSampledVisits > 0) {
int32_t targetVisitType = getVisits->AsInt32(1);
useRedirectBonus = targetVisitType == nsINavHistoryService::TRANSITION_REDIRECT_PERMANENT ||
(targetVisitType == nsINavHistoryService::TRANSITION_REDIRECT_TEMPORARY &&
visitType != nsINavHistoryService::TRANSITION_TYPED);
}
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(3, &redirect);
NS_ENSURE_SUCCESS(rv, rv);
visitIsRedirect = !!redirect ? eIsRedirect : eIsNotRedirect;
}
bonus = history->GetFrecencyTransitionBonus(visitType, true, visitIsRedirect == eIsRedirect);
bonus = history->GetFrecencyTransitionBonus(visitType, true, useRedirectBonus);
// Add the bookmark visit bonus.
if (hasBookmark) {
@ -807,7 +795,7 @@ namespace places {
// If bonus was zero, we can skip the work to determine the weight.
if (bonus) {
int32_t ageInDays = getVisits->AsInt32(0);
int32_t ageInDays = getVisits->AsInt32(2);
int32_t weight = history->GetFrecencyAgedWeight(ageInDays);
pointsForSampledVisits += (float)(weight * (bonus / 100.0));
}

5
toolkit/components/places/SQLFunctions.h
Просмотреть файл

@ -198,8 +198,9 @@ private:
*
* @param {int64_t} pageId
* The id of the page. Pass -1 if the page is being added right now.
* @param [optional] {int32_t} redirect
* Whether the page visit is a redirect. Default is false.
* @param [optional] {int32_t} useRedirectBonus
* Whether we should use the lower redirect bonus for the most recent
* page visit. If not passed in, it will use a database guess.
*/
class CalculateFrecencyFunction final : public mozIStorageFunction
{

180
toolkit/components/places/tests/browser/browser_multi_redirect_frecency.js
Просмотреть файл

@ -11,6 +11,8 @@ const REDIRECT_SOURCE_VISIT_BONUS =
Services.prefs.getIntPref("places.frecency.redirectSourceVisitBonus");
const PERM_REDIRECT_VISIT_BONUS =
Services.prefs.getIntPref("places.frecency.permRedirectVisitBonus");
const TYPED_VISIT_BONUS =
Services.prefs.getIntPref("places.frecency.typedVisitBonus");
// Ensure that decay frecency doesn't kick in during tests (as a result
// of idle-daily).
@ -28,21 +30,9 @@ async function check_uri(uri, frecency, hidden) {
"Hidden value of the page is the expected one");
}
let redirectSourceFrecency = 0;
let redirectTargetFrecency = 0;
add_task(async function test_multiple_redirect() {
// Used to verify the redirect bonus overrides the typed bonus.
PlacesUtils.history.markPageAsTyped(REDIRECT_URI);
redirectSourceFrecency += REDIRECT_SOURCE_VISIT_BONUS;
// TODO Bug 487813 - This should be TYPED_VISIT_BONUS, however as we don't
// currently track redirects across multiple redirects, we fallback to the
// PERM_REDIRECT_VISIT_BONUS.
redirectTargetFrecency += PERM_REDIRECT_VISIT_BONUS;
async function waitVisitedNotifications() {
let redirectNotified = false;
let visitedPromise = PlacesTestUtils.waitForNotification("onVisits", visits => {
await PlacesTestUtils.waitForNotification("onVisits", visits => {
is(visits.length, 1, "Was notified for the right number of visits.");
let {uri} = visits[0];
info("Received onVisits: " + uri.spec);
@ -51,83 +41,107 @@ add_task(async function test_multiple_redirect() {
}
return uri.equals(TARGET_URI);
}, "history");
return redirectNotified;
}
let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, REDIRECT_URI.spec);
info("Waiting for onVisits");
await visitedPromise;
ok(redirectNotified, "The redirect should have been notified");
let firstRedirectBonus = 0;
let nextRedirectBonus = 0;
let targetBonus = 0;
await check_uri(REDIRECT_URI, redirectSourceFrecency, 1);
await check_uri(INTERMEDIATE_URI_1, redirectSourceFrecency, 1);
await check_uri(INTERMEDIATE_URI_2, redirectSourceFrecency, 1);
await check_uri(TARGET_URI, redirectTargetFrecency, 0);
add_task(async function test_multiple_redirect() {
// The redirect source bonus overrides the link bonus.
let visitedPromise = waitVisitedNotifications();
await BrowserTestUtils.withNewTab({
gBrowser,
url: REDIRECT_URI.spec,
}, async function() {
info("Waiting for onVisits");
let redirectNotified = await visitedPromise;
ok(redirectNotified, "The redirect should have been notified");
BrowserTestUtils.removeTab(tab);
firstRedirectBonus += REDIRECT_SOURCE_VISIT_BONUS;
await check_uri(REDIRECT_URI, firstRedirectBonus, 1);
nextRedirectBonus += REDIRECT_SOURCE_VISIT_BONUS;
await check_uri(INTERMEDIATE_URI_1, nextRedirectBonus, 1);
await check_uri(INTERMEDIATE_URI_2, nextRedirectBonus, 1);
// TODO Bug 487813 - This should be TYPED_VISIT_BONUS, however as we don't
// currently track redirects across multiple redirects, we fallback to the
// PERM_REDIRECT_VISIT_BONUS.
targetBonus += PERM_REDIRECT_VISIT_BONUS;
await check_uri(TARGET_URI, targetBonus, 0);
});
});
add_task(async function redirect_check_second_typed_visit() {
// A second visit with a typed url.
add_task(async function test_multiple_redirect_typed() {
// The typed bonus wins because the redirect is permanent.
PlacesUtils.history.markPageAsTyped(REDIRECT_URI);
let visitedPromise = waitVisitedNotifications();
await BrowserTestUtils.withNewTab({
gBrowser,
url: REDIRECT_URI.spec,
}, async function() {
info("Waiting for onVisits");
let redirectNotified = await visitedPromise;
ok(redirectNotified, "The redirect should have been notified");
redirectSourceFrecency += REDIRECT_SOURCE_VISIT_BONUS;
// TODO Bug 487813 - This should be TYPED_VISIT_BONUS, however as we don't
// currently track redirects across multiple redirects, we fallback to the
// PERM_REDIRECT_VISIT_BONUS.
redirectTargetFrecency += PERM_REDIRECT_VISIT_BONUS;
let redirectNotified = false;
let visitedPromise = PlacesTestUtils.waitForNotification("onVisits", visits => {
Assert.equal(visits.length, 1, "Was notified for the right number of visits.");
let {uri} = visits[0];
info("Received onVisits: " + uri.spec);
if (uri.equals(REDIRECT_URI)) {
redirectNotified = true;
}
return uri.equals(TARGET_URI);
}, "history");
let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, REDIRECT_URI.spec);
info("Waiting for onVisits");
await visitedPromise;
ok(redirectNotified, "The redirect should have been notified");
await check_uri(REDIRECT_URI, redirectSourceFrecency, 1);
await check_uri(INTERMEDIATE_URI_1, redirectSourceFrecency, 1);
await check_uri(INTERMEDIATE_URI_2, redirectSourceFrecency, 1);
await check_uri(TARGET_URI, redirectTargetFrecency, 0);
BrowserTestUtils.removeTab(tab);
firstRedirectBonus += TYPED_VISIT_BONUS;
await check_uri(REDIRECT_URI, firstRedirectBonus, 1);
nextRedirectBonus += REDIRECT_SOURCE_VISIT_BONUS;
await check_uri(INTERMEDIATE_URI_1, nextRedirectBonus, 1);
await check_uri(INTERMEDIATE_URI_2, nextRedirectBonus, 1);
// TODO Bug 487813 - This should be TYPED_VISIT_BONUS, however as we don't
// currently track redirects across multiple redirects, we fallback to the
// PERM_REDIRECT_VISIT_BONUS.
targetBonus += PERM_REDIRECT_VISIT_BONUS;
await check_uri(TARGET_URI, targetBonus, 0);
});
});
add_task(async function test_second_typed_visit() {
// The typed bonus wins because the redirect is permanent.
PlacesUtils.history.markPageAsTyped(REDIRECT_URI);
let visitedPromise = waitVisitedNotifications();
await BrowserTestUtils.withNewTab({
gBrowser,
url: REDIRECT_URI.spec,
}, async function() {
info("Waiting for onVisits");
let redirectNotified = await visitedPromise;
ok(redirectNotified, "The redirect should have been notified");
add_task(async function redirect_check_subsequent_link_visit() {
// Another visit, but this time as a visited url.
redirectSourceFrecency += REDIRECT_SOURCE_VISIT_BONUS;
// TODO Bug 487813 - This should be TYPED_VISIT_BONUS, however as we don't
// currently track redirects across multiple redirects, we fallback to the
// PERM_REDIRECT_VISIT_BONUS.
redirectTargetFrecency += PERM_REDIRECT_VISIT_BONUS;
let redirectNotified = false;
let visitedPromise = PlacesTestUtils.waitForNotification("onVisits", visits => {
Assert.equal(visits.length, 1, "Was notified for the right number of visits.");
let {uri} = visits[0];
info("Received onVisits: " + uri.spec);
if (uri.equals(REDIRECT_URI)) {
redirectNotified = true;
}
return uri.equals(TARGET_URI);
}, "history");
let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, REDIRECT_URI.spec);
info("Waiting for onVisits");
await visitedPromise;
ok(redirectNotified, "The redirect should have been notified");
await check_uri(REDIRECT_URI, redirectSourceFrecency, 1);
await check_uri(INTERMEDIATE_URI_1, redirectSourceFrecency, 1);
await check_uri(INTERMEDIATE_URI_2, redirectSourceFrecency, 1);
await check_uri(TARGET_URI, redirectTargetFrecency, 0);
BrowserTestUtils.removeTab(tab);
firstRedirectBonus += TYPED_VISIT_BONUS;
await check_uri(REDIRECT_URI, firstRedirectBonus, 1);
nextRedirectBonus += REDIRECT_SOURCE_VISIT_BONUS;
await check_uri(INTERMEDIATE_URI_1, nextRedirectBonus, 1);
await check_uri(INTERMEDIATE_URI_2, nextRedirectBonus, 1);
// TODO Bug 487813 - This should be TYPED_VISIT_BONUS, however as we don't
// currently track redirects across multiple redirects, we fallback to the
// PERM_REDIRECT_VISIT_BONUS.
targetBonus += PERM_REDIRECT_VISIT_BONUS;
await check_uri(TARGET_URI, targetBonus, 0);
});
});
add_task(async function test_subsequent_link_visit() {
// Another non typed visit.
let visitedPromise = waitVisitedNotifications();
await BrowserTestUtils.withNewTab({
gBrowser,
url: REDIRECT_URI.spec,
}, async function() {
info("Waiting for onVisits");
let redirectNotified = await visitedPromise;
ok(redirectNotified, "The redirect should have been notified");
firstRedirectBonus += REDIRECT_SOURCE_VISIT_BONUS;
await check_uri(REDIRECT_URI, firstRedirectBonus, 1);
nextRedirectBonus += REDIRECT_SOURCE_VISIT_BONUS;
await check_uri(INTERMEDIATE_URI_1, nextRedirectBonus, 1);
await check_uri(INTERMEDIATE_URI_2, nextRedirectBonus, 1);
// TODO Bug 487813 - This should be TYPED_VISIT_BONUS, however as we don't
// currently track redirects across multiple redirects, we fallback to the
// PERM_REDIRECT_VISIT_BONUS.
targetBonus += PERM_REDIRECT_VISIT_BONUS;
await check_uri(TARGET_URI, targetBonus, 0);
});
});