1754591 - Keep track of blocked quick suggest suggestions

Status: RESOLVED FIXED

(Firefox :: Address Bar, task, P1)

Description

[Drew Willcoxon :adw]

This bug is for the following:

• Set up a data store for keeping track of blocked quick suggest suggestions.
• Respond to clicks on the block button in best match rows by adding the suggestions to the data store.

These are two separate tasks, but I don't think it's necessarily helpful to break them into two different bugs. Maybe the data store should be initialized the first time a suggestion is blocked, for example. And if we go the JSON route, there's not much to setting up the store.

The first part can procede in parallel to adding the block button to the view (bug 1754588), but the second part depends on it obviously.

Notes:

• The spec calls for blocking best matches only, but blocking is per suggestion, so there's no reason to tie the implementation to best match. And in the future we may need to allow non-best-match suggestions to be blocked.
• We could maybe get away with keeping a JSON file on disk for this, at least for the MVP. That would mean the whole data store would be in memory, but we already keep all suggestion data in memory anyway. I was thinking a SQLite database, but that seems like overkill right now. The use case doesn't require any relational operations on the data, and there's no guarantee the memory footprint of a SQLite connection would be smaller than a simple JS object or Map. I would expect the size of the data to be small also.

Comment 1

[Drew Willcoxon :adw]

Attachment: Bug 1754591 - Store blocked quick suggest suggestions.

This stores blocked suggestions. It's a big patch that does the following:

• Call UrlbarController.handleDeleteEntry() when blocking results instead of
  simply removing them from the view. One thing I didn't realize when reviewing
  D139097 is that we need to remove the result from context.results, not only
  the view. handleDeleteEntry() already does that, and I think it makes sense
  for all code paths that block/remove results to go through it.

• Modify handleDeleteEntry() to call a new provider.blockResult() method
  that providers can implement to block/remove results. The quick suggest
  provider implements it to store blocked suggestions. In the future other
  providers could implement it too.

• Add a new browser.urlbar.quickSuggest.blockedDigests pref to store blocked
  suggestions. I talked with Nan about how we might store them, and he pointed
  to the example of how blocked tiles on the newtab page are stored, which is by
  MD5 digests of their URLs in a pref. Before I talked to him, I was planning on
  storing blocked data in a JSON file, but he said tiles used to do that and it
  became a performance problem as the file got big. Re: digests, we could store
  URLs themselves instead, but digest strings are always 40 characters, while
  the average URL length in the current suggestions is ~60, so we save a little
  space, and it's consistent with the tiles implementation. Ideally suggestions
  would have inherent persistent IDs we could use instead, but they do not. I
  also talked with Nan about asking adM for that, so we might get that in the
  future.

• Add an originalUrl property to quick suggest result payloads so we can keep
  track of the original URL as provided by the backing source with unreplaced
  timestamp template substrings. The difference between url and originalUrl
  is that in url, the timestamp template is replaced with an actual timestamp.
  url is therefore not suitable for using as the basis of a persistent ID.
  See the comment for isURLEquivalentToResultURL() for more on templates.

• Add TIMESTAMP_TEMPLATE and TIMESTAMP_LENGTH getters to the provider class
  so we don't have to copy-paste them when using them in tests.

• Factor out a new TaskQueue class from the task queue stuff used inside
  UrlbarQuickSuggest. The quick suggest provider uses this to serialize access
  to blocked suggestions, since getting suggestion URL digests is async.

• For tests, move the import of UrlbarProviderQuickSuggest to head.js so
  individual tests don't need to import it.

• This does not prevent blocked suggestions from being returned by the provider
  because that will be covered by bug 1754595. It should be straightforward to
  implement that now using isSuggestionBlocked().

Comment 2

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=3e8abf11221d9d783e989eb3a834bbd845c63a8a

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ebc274068268
Store blocked quick suggest suggestions. r=daisuke

Comment 4

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/ebc274068268

Comment 5

[Drew Willcoxon :adw]

STR for QA:

The STR are similar to bug 1754594. This bug does not prevent blocked suggestions from showing up in new searches, so that can't be tested yet. Bug 1754595 will cover that.

1. Enable Firefox Suggest
2. Enable best match by setting browser.urlbar.bestMatch.enabled to true
3. Type "amazon" to trigger a best match
4. Pick the thumbs-down block button in the best match
5. Verify the best match row disappears
6. Verify the browser.urlbar.quickSuggest.blockedDigests pref value is: ["8fa0c37f5110e5ab0365bd07e5be7fd3bb2b17ef"]
7. Type "betty" to trigger a Wikipedia best match
8. Pick the thumbs-down block button in the best match
9. Verify the best match row disappears
10. Verify the browser.urlbar.quickSuggest.blockedDigests pref value is: ["8fa0c37f5110e5ab0365bd07e5be7fd3bb2b17ef","e97806b6cf95243c669a6a0047c22da72213148f"]