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

Issue 790733 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task



Sign in to add a comment

Refactor DataReductionProxyConfig's ShouldEnable previews methods

Project Member Reported by thanhdle@chromium.org, Nov 30 2017

Issue description

ShouldEnableLitePages and ShouldEnableLoFi methods are identical, and both are checked at the same time, which could be run more times that they should.                                                               

Removed one, and change the method name to ShouldEnableServerPreviews.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 1 2017

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

commit adcc475ecd652afb0a923a06d5fd9e107b6c0eff
Author: Thanh Le <thanhdle@chromium.org>
Date: Fri Dec 01 17:10:00 2017

Refactor DataReductionProxyConfig's ShouldEnable previews methods.

ShouldEnableLitePages and ShouldEnableLoFi methods are identical,
and both are checked at the same time, which could be run more times
that they should.

Removed one, and change the method name to ShouldEnableServerPreviews.

Bug:  790733 
Change-Id: I9c787b4a0b8919fc250d260638b5b62b067e375c
Reviewed-on: https://chromium-review.googlesource.com/801994
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Commit-Queue: Thanh Le <thanhdle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520967}
[modify] https://crrev.com/adcc475ecd652afb0a923a06d5fd9e107b6c0eff/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/adcc475ecd652afb0a923a06d5fd9e107b6c0eff/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
[modify] https://crrev.com/adcc475ecd652afb0a923a06d5fd9e107b6c0eff/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
[modify] https://crrev.com/adcc475ecd652afb0a923a06d5fd9e107b6c0eff/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/adcc475ecd652afb0a923a06d5fd9e107b6c0eff/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc
[modify] https://crrev.com/adcc475ecd652afb0a923a06d5fd9e107b6c0eff/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h
[modify] https://crrev.com/adcc475ecd652afb0a923a06d5fd9e107b6c0eff/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc

Status: Fixed (was: Started)
Possible follow-on here would be to merge DataReductionProxyConfig::ShouldEnableServerPreviews() into DataReductionProxyConfig::ShouldAcceptServerPreview(). 
Only need single method here now I think.
Status: Started (was: Fixed)
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 2 2017

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

commit 7f80bcc58221d050abb57c1c8c26f4c10cea2514
Author: Thanh Le <thanhdle@chromium.org>
Date: Sat Dec 02 00:43:37 2017

Refactor DataReductionProxyConfig ShouldEnableServerPreviews().

Merge DataReductionProxyConfig::ShouldEnableServerPreviews() into
DataReductionProxyConfig::ShouldAcceptServerPreview().

Bug:  790733 
Change-Id: Iea803f50c5f0346e4ade31e80fcb98f18d3c99d7
Reviewed-on: https://chromium-review.googlesource.com/803670
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Commit-Queue: Thanh Le <thanhdle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521159}
[modify] https://crrev.com/7f80bcc58221d050abb57c1c8c26f4c10cea2514/chrome/browser/loader/chrome_resource_dispatcher_host_delegate.cc
[modify] https://crrev.com/7f80bcc58221d050abb57c1c8c26f4c10cea2514/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
[modify] https://crrev.com/7f80bcc58221d050abb57c1c8c26f4c10cea2514/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
[modify] https://crrev.com/7f80bcc58221d050abb57c1c8c26f4c10cea2514/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/7f80bcc58221d050abb57c1c8c26f4c10cea2514/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.cc
[modify] https://crrev.com/7f80bcc58221d050abb57c1c8c26f4c10cea2514/components/data_reduction_proxy/core/browser/data_reduction_proxy_io_data.h
[modify] https://crrev.com/7f80bcc58221d050abb57c1c8c26f4c10cea2514/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc

Status: Fixed (was: Started)

Sign in to add a comment