New issue
Advanced search Search tips

Issue 666294 link

Starred by 2 users
Status: Available
Owner: ----
Cc:
ntp-dev+bugs@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Task



Sign in to add a comment

Require top_sites in MostVisitedSites

Project Member Reported by sfiera@chromium.org, Nov 17 2016 
Currently, MostVisitedSites allows a null TopSites* to be passed in. All of its logic around top sites is gated on top_sites_ conditionally being available. We would prefer to DCHECK(top_sites_) in the constructor.

It's currently not possible to make this change because iOS passes null in tests (per TopSitesFactory::ServiceIsNULLWhileTesting()). If we could change that to pass a dummy object, that would work. Or, if we could make MostVisitedSites a KeyedService (crbug/619584) then we could have MostVisitedSites also be null under tests easily.

Comment 1 by treib@chromium.org, Nov 17 2016

Labels: OS-All
Status: Assigned (was: Untriaged)
Some things to consider:
IIRC, TopSites is null for incognito profiles.
There's a command line param to make TopSites null: https://cs.chromium.org/chromium/src/chrome/browser/history/top_sites_factory.cc?rcl=1479284732&l=34 (used by some tests I think).

Comment 2 by fi...@chromium.org, Dec 22 2016

Labels: zine-triaged

Comment 3 by treib@chromium.org, Jan 11 2018

Labels: Type-Task

Comment 4 by fi...@chromium.org, Jan 12 2018

Components: -UI>Browser>NewTabPage UI>Browser>ContentSuggestions

Comment 5 by sfiera@chromium.org, May 7 2018

Owner: ----
Status: Available (was: Assigned)
(not working on client-side Chrome any more)
►

Sign in to add a comment