The first is already implemented.

Awesome :)

(Note that we don't have an explicit visited state, we just rely on the history DB. We do have code in SnippetsService already though that looks up a URL in the history DB).

So, are the following both implemented already?

- snippets that are already existing on the client (in the current set of snippets on the client)
- snippets that have already been visited (according to local  browsing history and the visited state of snippets in the current set of snippets on the client)

Jan, another candidate to look into.

Bernhard, I can't see the function that you mention in ntp_snippets_service. 
Is it gone in the meantime?

Sorry, it's in SnippetsBridge: https://cs.chromium.org/chromium/src/chrome/browser/android/ntp/ntp_snippets_bridge.cc?dr=CSs&sq=package:chromium&l=108

Thanks!

I'd move it to SnippetsService then (and call the SnippetsService from the bridge).

Hmm, HistoryService provides only asynchronous querying API.
This seems like an indication that it is not always super fast.

From the requirements here it seems to me that the only way to implement it is:
 - when new snippets are loaded (such as on service initialization), send the queries to HistoryService,
 - wait for all the queries to finish,
 - delete all visited snippets (based on responses from queries),
 - do further work (such as delete snippets at the end of the list if we have too many of them),
 - only now inform all observers (such as the UI) that new snippets have been loaded.

On random refetch in the background this seems okay.
Is this accaptable during initialization of the service?

Well, HistoryService needs to check with a database, which might incur disk access, which is why it happens in the background. OTOH, loading snippets from _their_ database might also need to access the disk, so I don't think an additional thread jump + blocking read would be that bad. 

Most probably cannot make it before the BP.

I was dragged away by other tasks. Now I wait for the new ContentSuggestionService / new ArticleSuggestionProvider to stabilize so that I can do the work there.

Did not touch it this week. I would like to first move forward the more important stuff (bookmarks, scaling zine).

I have not started on this, yet. 
This would consume a lot of my time until BP.

I do not feel very motivated because I see quite some more important tasks until BP and because the conceptual way is to filter these on the server anyway.

What do you think?

it's a P2, that's ok. My understanding is that we already remove Snippets that are already present on the device, so no duplicates. Correct?

Removing Snippets that have been visited previously is a P2.

Yes, we do filter duplicates on the client.
Since the whole "removing already-visited suggestions" IMO requires more discussion, removing milestone.

As there were no discussions around this in last months, does anyone oppose to marking this as WontFix?