Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-19 00:05:36 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1173748 - Part 2: mixup search suggestions at a given frecency threshold. r=adw

Browse Source
This commit is contained in:
Marco Bonardo 2015-07-03 13:37:41 +02:00
parent 1560d8c7ff
commit 6517f36330
4 changed files with 250 additions and 126 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
browser/base/content/urlbarBindings.xml
View File

@ -1631,8 +1631,12 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/.
if (!Services.prefs.getBoolPref("browser.urlbar.unifiedcomplete"))
return;
if (this._matchCount > 0 && this.selectedIndex == -1)
if (this._matchCount > 0 && this.selectedIndex == -1) {
// Don't handle this as a user-initiated action.
this._ignoreNextSelect = true;
this.selectedIndex = 0;
this._ignoreNextSelect = false;
}
this.input.gotResultForCurrentQuery = true;
if (this.input.handleEnterWhenGotResult) {
@ -1644,6 +1648,18 @@ file, You can obtain one at http://mozilla.org/MPL/2.0/.
</method>
</implementation>
<handlers>
<handler event="select"><![CDATA[
// When the user selects one of matches, stop the search to avoid
// changing the underlying result unexpectedly.
if (!this._ignoreNextSelect && this.selectedIndex >= 0) {
let controller = this.view.QueryInterface(Components.interfaces.nsIAutoCompleteController);
controller.stopSearch();
}
]]></handler>
</handlers>
</binding>
<binding id="addon-progress-notification" extends="chrome://global/content/bindings/notification.xml#popup-notification">

239
toolkit/components/places/UnifiedComplete.js
View File

@ -67,12 +67,10 @@ const TELEMETRY_6_FIRST_RESULTS = "PLACES_AUTOCOMPLETE_6_FIRST_RESULTS_TIME_MS";
// The default frecency value used when inserting matches with unknown frecency.
const FRECENCY_DEFAULT = 1000;
// Search suggestion results are mixed in with all other results after they
// become available, but they're only inserted once every N results whose
// frecencies are less than FRECENCY_DEFAULT. In other words, for every N
// results that fall below that frecency threshold, one search suggestion is
// inserted. This value = N.
const SEARCH_SUGGESTION_INSERT_INTERVAL = 2;
// Remote matches are appended when local matches are below a given frecency
// threshold (FRECENCY_DEFAULT) as soon as they arrive. However we'll
// always try to have at least MINIMUM_LOCAL_MATCHES local matches.
const MINIMUM_LOCAL_MATCHES = 5;
// A regex that matches "single word" hostnames for whitelisting purposes.
// The hostname will already have been checked for general validity, so we
@ -259,8 +257,8 @@ XPCOMUtils.defineLazyModuleGetter(this, "Sqlite",
"resource://gre/modules/Sqlite.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "OS",
"resource://gre/modules/osfile.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "Promise",
"resource://gre/modules/Promise.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "PromiseUtils",
"resource://gre/modules/PromiseUtils.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "Task",
"resource://gre/modules/Task.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "PlacesSearchAutocompleteProvider",
@ -619,7 +617,16 @@ function Search(searchString, searchParam, autocompleteListener,
this._usedURLs = new Set();
this._usedPlaceIds = new Set();
this._searchSuggestionInsertCounter = 0;
// Resolved when all the remote matches have been fetched.
this._remoteMatchesPromises = [];
// The index to insert remote matches at.
this._remoteMatchesStartIndex = 0;
// Counts the number of inserted local matches.
this._localMatchesCount = 0;
// Counts the number of inserted remote matches.
this._remoteMatchesCount = 0;
}
Search.prototype = {
@ -668,7 +675,7 @@ Search.prototype = {
// the first query.
if (!this._sleepTimer)
this._sleepTimer = Cc["@mozilla.org/timer;1"].createInstance(Ci.nsITimer);
this._sleepDeferred = Promise.defer();
this._sleepDeferred = PromiseUtils.defer();
this._sleepTimer.initWithCallback(() => this._sleepDeferred.resolve(),
aTimeMs, Ci.nsITimer.TYPE_ONE_SHOT);
return this._sleepDeferred.promise;
@ -714,11 +721,11 @@ Search.prototype = {
},
/**
* Cancels this search.
* Stop this search.
* After invoking this method, we won't run any more searches or heuristics,
* and no new matches may be added to the current result.
*/
cancel: function () {
stop() {
if (this._sleepTimer)
this._sleepTimer.cancel();
if (this._sleepDeferred) {
@ -747,9 +754,9 @@ Search.prototype = {
if (!this.pending)
return;
TelemetryStopwatch.start(TELEMETRY_1ST_RESULT);
TelemetryStopwatch.start(TELEMETRY_1ST_RESULT, this);
if (this._searchString)
TelemetryStopwatch.start(TELEMETRY_6_FIRST_RESULTS);
TelemetryStopwatch.start(TELEMETRY_6_FIRST_RESULTS, this);
// Since we call the synchronous parseSubmissionURL function later, we must
// wait for the initialization of PlacesSearchAutocompleteProvider first.
@ -830,28 +837,11 @@ Search.prototype = {
// IMPORTANT: No other first result heuristics should run after
// _matchHeuristicFallback().
yield this._sleep(Math.round(Prefs.delay / 2));
yield this._sleep(Prefs.delay);
if (!this.pending)
return;
// Start fetching search suggestions a little earlier than Prefs.delay since
// they're remote and will probably take longer to arrive.
if (this.hasBehavior("searches")) {
this._searchSuggestionController =
PlacesSearchAutocompleteProvider.getSuggestionController(
this._searchTokens.join(" "),
this._inPrivateWindow,
Prefs.maxRichResults
);
if (this.hasBehavior("restrict")) {
// We're done if we're restricting to search suggestions.
yield this._consumeAllSearchSuggestions();
return;
}
}
yield this._sleep(Math.round(Prefs.delay / 2));
yield this._matchSearchSuggestions();
if (!this.pending)
return;
@ -873,7 +863,7 @@ Search.prototype = {
// MATCH_BOUNDARY_ANYWHERE, search again with MATCH_ANYWHERE to get more
// results.
if (this._matchBehavior == MATCH_BOUNDARY_ANYWHERE &&
this._result.matchCount < Prefs.maxRichResults) {
this._localMatchesCount < Prefs.maxRichResults) {
this._matchBehavior = MATCH_ANYWHERE;
for (let [query, params] of [ this._adaptiveQuery,
this._searchQuery ]) {
@ -883,17 +873,40 @@ Search.prototype = {
}
}
// If we still don't have enough results, fill the remaining space with
// search suggestions.
yield this._consumeAllSearchSuggestions();
// Ensure to fill any remaining space.
yield Promise.all(this._remoteMatchesPromises);
}),
_consumeAllSearchSuggestions: Task.async(function* () {
if (this._searchSuggestionController && this.pending) {
yield this._searchSuggestionController.fetchCompletePromise;
while (this.pending && this._maybeAddSearchSuggestionMatch());
*_matchSearchSuggestions() {
if (!this.hasBehavior("searches"))
return;
this._searchSuggestionController =
PlacesSearchAutocompleteProvider.getSuggestionController(
this._searchTokens.join(" "),
this._inPrivateWindow,
Prefs.maxRichResults
);
let promise = this._searchSuggestionController.fetchCompletePromise
.then(() => {
while (this.pending && this._remoteMatchesCount < Prefs.maxRichResults) {
let [match, suggestion] = this._searchSuggestionController.consume();
if (!suggestion)
break;
// Don't include the restrict token, if present.
let searchString = this._searchTokens.join(" ");
this._addSearchEngineMatch(match, searchString, suggestion);
}
});
if (this.hasBehavior("restrict")) {
// We're done if we're restricting to search suggestions.
yield promise;
this.stop();
} else {
this._remoteMatchesPromises.push(promise);
}
}),
},
_matchKnownUrl: function* (conn, queries) {
// Hosts have no "/" in them.
@ -1054,6 +1067,7 @@ Search.prototype = {
icon: match.iconUrl,
style: "action searchengine",
frecency: FRECENCY_DEFAULT,
remote: !!suggestion
});
},
@ -1137,7 +1151,7 @@ Search.prototype = {
},
_onResultRow: function (row) {
TelemetryStopwatch.finish(TELEMETRY_1ST_RESULT);
TelemetryStopwatch.finish(TELEMETRY_1ST_RESULT, this);
let queryType = row.getResultByIndex(QUERYINDEX_QUERYTYPE);
let match;
switch (queryType) {
@ -1156,9 +1170,9 @@ Search.prototype = {
break;
}
this._addMatch(match);
// If the search has been canceled by the user or by _addMatch reaching the
// maximum number of results, we can stop the underlying Sqlite query.
if (!this.pending)
// If the search has been canceled by the user or by _addMatch, or we
// fetched enough results, we can stop the underlying Sqlite query.
if (!this.pending || this._localMatchesCount == Prefs.maxRichResults)
throw StopIteration;
},
@ -1186,89 +1200,72 @@ Search.prototype = {
parseResult.engineName;
},
_maybeAddSearchSuggestionMatch() {
if (this._searchSuggestionController) {
let [match, suggestion] = this._searchSuggestionController.consume();
if (suggestion) {
// Don't include the restrict token, if present.
let searchString = this._searchTokens.join(" ");
this._addSearchEngineMatch(match, searchString, suggestion);
return true;
}
}
return false;
},
_addMatch: function (match) {
_addMatch(match) {
// A search could be canceled between a query start and its completion,
// in such a case ensure we won't notify any result for it.
if (!this.pending)
return;
// Mix in search suggestions. Insert one suggestion every N non-suggestion
// matches that fall below the default frecency, and start inserting them as
// soon as they become available. N = SEARCH_SUGGESTION_INSERT_INTERVAL.
if (match.frecency < FRECENCY_DEFAULT) {
if (this._searchSuggestionInsertCounter %
SEARCH_SUGGESTION_INSERT_INTERVAL == 0) {
// Search engine matches are created with FRECENCY_DEFAULT, so there's
// no danger of infinite indirect recursion.
if (this._maybeAddSearchSuggestionMatch()) {
if (!this.pending) {
return;
}
this._searchSuggestionInsertCounter++;
}
} else {
this._searchSuggestionInsertCounter++;
}
}
let notifyResults = false;
// Must check both id and url, cause keywords dinamically modify the url.
// Must check both id and url, cause keywords dynamically modify the url.
let urlMapKey = stripHttpAndTrim(match.value);
if ((!match.placeId || !this._usedPlaceIds.has(match.placeId)) &&
!this._usedURLs.has(urlMapKey)) {
// Add this to our internal tracker to ensure duplicates do not end up in
// the result.
// Not all entries have a place id, thus we fallback to the url for them.
// We cannot use only the url since keywords entries are modified to
// include the search string, and would be returned multiple times. Ids
// are faster too.
if (match.placeId)
this._usedPlaceIds.add(match.placeId);
this._usedURLs.add(urlMapKey);
if (!match.style) {
match.style = "favicon";
}
// Restyle past searches, unless they are bookmarks or special results.
if (Prefs.restyleSearches && match.style == "favicon") {
this._maybeRestyleSearchMatch(match);
}
this._result.appendMatch(match.value,
match.comment,
match.icon || PlacesUtils.favicons.defaultFavicon.spec,
match.style,
match.finalCompleteValue || "");
notifyResults = true;
if ((match.placeId && this._usedPlaceIds.has(match.placeId)) ||
this._usedURLs.has(urlMapKey)) {
return;
}
// Add this to our internal tracker to ensure duplicates do not end up in
// the result.
// Not all entries have a place id, thus we fallback to the url for them.
// We cannot use only the url since keywords entries are modified to
// include the search string, and would be returned multiple times. Ids
// are faster too.
if (match.placeId)
this._usedPlaceIds.add(match.placeId);
this._usedURLs.add(urlMapKey);
match.style = match.style || "favicon";
// Restyle past searches, unless they are bookmarks or special results.
if (Prefs.restyleSearches && match.style == "favicon") {
this._maybeRestyleSearchMatch(match);
}
match.icon = match.icon || PlacesUtils.favicons.defaultFavicon.spec;
match.finalCompleteValue = match.finalCompleteValue || "";
this._result.insertMatchAt(this._getInsertIndexForMatch(match),
match.value,
match.comment,
match.icon,
match.style,
match.finalCompleteValue);
if (this._result.matchCount == 6)
TelemetryStopwatch.finish(TELEMETRY_6_FIRST_RESULTS);
TelemetryStopwatch.finish(TELEMETRY_6_FIRST_RESULTS, this);
if (this._result.matchCount == Prefs.maxRichResults) {
// We have enough results, so stop running our search.
// We don't need to notify results in this case, cause the main promise
// chain will do that for us when finishSearch is invoked.
this.cancel();
} else if (notifyResults) {
// Notify about results if we've gotten them.
this.notifyResults(true);
this.notifyResults(true);
},
_getInsertIndexForMatch(match) {
let index = 0;
if (match.remote) {
// Append after local matches.
index = this._remoteMatchesStartIndex + this._remoteMatchesCount;
this._remoteMatchesCount++;
} else {
// This is a local match.
if (match.frecency > FRECENCY_DEFAULT ||
this._localMatchesCount < MINIMUM_LOCAL_MATCHES) {
// Append before remote matches.
index = this._remoteMatchesStartIndex;
this._remoteMatchesStartIndex++
} else {
// Append after remote matches.
index = this._localMatchesCount + this._remoteMatchesCount;
}
this._localMatchesCount++;
}
return index;
},
_processHostRow: function (row) {
@ -1748,7 +1745,7 @@ UnifiedComplete.prototype = {
stopSearch: function () {
if (this._currentSearch) {
this._currentSearch.cancel();
this._currentSearch.stop();
}
// Don't notify since we are canceling this search. This also means we
// won't fire onSearchComplete for this search.
@ -1763,8 +1760,8 @@ UnifiedComplete.prototype = {
* results or not.
*/
finishSearch: function (notify=false) {
TelemetryStopwatch.cancel(TELEMETRY_1ST_RESULT);
TelemetryStopwatch.cancel(TELEMETRY_6_FIRST_RESULTS);
TelemetryStopwatch.cancel(TELEMETRY_1ST_RESULT, this);
TelemetryStopwatch.cancel(TELEMETRY_6_FIRST_RESULTS, this);
// Clear state now to avoid race conditions, see below.
let search = this._currentSearch;
delete this._currentSearch;

12
toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js
View File

@ -7,6 +7,8 @@ const Cc = Components.classes;
const Cr = Components.results;
const Cu = Components.utils;
const FRECENCY_DEFAULT = 10000;
Cu.import("resource://gre/modules/Services.jsm");
Cu.import("resource://testing-common/httpd.js");
@ -216,8 +218,10 @@ function* check_autocomplete(test) {
image: controller.getImageAt(i),
}
do_print(`Looking for "${result.value}", "${result.comment}" in expected results...`);
let j;
for (j = firstIndexToCheck; j < matches.length; j++) {
let lowerBound = test.checkSorting ? i : firstIndexToCheck;
let upperBound = test.checkSorting ? i + 1 : matches.length;
let found = false;
for (let j = lowerBound; j < upperBound; ++j) {
// Skip processed expected results
if (matches[j] == undefined)
continue;
@ -225,12 +229,12 @@ function* check_autocomplete(test) {
do_print("Got a match at index " + j + "!");
// Make it undefined so we don't process it again
matches[j] = undefined;
found = true;
break;
}
}
// We didn't hit the break, so we must have not found it
if (j == matches.length)
if (!found)
do_throw(`Didn't find the current result ("${result.value}", "${result.comment}") in matches`); //' (Emacs syntax highlighting fix)
}

107
toolkit/components/places/tests/unifiedcomplete/test_searchSuggestions.js
View File

@ -285,3 +285,110 @@ add_task(function* restrictToken() {
yield cleanUpSuggestions();
});
add_task(function* mixup_frecency() {
Services.prefs.setBoolPref(SUGGEST_PREF, true);
// Add a visit and a bookmark. Actually, make the bookmark visited too so
// that it's guaranteed, with its higher frecency, to appear above the search
// suggestions.
yield PlacesTestUtils.addVisits([
{ uri: NetUtil.newURI("http://example.com/lo0"),
title: "low frecency 0" },
{ uri: NetUtil.newURI("http://example.com/lo1"),
title: "low frecency 1" },
{ uri: NetUtil.newURI("http://example.com/lo2"),
title: "low frecency 2" },
{ uri: NetUtil.newURI("http://example.com/lo3"),
title: "low frecency 3" },
{ uri: NetUtil.newURI("http://example.com/lo4"),
title: "low frecency 4" },
]);
for (let i = 0; i < 4; i++) {
let href = `http://example.com/lo${i}`;
let frecency = frecencyForUrl(href);
Assert.ok(frecency < FRECENCY_DEFAULT,
`frecency for ${href}: ${frecency}, should be lower than ${FRECENCY_DEFAULT}`);
}
for (let i = 0; i < 5; i++) {
yield PlacesTestUtils.addVisits([
{ uri: NetUtil.newURI("http://example.com/hi0"),
title: "high frecency 0",
transition: TRANSITION_TYPED },
{ uri: NetUtil.newURI("http://example.com/hi1"),
title: "high frecency 1",
transition: TRANSITION_TYPED },
{ uri: NetUtil.newURI("http://example.com/hi2"),
title: "high frecency 2",
transition: TRANSITION_TYPED },
{ uri: NetUtil.newURI("http://example.com/hi3"),
title: "high frecency 3",
transition: TRANSITION_TYPED },
]);
}
for (let i = 0; i < 4; i++) {
let href = `http://example.com/hi${i}`;
yield addBookmark({ uri: href, title: `high frecency ${i}` });
let frecency = frecencyForUrl(href);
Assert.ok(frecency > FRECENCY_DEFAULT,
`frecency for ${href}: ${frecency}, should be higher than ${FRECENCY_DEFAULT}`);
}
// Do an unrestricted search to make sure everything appears in it, including
// the visit and bookmark.
yield check_autocomplete({
checkSorting: true,
search: "frecency",
matches: [
{ uri: NetUtil.newURI("http://example.com/hi3"),
title: "high frecency 3",
style: [ "bookmark" ] },
{ uri: NetUtil.newURI("http://example.com/hi2"),
title: "high frecency 2",
style: [ "bookmark" ] },
{ uri: NetUtil.newURI("http://example.com/hi1"),
title: "high frecency 1",
style: [ "bookmark" ] },
{ uri: NetUtil.newURI("http://example.com/hi0"),
title: "high frecency 0",
style: [ "bookmark" ] },
{ uri: NetUtil.newURI("http://example.com/lo4"),
title: "low frecency 4" },
{
uri: makeActionURI(("searchengine"), {
engineName: ENGINE_NAME,
input: "frecency foo",
searchQuery: "frecency",
searchSuggestion: "frecency foo",
}),
title: ENGINE_NAME,
style: ["action", "searchengine"],
icon: "",
},
{
uri: makeActionURI(("searchengine"), {
engineName: ENGINE_NAME,
input: "frecency bar",
searchQuery: "frecency",
searchSuggestion: "frecency bar",
}),
title: ENGINE_NAME,
style: ["action", "searchengine"],
icon: "",
},
{ uri: NetUtil.newURI("http://example.com/lo3"),
title: "low frecency 3" },
{ uri: NetUtil.newURI("http://example.com/lo2"),
title: "low frecency 2" },
{ uri: NetUtil.newURI("http://example.com/lo1"),
title: "low frecency 1" },
{ uri: NetUtil.newURI("http://example.com/lo0"),
title: "low frecency 0" },
],
});
yield cleanUpSuggestions();
});