Rename Lo-Fi previews to lite pages

Rename Lo-Fi previews to lite pages

TBR=asvitkine@chromium.org
BUG=639439
Committed: https://crrev.com/778536d860ed092880bdd114e8adee54c7f0dac3
Cr-Commit-Position: refs/heads/master@{#422191}

[megjablon, 2016-09-13 22:17:50]

Patchset #1 (id:1) has been deleted

[megjablon, 2016-09-13 22:18:01]

megjablon@chromium.org changed reviewers:
+ bengr@chromium.org

[megjablon, 2016-09-13 22:18:02]

PTAL

[bengr, 2016-09-22 22:04:49]

lgtm

https://codereview.chromium.org/2296043003/diff/40001/components/data_reducti...
File
components/data_reduction_proxy/core/common/data_reduction_proxy_switches.cc
(right):

https://codereview.chromium.org/2296043003/diff/40001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_switches.cc:86:
"enable-data-reduction-proxy-lite-page";
Please fix all integration tests along with this change.

[tbansal1, 2016-09-23 00:08:41]

tbansal@chromium.org changed reviewers:
+ tbansal@chromium.org

[tbansal1, 2016-09-23 00:08:42]

drive-by comment.

https://codereview.chromium.org/2296043003/diff/40001/components/data_reducti...
File
components/data_reduction_proxy/core/common/data_reduction_proxy_switches.cc
(right):

https://codereview.chromium.org/2296043003/diff/40001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_switches.cc:86:
"enable-data-reduction-proxy-lite-page";
nit: you will also need to update the field trial json configs.

[megjablon, 2016-09-27 00:08:11]

Please take another look. Added integration test changes.

https://codereview.chromium.org/2296043003/diff/40001/components/data_reducti...
File
components/data_reduction_proxy/core/common/data_reduction_proxy_switches.cc
(right):

https://codereview.chromium.org/2296043003/diff/40001/components/data_reducti...
components/data_reduction_proxy/core/common/data_reduction_proxy_switches.cc:86:
"enable-data-reduction-proxy-lite-page";
On 2016/09/22 22:04:48, bengr wrote:
> Please fix all integration tests along with this change.

Done.

[megjablon, 2016-09-27 00:51:00]

The CQ bit was checked by megjablon@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-27 00:51:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-27 01:14:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-27 01:14:48]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[megjablon, 2016-09-27 18:43:48]

The CQ bit was checked by megjablon@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-27 18:44:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-27 19:26:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-27 19:26:36]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[megjablon, 2016-09-27 19:40:27]

The CQ bit was checked by megjablon@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-27 19:41:04]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-27 20:44:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-27 20:44:28]

Dry run: This issue passed the CQ dry run.

[megjablon, 2016-09-29 21:01:08]

megjablon@chromium.org changed reviewers:
+ asvitkine@google.com, dfalcantara@chromium.org

[megjablon, 2016-09-29 21:01:09]

