New issue
Advanced search Search tips

Issue 680620 link

Starred by 1 user
Status: Fixed
Owner:
dougarnett@chromium.org
Closed: Feb 2017
Cc:
ryansturm@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Provide mechanism to clear client-side black list data for a previews treatment

Project Member Reported by ryansturm@chromium.org, Jan 12 2017 
Scenario:

Preview X is terrible and everyone ends up blacklisting out of previews altogether.

We turn off Preview X until a fix is made, but want to restore the blacklist to a state similar to before Preview X for other previews.


Solution:

Add a field trial controlled time based mechanism that can clear the blacklist database of a preview type before time X (or multiple types of previews before their respective times). This should also have a solution for users whose clock is incorrect (i.e., if there clock is before the "preview reset epoch", do nothing).

Alternatively, if we decide the solution is to turn off a "bad preview" then this can just remove preview type X from the database altogether. With this approach, if we wish to reintroduce the preview, it can be assigned a new value in the enum and we can never use the old value anymore.

Comment 1 by bengr@chromium.org, Jan 12 2017

Cc: ryansturm@chromium.org
Owner: dougarnett@chromium.org
Status: Assigned (was: Untriaged)
Summary: Provide mechanism to clear client-side black list data for a previews treatment (was: Add a mechanism to remove an experiment from the previews black list)
I think a better way to do this is to introduce the notion of a version to each previews treatment, and to bump the version whenever it has changed enough to warrant clearing all prior blacklist date for it.

In the black list DB, introduce a new table that provides the version of each previews treatment. Provide the current version via Finch. If the version in the table is lower than the version in Finch, clear the black list data for the corresponding treatment and set the new version in the table.

Comment 2 by dougarnett@chromium.org, Jan 13 2017 

Some mappings to code:
  - "previews treatment" => previews::PreviewsType
  - blacklist db => PreviewsOptOutStoreSQL
  - field trial support => IsPreviewsTypeEnable() [previews_experiments.h]
Project Member

Comment 3 by bugdroid1@chromium.org, Feb 15 2017 

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

commit 5e57dedf90886a800035abc8e70048266d1dd110
Author: dougarnett <dougarnett@chromium.org>
Date: Wed Feb 15 19:05:47 2017

Adds PreviewsType version mechanism for clearing blacklist entries.

Adds new table to the Previews opt-out store (PreviewsOptOutStoreSQL)
for holding the current version of PreviewsType's used on that client
with clean-up code to delete black list entries for a PreviewsType
when the version of that PreviewsType changes.

BUG= 680620 

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

[modify] https://crrev.com/5e57dedf90886a800035abc8e70048266d1dd110/components/previews/core/previews_experiments.cc
[modify] https://crrev.com/5e57dedf90886a800035abc8e70048266d1dd110/components/previews/core/previews_experiments.h
[modify] https://crrev.com/5e57dedf90886a800035abc8e70048266d1dd110/components/previews/core/previews_opt_out_store_sql.cc
[modify] https://crrev.com/5e57dedf90886a800035abc8e70048266d1dd110/components/previews/core/previews_opt_out_store_sql_unittest.cc

Comment 4 by dougarnett@chromium.org, Feb 27 2017

Labels: -M-57 M-58
Status: Fixed (was: Assigned)
Forgot to mark fixed after CL submitted. Also, we discussed in team meeting that this was not needed in 57.
►

Sign in to add a comment