New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 709959 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android , iOS
Pri: 3
Type: Bug



Sign in to add a comment

Extend ntp_tiles with support for less-than-8 Chrome Suggestions

Project Member Reported by mastiz@chromium.org, Apr 10 2017

Issue description

In the future with Chrome Home, less than 8 Most Visited tiles will be desired. Currently, the server-side Chrome Suggestions returns exactly 0 or 8.

(Btw, this is the main reason why NTP tile impressions correspond to TopSites, aka client-side suggestions, as opposed to server-side.)

Alternatives include changing the logic in MostVisitedSites to locally require a minimum number of server-side suggestions (variations-controlled?).

Perhaps, instead of using a fixed threshold, the number of tiles provided by TopSites could be used (capped), i.e. choose TopSites only if it provides more tiles than the server.

As part of this, we'll also need to decide whether the client sends some new query parameters to the server (so we can change the server-side behavior without breaking compatibility with older clients).

 

Comment 1 by sfiera@chromium.org, Apr 10 2017

I'm not sure I follow. I don't think ntp_tiles imposes any requirement about the number of server tiles; if the suggestions service returns 4, it'll use 4 and backfill with popular sites. (yes, even if we have 8 client tiles, but I don't know how much of a problem that would be in practice)

Comment 2 by edchin@chromium.org, Apr 10 2017

Cc: -sfiera@chromium.org gambard@chromium.org justincohen@chromium.org
Owner: sfiera@chromium.org
Status: Assigned (was: Untriaged)

Comment 3 by mastiz@chromium.org, Apr 11 2017

Here's another way to describe the goal: we'd like to change the server-side logic so it'll return an arbitrary number of tiles, *without* being perceivable by the user (or at least, we want to control this via finch).

Rationale: in the current setup, the server is partially responsible for implementing this logic to choose between top-sites and chrome suggestions. It does this by returning 0 when less than 8 tiles are available. This won't work well in the future.
Cc: noyau@chromium.org
Owner: fhorschig@chromium.org
Project Member

Comment 6 by bugdroid1@chromium.org, May 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/122c6655af011e18e58f4384e327174dd6212874

commit 122c6655af011e18e58f4384e327174dd6212874
Author: fhorschig <fhorschig@chromium.org>
Date: Fri May 05 10:21:30 2017

Add flag for disabling minimum of ntp tile suggestions

This flag will cause the the client to use history-based suggestions for
NTP tiles even if there are less than 8 suggestions available.

This is particularily useful for Chrome Home and the condensed NTP where
only 4 tiles are used and which does not contain any suggestions until
there would be enough data for 8 suggestions.

BUG= 709959 

Review-Url: https://codereview.chromium.org/2858883002
Cr-Commit-Position: refs/heads/master@{#469620}

[modify] https://crrev.com/122c6655af011e18e58f4384e327174dd6212874/chrome/browser/about_flags.cc
[modify] https://crrev.com/122c6655af011e18e58f4384e327174dd6212874/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/122c6655af011e18e58f4384e327174dd6212874/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/122c6655af011e18e58f4384e327174dd6212874/components/suggestions/BUILD.gn
[add] https://crrev.com/122c6655af011e18e58f4384e327174dd6212874/components/suggestions/features.cc
[add] https://crrev.com/122c6655af011e18e58f4384e327174dd6212874/components/suggestions/features.h
[modify] https://crrev.com/122c6655af011e18e58f4384e327174dd6212874/tools/metrics/histograms/enums.xml

Status: Started (was: Assigned)
Link to design doc: go/ntp-less-than-eight-suggestions
Server-side bug: b/38020931

Project Member

Comment 8 by bugdroid1@chromium.org, May 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/590d7f9b223b8fde7722c7f0c395f8cd2e7e3bc9

commit 590d7f9b223b8fde7722c7f0c395f8cd2e7e3bc9
Author: fhorschig <fhorschig@chromium.org>
Date: Wed May 10 14:17:14 2017

Test SuggestionsServiceImpl without friending anything

This CL changes only the tests (and test-only getters) for the
SuggestionsServiceImpl class.
Changed:
- Responding to Fetches with TestUrlFetchers -> no alternative URL hacks
- Blackboxed -> neither tests nor test class are friends of the Impl

Intentionally not changed (although considerable):
(- anything in the .cc file)
- visibility of privately inherited OnStateChanged
- constructor (to inject a clock)
- EXPECT_EQ (to go for consistency)

BUG= 709959 

Review-Url: https://codereview.chromium.org/2869013004
Cr-Commit-Position: refs/heads/master@{#470560}

[modify] https://crrev.com/590d7f9b223b8fde7722c7f0c395f8cd2e7e3bc9/components/suggestions/suggestions_service_impl.h
[modify] https://crrev.com/590d7f9b223b8fde7722c7f0c395f8cd2e7e3bc9/components/suggestions/suggestions_service_impl_unittest.cc

Project Member

Comment 9 by bugdroid1@chromium.org, May 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ec72b3b0616de6e11b11db904b12a5a7e2a1a9da

commit ec72b3b0616de6e11b11db904b12a5a7e2a1a9da
Author: fhorschig <fhorschig@chromium.org>
Date: Thu May 11 08:20:52 2017

Use |min| param to query a variable number of suggestions

In order to use the newly implemented server-side functionality of
receiving less than 8 suggestions, the client has to append a new
query parameter to the URL.

Therefore, this CL is blocked on cr/155185623

BUG= 709959 

Review-Url: https://codereview.chromium.org/2872573002
Cr-Commit-Position: refs/heads/master@{#470874}

[modify] https://crrev.com/ec72b3b0616de6e11b11db904b12a5a7e2a1a9da/components/suggestions/suggestions_service_impl.cc
[modify] https://crrev.com/ec72b3b0616de6e11b11db904b12a5a7e2a1a9da/components/suggestions/suggestions_service_impl_unittest.cc

Status: Fixed (was: Started)
cr/155185623 was submitted. Experimentation could now start. 
Launch tracking bug: issue 731687

Sign in to add a comment