dfalcantara: chrome/android/*
asvitkine: histograms.xml

[megjablon, 2016-09-29 21:07:57]

The CQ bit was checked by megjablon@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-29 21:08:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-29 22:07:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-29 22:07:14]

Dry run: This issue passed the CQ dry run.

[gone, 2016-09-30 17:33:01]

lgtm

[tbansal1, 2016-09-30 17:58:27]

https://chromiumcodereview.appspot.com/2296043003/diff/160001/chrome/app/gene...
File chrome/app/generated_resources.grd (right):

https://chromiumcodereview.appspot.com/2296043003/diff/160001/chrome/app/gene...
chrome/app/generated_resources.grd:6412: +         Enable lite pages in Data
Saver Lo-Fi mode. Previews of pages will be shown instead of image placeholders
when Lo-Fi is on. Data Saver and Lo-Fi must be enabled for lite pages to be
shown.
"Enable lite pages in Data Saver Lo-Fi mode"
This is confusing. Isn't litepage != Lo-Fi mode (from
https://bugs.chromium.org/p/chromium/issues/detail?id=639439).

https://chromiumcodereview.appspot.com/2296043003/diff/160001/components/data...
File
components/data_reduction_proxy/core/common/data_reduction_proxy_switches.cc
(right):

https://chromiumcodereview.appspot.com/2296043003/diff/160001/components/data...
components/data_reduction_proxy/core/common/data_reduction_proxy_switches.cc:82:
// Enables lite page previews from the data reduction proxy. This means a
Remove "previews"

https://chromiumcodereview.appspot.com/2296043003/diff/160001/tools/metrics/h...
File tools/metrics/histograms/histograms.xml (left):

https://chromiumcodereview.appspot.com/2296043003/diff/160001/tools/metrics/h...
tools/metrics/histograms/histograms.xml:8178: Counts of pageloads that received
or requested, but did not receive, various
For clarity, you might want to mention that this is recorded when the lite page
is requested or received through Data compression proxy.

https://chromiumcodereview.appspot.com/2296043003/diff/160001/tools/metrics/h...
File tools/metrics/histograms/histograms.xml (right):

https://chromiumcodereview.appspot.com/2296043003/diff/160001/tools/metrics/h...
tools/metrics/histograms/histograms.xml:8179: +    Lo-Fi lite page
transformations.
"Lo-Fi lite page" 
Did you mean "lite page"?

https://chromiumcodereview.appspot.com/2296043003/diff/160001/tools/metrics/h...
tools/metrics/histograms/histograms.xml:75600: +  <int value="0"
label="Pageloads that are Lo-Fi lite page previews"/>
s/"Lo-Fi lite page previews" /"lite pages"
same below.

[megjablon, 2016-09-30 18:31:03]

The CQ bit was checked by megjablon@chromium.org to run a CQ dry run

[megjablon, 2016-09-30 18:31:19]

https://chromiumcodereview.appspot.com/2296043003/diff/160001/chrome/app/gene...
File chrome/app/generated_resources.grd (right):

https://chromiumcodereview.appspot.com/2296043003/diff/160001/chrome/app/gene...
chrome/app/generated_resources.grd:6412: +         Enable lite pages in Data
Saver Lo-Fi mode. Previews of pages will be shown instead of image placeholders
when Lo-Fi is on. Data Saver and Lo-Fi must be enabled for lite pages to be
shown.
On 2016/09/30 17:58:27, tbansal1 wrote:
> "Enable lite pages in Data Saver Lo-Fi mode"
> This is confusing. Isn't litepage != Lo-Fi mode (from
> https://bugs.chromium.org/p/chromium/issues/detail?id=639439).

The implementation of Lite Pages is such that Previews is used in place of Lo-Fi
whenever Lo-Fi would be triggered. I don't mind changing this, but I should go
through and rename everything along the pipeline from the RenderFrameImpl
trigger and rework how these flags are triggered then. Can you file a bug
against me if you prefer this way and we can discuss?

https://chromiumcodereview.appspot.com/2296043003/diff/160001/components/data...
File
components/data_reduction_proxy/core/common/data_reduction_proxy_switches.cc
(right):

https://chromiumcodereview.appspot.com/2296043003/diff/160001/components/data...
components/data_reduction_proxy/core/common/data_reduction_proxy_switches.cc:82:
// Enables lite page previews from the data reduction proxy. This means a
On 2016/09/30 17:58:27, tbansal1 wrote:
> Remove "previews"

Done.

https://chromiumcodereview.appspot.com/2296043003/diff/160001/tools/metrics/h...
File tools/metrics/histograms/histograms.xml (left):

https://chromiumcodereview.appspot.com/2296043003/diff/160001/tools/metrics/h...
tools/metrics/histograms/histograms.xml:8178: Counts of pageloads that received
or requested, but did not receive, various
On 2016/09/30 17:58:27, tbansal1 wrote:
> For clarity, you might want to mention that this is recorded when the lite
page
> is requested or received through Data compression proxy.

Done.

https://chromiumcodereview.appspot.com/2296043003/diff/160001/tools/metrics/h...
File tools/metrics/histograms/histograms.xml (right):

https://chromiumcodereview.appspot.com/2296043003/diff/160001/tools/metrics/h...
tools/metrics/histograms/histograms.xml:8179: +    Lo-Fi lite page
transformations.
On 2016/09/30 17:58:27, tbansal1 wrote:
> "Lo-Fi lite page" 
> Did you mean "lite page"?

Keeping Lo-Fi Lite Page here since otherwise the name of the histogram is very
confusing (DataReductionProxy.LoFi.TransformationType). If we want to add a new
histogram with a better name, I can do that separately. I need to get this CL in
today.

https://chromiumcodereview.appspot.com/2296043003/diff/160001/tools/metrics/h...
tools/metrics/histograms/histograms.xml:75600: +  <int value="0"
label="Pageloads that are Lo-Fi lite page previews"/>
On 2016/09/30 17:58:27, tbansal1 wrote:
> s/"Lo-Fi lite page previews" /"lite pages"
> same below.

Done.

[commit-bot: I haz the power, 2016-09-30 18:32:13]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tbansal1, 2016-09-30 18:41:55]

lgtm

https://chromiumcodereview.appspot.com/2296043003/diff/160001/chrome/app/gene...
File chrome/app/generated_resources.grd (right):

https://chromiumcodereview.appspot.com/2296043003/diff/160001/chrome/app/gene...
chrome/app/generated_resources.grd:6412: +         Enable lite pages in Data
Saver Lo-Fi mode. Previews of pages will be shown instead of image placeholders
when Lo-Fi is on. Data Saver and Lo-Fi must be enabled for lite pages to be
shown.
On 2016/09/30 18:31:18, megjablon wrote:
> On 2016/09/30 17:58:27, tbansal1 wrote:
> > "Enable lite pages in Data Saver Lo-Fi mode"
> > This is confusing. Isn't litepage != Lo-Fi mode (from
> > https://bugs.chromium.org/p/chromium/issues/detail?id=639439).
> 
> The implementation of Lite Pages is such that Previews is used in place of
Lo-Fi
> whenever Lo-Fi would be triggered. I don't mind changing this, but I should go
> through and rename everything along the pipeline from the RenderFrameImpl
> trigger and rework how these flags are triggered then. Can you file a bug
> against me if you prefer this way and we can discuss?

I will leave it up to bengr@ to decide.

[megjablon, 2016-09-30 19:16:16]

Description was changed from

==========
Rename Lo-Fi previews to lite pages

BUG=639439
==========

to

==========
Rename Lo-Fi previews to lite pages

TBR=asvitkine@chromium.org
BUG=639439
==========

[commit-bot: I haz the power, 2016-09-30 19:30:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-30 19:30:15]

Dry run: This issue passed the CQ dry run.

[megjablon, 2016-09-30 19:40:41]

The CQ bit was checked by megjablon@chromium.org

[megjablon, 2016-09-30 19:40:42]

The patchset sent to the CQ was uploaded after l-g-t-m from bengr@chromium.org,
dfalcantara@chromium.org
Link to the patchset:
https://chromiumcodereview.appspot.com/2296043003/#ps180001 (title: "tbansal
comments")

[commit-bot: I haz the power, 2016-09-30 19:41:28]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-30 19:49:41]

Description was changed from

==========
Rename Lo-Fi previews to lite pages

TBR=asvitkine@chromium.org
BUG=639439
==========

to

==========
Rename Lo-Fi previews to lite pages

TBR=asvitkine@chromium.org
BUG=639439
==========

[commit-bot: I haz the power, 2016-09-30 19:49:42]

Committed patchset #9 (id:180001)

[commit-bot: I haz the power, 2016-09-30 19:53:09]

Description was changed from

==========
Rename Lo-Fi previews to lite pages

TBR=asvitkine@chromium.org
BUG=639439
==========

to

==========
Rename Lo-Fi previews to lite pages

TBR=asvitkine@chromium.org
BUG=639439

Committed: https://crrev.com/778536d860ed092880bdd114e8adee54c7f0dac3
Cr-Commit-Position: refs/heads/master@{#422191}
==========

[commit-bot: I haz the power, 2016-09-30 19:53:10]

Patchset 9 (id:??) landed as
https://crrev.com/778536d860ed092880bdd114e8adee54c7f0dac3
Cr-Commit-Position: refs/heads/master@{#422191}