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

Issue 649876 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

Too many bookmark signals for Important Sites

Project Member Reported by dmu...@chromium.org, Sep 23 2016

Issue description

We need to limit bookmark signals when a user has a ton. My idea is:

If a user has less than N (5?) bookmarks, then just use those as signals.

Otherwise, only include the top M (10? or just 5 again?) bookmark origins that have site engagement, sorted.

WDYT Dru & Rebecca? I was thinking maybe these constants should both be 5. We basically don't want an endless amount of bookmark signals if we want blacklisting to make the dialog go away eventually.

See the UMA here for bookmark numbers:
https://uma.googleplex.com/p/chrome/histograms/?endDate=09-22-2016&dayCount=1&histograms=Sync.ModelTypeCount.BOOKMARK&fixupData=true&showMax=true&filters=channel%2Ceq%2C4%2Cisofficial%2Ceq%2CTrue&implicitFilters=isofficial

Assign back to me.
 

Comment 1 by dk...@chromium.org, Sep 26 2016

Owner: dmu...@chromium.org
5 seems reasonable to me (for both constants). There's a big drop off after that point and it aligns with the general limit of 5 we have so far. I think we can always raise the signal if numbers warrant it in the future.

N = M = 5 SGTM.

Comment 2 by dmu...@chromium.org, Sep 26 2016

Awesome! Thanks.

Comment 3 by rolfe@chromium.org, Sep 26 2016

what dknox says : )

Comment 4 by dmu...@chromium.org, Sep 26 2016

Status: Started (was: Unconfirmed)

Comment 5 by dmu...@chromium.org, Sep 26 2016

Labels: dmurph-shortlist-bugs
Project Member

Comment 6 by bugdroid1@chromium.org, Sep 26 2016

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

commit e11f01dc5c09c5e1eb4c748297a5a2c57e719b65
Author: dmurph <dmurph@chromium.org>
Date: Mon Sep 26 22:05:15 2016

[ImportantSites] Limiting the # of Bookmark signals

We don't want to include every bookmarked site as an important site as
there may be TONS. Here we try to limit them to 10 at most:
* If there are 5 or less, just use those as signals,
* Otherwise filter for bookmarks with site engagement > 0, sort &
  limit to 5.

R=tedchoc@chromium.org
BUG= 649876 

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

[modify] https://crrev.com/e11f01dc5c09c5e1eb4c748297a5a2c57e719b65/chrome/browser/android/preferences/important_sites_util.cc
[modify] https://crrev.com/e11f01dc5c09c5e1eb4c748297a5a2c57e719b65/chrome/browser/android/preferences/important_sites_util_unittest.cc

Comment 7 by dmu...@chromium.org, Sep 26 2016

Status: Fixed (was: Started)

Sign in to add a comment