Assigning to ryansturm@. Ryan, what's the desired behavior here?

It also looks like lofi_off() is used when deciding whether or not to add the CPAT header: https://cs.chromium.org/chromium/src/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc?type=cs&q=lofi_off()&l=339

And again here: https://cs.chromium.org/chromium/src/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc?type=cs&q=lofi_off()&l=1083

Dougarnett@ is this on your radar as part of the transition?

Seemed to me we should get rid of lofi_off once transitioned to the new blacklist. It does seem like a bit of an issue to check lofi_off if the
new blacklist is in use (if it can be set per previously persisted 3x3 state).
I don't have full background on the motivation for lofi_off though - as I
recall some path that seem to just set it for the session.

My proposal (please object as appropriate):
  1. Only check lofi_off if not using the new blacklist (for either type of LoFi)
  2. Kill lofi_off once fully cutover to new blacklist.
  

Re #4: I agree, doing 1 and 2 in order sounds good. That way, users who have previously gotten themselves into a forever opted-out state have a chance to try LoFi and other previews again.

Aposner: any guidance here around respecting old prefs vs ignoring them?

Aposner: any guidance here around respecting old prefs vs ignoring them?

Didn't mean to remove sclittle, adding back.

Looks like this change caused a change in behavior for this area:

https://chromium.googlesource.com/chromium/src/+/cbda5f6b2b2d76236388e663c3439ef91c59ec0a%5E%21/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc

Since it is M60, we would need a fix for M61 to either make client side previews ignore the old prefs or make server side previews respect the old prefs (3x3 rule). This is only happening with the transition blacklist experiment enabled, so we could push that to M62 if we don't want to merge to M61.

Chatted with Ryan offline - I'm inclined to ignore the old prefs, so users who were previously opted out of WebLite Previews will be reintroduced to new Previews. 

I do see pros/cons both to respecting and ignoring the old prefs. If we respect them, users who opted out of WL don't get to see new Previews that are higher fidelity. However, if we ignore them there's certainly some likelihood that if a user doesn't like one type of Preview, they won't like any modifications to the page. 

Certainly open to hearing other opinions here, but I'd propose we move forward ignoring the old prefs and merging a fix to M61 since it'd be good to avoid reintroducing previously opted out users to the same Previews without introducing new types, assuming I'm correctly interpreting the current situation.

Ignoring the old prefs and reintroducing sgtm - especially since it was an inferred opt-out in the first place (vs. more direct user opt-out of feature).

ignoring the old prefs sgtm.

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

commit c7673aa794736e35e5cd1cff653117ea8409c007
Author: Ryan Sturm <ryansturm@chromium.org>
Date: Wed Jul 26 21:02:54 2017

Removing the pref check for client-lofi when in the blacklist experiment

This change removes the check for lofi_off that reflects historical opt
out preferences for server previews, but is no longer checked for server
previews.

Bug:  745250 
Change-Id: I22fe6f6887aace8b37a7c9b5dfa5cb2cbee6a923
Reviewed-on: https://chromium-review.googlesource.com/585362
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Ryan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#489753}
[modify] https://crrev.com/c7673aa794736e35e5cd1cff653117ea8409c007/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/c7673aa794736e35e5cd1cff653117ea8409c007/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
[modify] https://crrev.com/c7673aa794736e35e5cd1cff653117ea8409c007/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/c7673aa794736e35e5cd1cff653117ea8409c007/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc

sclittle: does this need to be merged to M61 for client side lo-fi considering the transition is launching in 61?

I think this should probably be merged into 61 if the transition is launching in 61.

Makes sense to me. Would you be able to merge it sclittle, I'm wfh for a while. I believe both field trial are planned to launch in 61.

OK, requesting merge into 61.

Your change meets the bar and is auto-approved for M61. Please go ahead and merge the CL to branch 3163 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid @(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Would be good to get this merged looking at this Dev UMA: https://uma.googleplex.com/p/chrome/histograms?endDate=20170803&dayCount=7&histograms=DataReductionProxy.Protocol.AcceptTransform%2CDataReductionProxy.Protocol.NotAcceptingTransform%2CPreviews.OptOut.UserOptedOut.LoFi&fixupData=true&showMax=true&filters=platform%2Ceq%2CA%2Cchannel%2Ceq%2C2%2Call_version%2Ccnt%2C61.0.3163%2Cisofficial%2Ceq%2CTrue&implicitFilters=isofficial

See about 65k server directed preview pages (20k lite, 45k lofi) and PreviewsUserOptedOut histogram has 66k entries with just over 5% having opted-out: 3,466 yet looking at NotAcceptingTransform histogram, there were 149k page load that were blacklisted.

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

commit 189a60a3757d3802ec44fa1d0d5fb2bb806bdb0c
Author: Scott Little <sclittle@chromium.org>
Date: Fri Aug 04 22:34:28 2017

Removing the pref check for client-lofi when in the blacklist experiment

This change removes the check for lofi_off that reflects historical opt
out preferences for server previews, but is no longer checked for server
previews.

TBR=ryansturm@chromium.org

(cherry picked from commit c7673aa794736e35e5cd1cff653117ea8409c007)

Bug:  745250 
Change-Id: I22fe6f6887aace8b37a7c9b5dfa5cb2cbee6a923
Reviewed-on: https://chromium-review.googlesource.com/585362
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Ryan Sturm <ryansturm@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#489753}
Reviewed-on: https://chromium-review.googlesource.com/602878
Reviewed-by: Scott Little <sclittle@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#330}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/189a60a3757d3802ec44fa1d0d5fb2bb806bdb0c/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/189a60a3757d3802ec44fa1d0d5fb2bb806bdb0c/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
[modify] https://crrev.com/189a60a3757d3802ec44fa1d0d5fb2bb806bdb0c/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/189a60a3757d3802ec44fa1d0d5fb2bb806bdb0c/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc