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

Issue 760294 link

Starred by 9 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 22
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug

Blocked on:
issue 779219
issue 799376
issue 805072
issue 806114
issue 813184
issue 813186
issue 830088
issue 839628
issue 849292
issue 849387

Blocking:
issue 724704
issue 841842



Sign in to add a comment

Chrome should verify connection to data reduction proxy

Project Member Reported by bengr@chromium.org, Aug 29 2017

Issue description

Chrome should fall back to a direct connection when it can't connect to the data reduction proxy, but doesn't fall back if it succeeds in connecting but cannot use the proxy.

For example, a misconfigured dns mapping could point Chrome to the wrong IP address, a connection could be established, but the server at that address might not operate as a data reduction proxy. See https://bugs.chromium.org/p/chromium/issues/detail?id=756645.

Chrome should verify that the connection is actually to the data reduction proxy and that the connection can served resources. If not, Chrome should fall back to a direct connection immediately.

Note that the verification mechanism should be robust to failures of server that serves a verification URL.
 

Comment 1 by bengr@chromium.org, Aug 29 2017

Components: Internals>Network>DataProxy
What do you mean by "robust to failures of server that serves a verification URL"?
Cc: msrchandra@chromium.org sandeepkumars@chromium.org nyerramilli@chromium.org
 Issue 756645  has been merged into this issue.
Cc: -tbansal@chromium.org
Owner: tbansal@chromium.org
Status: Assigned (was: Available)
I can take this since I am working on the warmup connection feature where Chrome fetches a known resource from Google server at startup (and on connection change events).
Issue 750117 has been merged into this issue.
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 3 2017

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

commit a2bf928ee91f4eadc5b760f09bbc86f592213d76
Author: Tarun Bansal <tbansal@chromium.org>
Date: Tue Oct 03 20:35:00 2017

Refactor Data Reduction Proxy URL fetcher delegates out 

Refactor the secure proxy checker and the warmup URL fetcher out
of the Data Reduction Proxy (DRP) config files.

Bug:  760294 
Change-Id: Idfb939bb5ba370cde31e25eca2bf9deeccffacc0
Reviewed-on: https://chromium-review.googlesource.com/693761
Reviewed-by: Scott Little <sclittle@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506159}
[modify] https://crrev.com/a2bf928ee91f4eadc5b760f09bbc86f592213d76/components/data_reduction_proxy/core/browser/BUILD.gn
[modify] https://crrev.com/a2bf928ee91f4eadc5b760f09bbc86f592213d76/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
[modify] https://crrev.com/a2bf928ee91f4eadc5b760f09bbc86f592213d76/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
[modify] https://crrev.com/a2bf928ee91f4eadc5b760f09bbc86f592213d76/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h
[modify] https://crrev.com/a2bf928ee91f4eadc5b760f09bbc86f592213d76/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/a2bf928ee91f4eadc5b760f09bbc86f592213d76/components/data_reduction_proxy/core/browser/data_reduction_proxy_request_options.cc
[add] https://crrev.com/a2bf928ee91f4eadc5b760f09bbc86f592213d76/components/data_reduction_proxy/core/browser/secure_proxy_checker.cc
[add] https://crrev.com/a2bf928ee91f4eadc5b760f09bbc86f592213d76/components/data_reduction_proxy/core/browser/secure_proxy_checker.h
[add] https://crrev.com/a2bf928ee91f4eadc5b760f09bbc86f592213d76/components/data_reduction_proxy/core/browser/warmup_url_fetcher.cc
[add] https://crrev.com/a2bf928ee91f4eadc5b760f09bbc86f592213d76/components/data_reduction_proxy/core/browser/warmup_url_fetcher.h

Labels: -Pri-3 M-64 Pri-1
Project Member

Comment 8 by bugdroid1@chromium.org, Oct 6 2017

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

commit f49c026e17f761543491e57d8925fcf7813133fd
Author: Tarun Bansal <tbansal@chromium.org>
Date: Fri Oct 06 17:32:24 2017

Expose proxy server used by the URL fetcher

Expose the proxy server that was used by the URL fetcher

This will be used in a later CL by data reduction proxy to verify
that a certain resource was fetched over a data reduction
proxy or not.

Bug:  760294 
Change-Id: I6c060517f6f1dd2102b8448856df9dbbe90697c5
Reviewed-on: https://chromium-review.googlesource.com/703794
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507113}
[modify] https://crrev.com/f49c026e17f761543491e57d8925fcf7813133fd/net/url_request/test_url_fetcher_factory.cc
[modify] https://crrev.com/f49c026e17f761543491e57d8925fcf7813133fd/net/url_request/test_url_fetcher_factory.h
[modify] https://crrev.com/f49c026e17f761543491e57d8925fcf7813133fd/net/url_request/url_fetcher.h
[modify] https://crrev.com/f49c026e17f761543491e57d8925fcf7813133fd/net/url_request/url_fetcher_core.cc
[modify] https://crrev.com/f49c026e17f761543491e57d8925fcf7813133fd/net/url_request/url_fetcher_core.h
[modify] https://crrev.com/f49c026e17f761543491e57d8925fcf7813133fd/net/url_request/url_fetcher_impl.cc
[modify] https://crrev.com/f49c026e17f761543491e57d8925fcf7813133fd/net/url_request/url_fetcher_impl.h
[modify] https://crrev.com/f49c026e17f761543491e57d8925fcf7813133fd/net/url_request/url_fetcher_impl_unittest.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Oct 12 2017

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

commit 89cb8de76b47a274111b8b6b013322f1c8779a2c
Author: Tarun Bansal <tbansal@chromium.org>
Date: Thu Oct 12 20:14:35 2017

Add histograms to record result of warmup URL fetching in
data reduction proxy (DRP).


Bug:  760294 
Change-Id: Ibd0085ed90aa800d7d1def8ea2f06c95dc632448
Reviewed-on: https://chromium-review.googlesource.com/710737
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#508430}
[modify] https://crrev.com/89cb8de76b47a274111b8b6b013322f1c8779a2c/components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_stats.cc
[modify] https://crrev.com/89cb8de76b47a274111b8b6b013322f1c8779a2c/components/data_reduction_proxy/core/browser/warmup_url_fetcher.cc
[modify] https://crrev.com/89cb8de76b47a274111b8b6b013322f1c8779a2c/components/data_reduction_proxy/core/browser/warmup_url_fetcher_unittest.cc
[modify] https://crrev.com/89cb8de76b47a274111b8b6b013322f1c8779a2c/components/data_reduction_proxy/core/common/data_reduction_proxy_util.cc
[modify] https://crrev.com/89cb8de76b47a274111b8b6b013322f1c8779a2c/components/data_reduction_proxy/core/common/data_reduction_proxy_util.h
[modify] https://crrev.com/89cb8de76b47a274111b8b6b013322f1c8779a2c/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/89cb8de76b47a274111b8b6b013322f1c8779a2c/tools/metrics/histograms/histograms.xml

Project Member

Comment 11 by bugdroid1@chromium.org, Oct 25 2017

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

commit 35ba756375d8cc60102cd9b76815d64070fa809e
Author: Tarun Bansal <tbansal@chromium.org>
Date: Wed Oct 25 21:15:05 2017

Add mechanism to prevent usage of insecure data saver proxies

Subsequent CLs will try to fetch warmup URL over HTTP data
saver proxies. If the fetch fails, this functionality will be
used to prevent usage of HTTP data saver proxies (while still
allowing HTTPS proxies to be used).

Bug:  760294 
Change-Id: I21173ce4e9fd142a42af83f1185dcbe18e4d1301
Reviewed-on: https://chromium-review.googlesource.com/730879
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511592}
[modify] https://crrev.com/35ba756375d8cc60102cd9b76815d64070fa809e/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
[modify] https://crrev.com/35ba756375d8cc60102cd9b76815d64070fa809e/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
[modify] https://crrev.com/35ba756375d8cc60102cd9b76815d64070fa809e/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc
[modify] https://crrev.com/35ba756375d8cc60102cd9b76815d64070fa809e/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h
[modify] https://crrev.com/35ba756375d8cc60102cd9b76815d64070fa809e/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/35ba756375d8cc60102cd9b76815d64070fa809e/components/data_reduction_proxy/core/browser/data_reduction_proxy_configurator.cc
[modify] https://crrev.com/35ba756375d8cc60102cd9b76815d64070fa809e/components/data_reduction_proxy/core/browser/data_reduction_proxy_configurator.h
[modify] https://crrev.com/35ba756375d8cc60102cd9b76815d64070fa809e/components/data_reduction_proxy/core/browser/data_reduction_proxy_configurator_unittest.cc
[modify] https://crrev.com/35ba756375d8cc60102cd9b76815d64070fa809e/components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate.cc
[modify] https://crrev.com/35ba756375d8cc60102cd9b76815d64070fa809e/components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate_unittest.cc
[modify] https://crrev.com/35ba756375d8cc60102cd9b76815d64070fa809e/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
[modify] https://crrev.com/35ba756375d8cc60102cd9b76815d64070fa809e/components/data_reduction_proxy/core/browser/data_reduction_proxy_settings_unittest.cc
[modify] https://crrev.com/35ba756375d8cc60102cd9b76815d64070fa809e/components/data_reduction_proxy/core/common/data_reduction_proxy_event_store_unittest.cc

Blockedon: 779219
Issue 426074 has been merged into this issue.
Cc: robertogden@chromium.org
Project Member

Comment 15 by bugdroid1@chromium.org, Nov 13 2017

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

commit c5dcb8488883447904af5b68e0aa9c1d68d56508
Author: Tarun Bansal <tbansal@chromium.org>
Date: Mon Nov 13 22:55:51 2017

Miscellaneous cleanups to the data reduction proxy

* Remove some of the leftover tamper detection code
* Add an explicit callback when a new client config is fetched
* Limit the use of mock config for only the tests where it is actually required
* Remove the deprecated mock call to the network quality estimator

Bug:  760294 ,  675761 
Change-Id: Iaf8077e83bacca574aa6dea0ed9906f0d25d0176
Reviewed-on: https://chromium-review.googlesource.com/766072
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#516088}
[modify] https://crrev.com/c5dcb8488883447904af5b68e0aa9c1d68d56508/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
[modify] https://crrev.com/c5dcb8488883447904af5b68e0aa9c1d68d56508/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
[modify] https://crrev.com/c5dcb8488883447904af5b68e0aa9c1d68d56508/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client.cc
[modify] https://crrev.com/c5dcb8488883447904af5b68e0aa9c1d68d56508/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h
[modify] https://crrev.com/c5dcb8488883447904af5b68e0aa9c1d68d56508/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/c5dcb8488883447904af5b68e0aa9c1d68d56508/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
[modify] https://crrev.com/c5dcb8488883447904af5b68e0aa9c1d68d56508/components/data_reduction_proxy/core/common/data_reduction_proxy_headers.cc
[modify] https://crrev.com/c5dcb8488883447904af5b68e0aa9c1d68d56508/components/data_reduction_proxy/core/common/data_reduction_proxy_headers.h
[modify] https://crrev.com/c5dcb8488883447904af5b68e0aa9c1d68d56508/components/data_reduction_proxy/core/common/data_reduction_proxy_headers_unittest.cc

Labels: -Pri-1 Pri-2
Status: Started (was: Assigned)
What's the status of this?
Cc: pnangunoori@chromium.org tbansal@chromium.org
 Issue 773645  has been merged into this issue.
Re #16: Still working on it. 

Things that are left:
1. Disable data saver proxy if probe fails.
2. Add timeout to probes. Disable data saver proxy if probe timeouts.
3. Retry probe if it fails with a longer timeout.
4. Retry probe with different data saver proxies.
bengr design doc: http://shortn/_5L3zN6IVLk (sorry, internal only).
Project Member

Comment 20 by bugdroid1@chromium.org, Dec 6 2017

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

commit 34a83025d52873f9ab05bb86c22479627a90bfcb
Author: Tarun Bansal <tbansal@chromium.org>
Date: Wed Dec 06 02:12:36 2017

Disable data saver proxy if the warmup URL fetch fails

If the fetch of the warmup URL (aka probe URL) fails, the
corresponding proxy is disabled, and the config is reloaded.

A future CL will add retries -- If the fetch of the probe URL
due to network flakiness fails, then the probe will be retried
(up to 3 times) on the same proxy.

Bug:  760294 
Change-Id: Ib255f8178ef106f5e2882ad3dc32bde143c40a90
Reviewed-on: https://chromium-review.googlesource.com/807592
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521951}
[modify] https://crrev.com/34a83025d52873f9ab05bb86c22479627a90bfcb/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
[modify] https://crrev.com/34a83025d52873f9ab05bb86c22479627a90bfcb/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
[modify] https://crrev.com/34a83025d52873f9ab05bb86c22479627a90bfcb/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc
[modify] https://crrev.com/34a83025d52873f9ab05bb86c22479627a90bfcb/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h
[modify] https://crrev.com/34a83025d52873f9ab05bb86c22479627a90bfcb/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/34a83025d52873f9ab05bb86c22479627a90bfcb/components/data_reduction_proxy/core/browser/warmup_url_fetcher.cc
[modify] https://crrev.com/34a83025d52873f9ab05bb86c22479627a90bfcb/components/data_reduction_proxy/core/browser/warmup_url_fetcher.h
[modify] https://crrev.com/34a83025d52873f9ab05bb86c22479627a90bfcb/components/data_reduction_proxy/core/browser/warmup_url_fetcher_unittest.cc
[modify] https://crrev.com/34a83025d52873f9ab05bb86c22479627a90bfcb/components/data_reduction_proxy/core/common/data_reduction_proxy_server.cc
[modify] https://crrev.com/34a83025d52873f9ab05bb86c22479627a90bfcb/components/data_reduction_proxy/core/common/data_reduction_proxy_server.h
[modify] https://crrev.com/34a83025d52873f9ab05bb86c22479627a90bfcb/tools/metrics/histograms/histograms.xml

Project Member

Comment 21 by bugdroid1@chromium.org, Dec 7 2017

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

commit a023ae0ce55b5cd54317af4d595cd15e35ffef66
Author: Tarun Bansal <tbansal@chromium.org>
Date: Thu Dec 07 19:50:51 2017

Minor cleanups to the data reduction proxy component

1. Remove a method that is unused from DataReductionProxyConfig class
2. Remove unnecessary virtual designation on some of the methods
3. Expand on the method comment in data_reduction_proxy_config_values.h

Bug:  760294 
Change-Id: I1e0df907628f9c34f2e967c40726d6f9bba93d12
Reviewed-on: https://chromium-review.googlesource.com/814774
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522513}
[modify] https://crrev.com/a023ae0ce55b5cd54317af4d595cd15e35ffef66/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
[modify] https://crrev.com/a023ae0ce55b5cd54317af4d595cd15e35ffef66/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
[modify] https://crrev.com/a023ae0ce55b5cd54317af4d595cd15e35ffef66/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h
[modify] https://crrev.com/a023ae0ce55b5cd54317af4d595cd15e35ffef66/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/a023ae0ce55b5cd54317af4d595cd15e35ffef66/components/data_reduction_proxy/core/browser/data_reduction_proxy_configurator.h
[modify] https://crrev.com/a023ae0ce55b5cd54317af4d595cd15e35ffef66/components/data_reduction_proxy/core/common/data_reduction_proxy_config_values.h

Project Member

Comment 22 by bugdroid1@chromium.org, Dec 8 2017

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

commit 36222509aa6e819815938cbf2709b4849735537c
Author: Tarun Bansal <tbansal@chromium.org>
Date: Fri Dec 08 23:15:23 2017

Revert "Disable data saver proxy if the warmup URL fetch fails"

This reverts commit 34a83025d52873f9ab05bb86c22479627a90bfcb.

Reason for revert: Speculative revert for crbug.com/792841

Original change's description:
> Disable data saver proxy if the warmup URL fetch fails
> 
> If the fetch of the warmup URL (aka probe URL) fails, the
> corresponding proxy is disabled, and the config is reloaded.
> 
> A future CL will add retries -- If the fetch of the probe URL
> due to network flakiness fails, then the probe will be retried
> (up to 3 times) on the same proxy.
> 
> Bug:  760294 
> Change-Id: Ib255f8178ef106f5e2882ad3dc32bde143c40a90
> Reviewed-on: https://chromium-review.googlesource.com/807592
> Reviewed-by: Steven Holte <holte@chromium.org>
> Reviewed-by: Doug Arnett <dougarnett@chromium.org>
> Commit-Queue: Tarun Bansal <tbansal@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#521951}

TBR=holte@chromium.org,tbansal@chromium.org,dougarnett@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  760294 , 792841
Change-Id: I804763df69665a5e3af13acdae9906c0a847cd3d
Reviewed-on: https://chromium-review.googlesource.com/817600
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522913}
[modify] https://crrev.com/36222509aa6e819815938cbf2709b4849735537c/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
[modify] https://crrev.com/36222509aa6e819815938cbf2709b4849735537c/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
[modify] https://crrev.com/36222509aa6e819815938cbf2709b4849735537c/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc
[modify] https://crrev.com/36222509aa6e819815938cbf2709b4849735537c/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h
[modify] https://crrev.com/36222509aa6e819815938cbf2709b4849735537c/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/36222509aa6e819815938cbf2709b4849735537c/components/data_reduction_proxy/core/browser/warmup_url_fetcher.cc
[modify] https://crrev.com/36222509aa6e819815938cbf2709b4849735537c/components/data_reduction_proxy/core/browser/warmup_url_fetcher.h
[modify] https://crrev.com/36222509aa6e819815938cbf2709b4849735537c/components/data_reduction_proxy/core/browser/warmup_url_fetcher_unittest.cc
[modify] https://crrev.com/36222509aa6e819815938cbf2709b4849735537c/components/data_reduction_proxy/core/common/data_reduction_proxy_server.cc
[modify] https://crrev.com/36222509aa6e819815938cbf2709b4849735537c/components/data_reduction_proxy/core/common/data_reduction_proxy_server.h
[modify] https://crrev.com/36222509aa6e819815938cbf2709b4849735537c/tools/metrics/histograms/histograms.xml

Project Member

Comment 24 by bugdroid1@chromium.org, Dec 12 2017

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

commit 2407ed2ba75414d7766ce83b226b6102f830afe0
Author: Tarun Bansal <tbansal@chromium.org>
Date: Tue Dec 12 17:14:26 2017

Do not restart probe requests to data saver proxy

Probe requests (i.e., warmup URL fetches) may be
intercepted and restarted by the data reduction proxy
component. This CL prevents the probe requests from
being restarted.

Bug:  760294 ,792841
Change-Id: Iee54a9dce507d48121e71c58ff2079b9b1b67f1b
Reviewed-on: https://chromium-review.googlesource.com/818385
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523463}
[modify] https://crrev.com/2407ed2ba75414d7766ce83b226b6102f830afe0/components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor.cc

Project Member

Comment 25 by bugdroid1@chromium.org, Dec 18 2017

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

commit be009940b1f647e4ad238d9c58a17252f91e86b3
Author: Tarun Bansal <tbansal@chromium.org>
Date: Mon Dec 18 21:11:51 2017

Reland of: Disable data saver proxy if the warmup URL fetch fails

If the fetch of the warmup URL (aka probe URL) fails, the
corresponding proxy is disabled, and the config is reloaded.

A future CL will add retries -- If the fetch of the probe URL
due to network flakiness fails, then the probe will be retried
(up to 3 times) on the same proxy.

The CL was reverted because it was causing crashes in the wild.

The latest patch guards the code behind a field trial experiment.
Also, crrev.com/2407ed2ba75414d7766ce83b226b6102f830afe0 is
expected to fix the issue that caused crashes.

Bug:  760294 
Change-Id: I850c9444904b3b6be8158b8aef5224bcd67b156f
TBR: holte@chromium.org
Reviewed-on: https://chromium-review.googlesource.com/822214
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524797}
[modify] https://crrev.com/be009940b1f647e4ad238d9c58a17252f91e86b3/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
[modify] https://crrev.com/be009940b1f647e4ad238d9c58a17252f91e86b3/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
[modify] https://crrev.com/be009940b1f647e4ad238d9c58a17252f91e86b3/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_service_client_unittest.cc
[modify] https://crrev.com/be009940b1f647e4ad238d9c58a17252f91e86b3/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h
[modify] https://crrev.com/be009940b1f647e4ad238d9c58a17252f91e86b3/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/be009940b1f647e4ad238d9c58a17252f91e86b3/components/data_reduction_proxy/core/browser/warmup_url_fetcher.cc
[modify] https://crrev.com/be009940b1f647e4ad238d9c58a17252f91e86b3/components/data_reduction_proxy/core/browser/warmup_url_fetcher.h
[modify] https://crrev.com/be009940b1f647e4ad238d9c58a17252f91e86b3/components/data_reduction_proxy/core/browser/warmup_url_fetcher_unittest.cc
[modify] https://crrev.com/be009940b1f647e4ad238d9c58a17252f91e86b3/tools/metrics/histograms/histograms.xml

Project Member

Comment 26 by bugdroid1@chromium.org, Dec 27 2017

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

commit 8aebce8d29600205d2a720326d2e166f6a130003
Author: Tarun Bansal <tbansal@chromium.org>
Date: Wed Dec 27 21:39:42 2017

Retry fetching of probe URL up to 3 times

In data reduction proxy, each proxy type is probed separately
up to 3 times.

In the proxy resolution function, add logic for probing a
proxy even if the proxy is otherwise disabled.

Bug:  760294 
Change-Id: I57fe51ddb736552e3398de928be31d2a9b5cc3f8
Reviewed-on: https://chromium-review.googlesource.com/764859
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526244}
[modify] https://crrev.com/8aebce8d29600205d2a720326d2e166f6a130003/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
[modify] https://crrev.com/8aebce8d29600205d2a720326d2e166f6a130003/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
[modify] https://crrev.com/8aebce8d29600205d2a720326d2e166f6a130003/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.cc
[modify] https://crrev.com/8aebce8d29600205d2a720326d2e166f6a130003/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h
[modify] https://crrev.com/8aebce8d29600205d2a720326d2e166f6a130003/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/8aebce8d29600205d2a720326d2e166f6a130003/components/data_reduction_proxy/core/browser/data_reduction_proxy_configurator.cc
[modify] https://crrev.com/8aebce8d29600205d2a720326d2e166f6a130003/components/data_reduction_proxy/core/browser/data_reduction_proxy_configurator.h
[modify] https://crrev.com/8aebce8d29600205d2a720326d2e166f6a130003/components/data_reduction_proxy/core/browser/data_reduction_proxy_configurator_unittest.cc
[modify] https://crrev.com/8aebce8d29600205d2a720326d2e166f6a130003/components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate.cc
[modify] https://crrev.com/8aebce8d29600205d2a720326d2e166f6a130003/components/data_reduction_proxy/core/browser/data_reduction_proxy_delegate_unittest.cc
[modify] https://crrev.com/8aebce8d29600205d2a720326d2e166f6a130003/components/data_reduction_proxy/core/browser/network_properties_manager.cc
[modify] https://crrev.com/8aebce8d29600205d2a720326d2e166f6a130003/components/data_reduction_proxy/core/browser/network_properties_manager.h
[modify] https://crrev.com/8aebce8d29600205d2a720326d2e166f6a130003/components/data_reduction_proxy/core/browser/warmup_url_fetcher.cc
[modify] https://crrev.com/8aebce8d29600205d2a720326d2e166f6a130003/components/data_reduction_proxy/core/browser/warmup_url_fetcher.h
[modify] https://crrev.com/8aebce8d29600205d2a720326d2e166f6a130003/components/data_reduction_proxy/core/browser/warmup_url_fetcher_unittest.cc
[modify] https://crrev.com/8aebce8d29600205d2a720326d2e166f6a130003/components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc
[modify] https://crrev.com/8aebce8d29600205d2a720326d2e166f6a130003/components/data_reduction_proxy/core/common/data_reduction_proxy_params.h
[modify] https://crrev.com/8aebce8d29600205d2a720326d2e166f6a130003/components/data_reduction_proxy/core/common/data_reduction_proxy_params_unittest.cc

Project Member

Comment 27 by bugdroid1@chromium.org, Jan 4 2018

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

commit b441bd1035b1749791df52069a883d2e3a3bb0f1
Author: Tarun Bansal <tbansal@chromium.org>
Date: Thu Jan 04 22:56:55 2018

Data reduction proxy: Add delays between consecutive warmup probe URL

Add delays between the consecutive warmup probes. The delay is
exponentially increasing.

Bug:  760294 
Change-Id: I2d0ea28d07f532d81d84a2463b72a9243d90af3d
Reviewed-on: https://chromium-review.googlesource.com/850838
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527128}
[modify] https://crrev.com/b441bd1035b1749791df52069a883d2e3a3bb0f1/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
[modify] https://crrev.com/b441bd1035b1749791df52069a883d2e3a3bb0f1/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
[modify] https://crrev.com/b441bd1035b1749791df52069a883d2e3a3bb0f1/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.cc
[modify] https://crrev.com/b441bd1035b1749791df52069a883d2e3a3bb0f1/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_test_utils.h
[modify] https://crrev.com/b441bd1035b1749791df52069a883d2e3a3bb0f1/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/b441bd1035b1749791df52069a883d2e3a3bb0f1/components/data_reduction_proxy/core/browser/data_reduction_proxy_configurator_unittest.cc
[modify] https://crrev.com/b441bd1035b1749791df52069a883d2e3a3bb0f1/components/data_reduction_proxy/core/browser/network_properties_manager.cc
[modify] https://crrev.com/b441bd1035b1749791df52069a883d2e3a3bb0f1/components/data_reduction_proxy/core/browser/network_properties_manager.h
[modify] https://crrev.com/b441bd1035b1749791df52069a883d2e3a3bb0f1/components/data_reduction_proxy/core/browser/warmup_url_fetcher.cc
[modify] https://crrev.com/b441bd1035b1749791df52069a883d2e3a3bb0f1/components/data_reduction_proxy/core/browser/warmup_url_fetcher.h
[modify] https://crrev.com/b441bd1035b1749791df52069a883d2e3a3bb0f1/components/data_reduction_proxy/core/browser/warmup_url_fetcher_unittest.cc

Project Member

Comment 28 by bugdroid1@chromium.org, Jan 5 2018

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

commit 354c7688915f6f86de4dd4b6b6d3cd5df7daafc2
Author: Tarun Bansal <tbansal@chromium.org>
Date: Fri Jan 05 00:49:53 2018

Pass network quality estimator to warmup probe URL fetcher

Update the tests to ensure that the network
quality estimator is set on the URLRequestContextGetter that
is passed to the warmup URL fetcher. This would be used
in a subsequent CL to compute the fetcher timeout.

Bug:  760294 
Change-Id: I2c2081352127d7e21a00fb1935087e8555bf6fd6
Reviewed-on: https://chromium-review.googlesource.com/849759
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527173}
[modify] https://crrev.com/354c7688915f6f86de4dd4b6b6d3cd5df7daafc2/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/354c7688915f6f86de4dd4b6b6d3cd5df7daafc2/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
[modify] https://crrev.com/354c7688915f6f86de4dd4b6b6d3cd5df7daafc2/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
[modify] https://crrev.com/354c7688915f6f86de4dd4b6b6d3cd5df7daafc2/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
[modify] https://crrev.com/354c7688915f6f86de4dd4b6b6d3cd5df7daafc2/components/data_reduction_proxy/core/browser/warmup_url_fetcher.cc
[modify] https://crrev.com/354c7688915f6f86de4dd4b6b6d3cd5df7daafc2/components/data_reduction_proxy/core/browser/warmup_url_fetcher_unittest.cc

Project Member

Comment 29 by bugdroid1@chromium.org, Jan 5 2018

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

commit 1d2905e07fa0b31f53feb07d06110361a59d7ac5
Author: Vadym Doroshenko <dvadym@chromium.org>
Date: Fri Jan 05 10:20:33 2018

Revert "Pass network quality estimator to warmup probe URL fetcher"

This reverts commit 354c7688915f6f86de4dd4b6b6d3cd5df7daafc2.

Reason for revert: this CL is most likely reason of failing test DataReductionProxyProtocolEmbeddedServerTest.EmbeddedTestServerBypassRetryOnPostConnectionErrors on Windows, this tests started failing on build https://uberchromegw.corp.google.com/i/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/65587

Original change's description:
> Pass network quality estimator to warmup probe URL fetcher
> 
> Update the tests to ensure that the network
> quality estimator is set on the URLRequestContextGetter that
> is passed to the warmup URL fetcher. This would be used
> in a subsequent CL to compute the fetcher timeout.
> 
> Bug:  760294 
> Change-Id: I2c2081352127d7e21a00fb1935087e8555bf6fd6
> Reviewed-on: https://chromium-review.googlesource.com/849759
> Commit-Queue: Tarun Bansal <tbansal@chromium.org>
> Reviewed-by: Doug Arnett <dougarnett@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#527173}

TBR=tbansal@chromium.org,dougarnett@chromium.org

Change-Id: I7ff5b193e2282f759469bd3cd318fcb6bb53d05c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  760294 
Reviewed-on: https://chromium-review.googlesource.com/852112
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Commit-Queue: Vadym Doroshenko <dvadym@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527251}
[modify] https://crrev.com/1d2905e07fa0b31f53feb07d06110361a59d7ac5/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/1d2905e07fa0b31f53feb07d06110361a59d7ac5/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
[modify] https://crrev.com/1d2905e07fa0b31f53feb07d06110361a59d7ac5/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
[modify] https://crrev.com/1d2905e07fa0b31f53feb07d06110361a59d7ac5/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
[modify] https://crrev.com/1d2905e07fa0b31f53feb07d06110361a59d7ac5/components/data_reduction_proxy/core/browser/warmup_url_fetcher.cc
[modify] https://crrev.com/1d2905e07fa0b31f53feb07d06110361a59d7ac5/components/data_reduction_proxy/core/browser/warmup_url_fetcher_unittest.cc

Comment 30 by kbr@chromium.org, Jan 5 2018

Blockedon: 799376
Project Member

Comment 31 by bugdroid1@chromium.org, Jan 5 2018

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

commit 92fb984481eb64b47a0c58855743d1f36aa7f1a4
Author: Tarun Bansal <tbansal@chromium.org>
Date: Fri Jan 05 22:54:19 2018

Reland "Pass network quality estimator to warmup probe URL fetcher"

Update the tests to ensure that the network
quality estimator is set on the URLRequestContextGetter that
is passed to the warmup URL fetcher. This would be used
in a subsequent CL to compute the fetcher timeout.

Bug:  760294 
Change-Id: I78805cb8494aa63bf3dd9d07f6a109f3405818fd
Reviewed-on: https://chromium-review.googlesource.com/852893
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527419}
[modify] https://crrev.com/92fb984481eb64b47a0c58855743d1f36aa7f1a4/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/92fb984481eb64b47a0c58855743d1f36aa7f1a4/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
[modify] https://crrev.com/92fb984481eb64b47a0c58855743d1f36aa7f1a4/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
[modify] https://crrev.com/92fb984481eb64b47a0c58855743d1f36aa7f1a4/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
[modify] https://crrev.com/92fb984481eb64b47a0c58855743d1f36aa7f1a4/components/data_reduction_proxy/core/browser/warmup_url_fetcher.cc
[modify] https://crrev.com/92fb984481eb64b47a0c58855743d1f36aa7f1a4/components/data_reduction_proxy/core/browser/warmup_url_fetcher_unittest.cc

Project Member

Comment 32 by bugdroid1@chromium.org, Jan 6 2018

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

commit d2ae2b45cffbb84d41edd549a06804c0f49c0702
Author: Mike Wittman <wittman@chromium.org>
Date: Sat Jan 06 00:50:49 2018

Revert "Reland "Pass network quality estimator to warmup probe URL fetcher""

This reverts commit 92fb984481eb64b47a0c58855743d1f36aa7f1a4.

Reason for revert: crashing Win7 Tests DataReductionProxyProtocolEmbeddedServerTest.EmbeddedTestServerBypassRetryOnPostConnectionErrors

Received fatal exception EXCEPTION_ACCESS_VIOLATION
Backtrace:
	net::NetLog::ThreadSafeObserver::OnAddEntryData [0x02F98C2C+28]
	net::NetLog::AddEntry [0x02F98D40+108]
	net::NetLogWithSource::AddEntry [0x02FA6B57+33]
	net::NetLogWithSource::AddEvent [0x02FA6B7E+16]
	net::nqe::internal::EventCreator::MaybeAddNetworkQualityChangedEventToNetLog [0x030E83F1+239]
	net::NetworkQualityEstimator::ComputeEffectiveConnectionType [0x0306231C+588]
	net::NetworkQualityEstimator::MaybeComputeEffectiveConnectionType [0x03062965+309]
	net::NetworkQualityEstimator::AddAndNotifyObserversOfThroughput [0x03061E2D+355]
	net::NetworkQualityEstimator::OnNewThroughputObservationAvailable [0x03061749+89]
	base::internal::Invoker<base::internal::BindState<void (__thiscall content::ServiceWorkerRegistration::*)(enum content::ServiceWorkerStatusCode),scoped_refptr<content::ServiceWorkerRegistration> >,void __cdecl(enum content::ServiceWorkerStatusCode)>::RunO [0x02BBD092+18]
	base::internal::Invoker<base::internal::BindState<base::RepeatingCallback<bool __cdecl(storage::QuotaDatabase *)>,base::internal::UnretainedWrapper<storage::QuotaDatabase> >,bool __cdecl(void)>::Run [0x03DEE260+16]
	base::debug::TaskAnnotator::RunTask [0x02F4B2F9+153]
	base::internal::IncomingTaskQueue::RunTask [0x02F4FBF3+19]
	base::MessageLoop::RunTask [0x02ED9126+438]
	base::MessageLoop::DeferOrRunPendingTask [0x02ED9467+87]
	base::MessageLoop::DoWork [0x02ED957E+222]
	base::MessagePumpForIO::DoRunLoop [0x02F51E43+35]
	base::MessagePumpWin::Run [0x02F514FE+110]
	base::MessageLoop::Run [0x02ED8E7F+31]
	base::RunLoop::Run [0x02EDA01E+46]
	net::test_server::EmbeddedTestServer::PostTaskToIOThreadAndWait [0x03350435+261]
	net::test_server::EmbeddedTestServer::ShutdownAndWaitUntilComplete [0x0334F8CF+79]
	net::test_server::EmbeddedTestServer::~EmbeddedTestServer [0x0334F775+43]
	net::TestNetworkQualityEstimator::~TestNetworkQualityEstimator [0x0334F40E+48]
	net::TestNetworkQualityEstimator::`scalar deleting destructor' [0x0334F1CB+11]
	data_reduction_proxy::DataReductionProxyTestContext::~DataReductionProxyTestContext [0x04A011BC+102]
	data_reduction_proxy::DataReductionProxyTestContext::`scalar deleting destructor' [0x049FDC7B+11]
	data_reduction_proxy::DataReductionProxyProtocolEmbeddedServerTest::~DataReductionProxyProtocolEmbeddedServerTest [0x01C42B0E+76]
	data_reduction_proxy::DataReductionProxyProtocolEmbeddedServerTest_EmbeddedTestServerBypassRetryOnPostConnectionErrors_Test::`scalar deleting destructor' [0x01C4296B+11]
	content::ServiceWorkerQuotaClient::OnQuotaManagerDestroyed [0x028C75FD+13]
	testing::TestInfo::Run [0x01FA0AED+273]
	testing::TestCase::Run [0x01FA0E43+237]
	testing::internal::UnitTestImpl::RunAllTests [0x01FA4C33+627]
	testing::UnitTest::Run [0x01FA48DB+153]
	base::TestSuite::Run [0x03363C6A+102]
	base::LaunchUnitTests [0x03361040+416]
	base::LaunchUnitTests [0x03360F16+118]
	main [0x01AB9954+72]
	__scrt_common_main_seh [0x04FDF87A+248] (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:283)
	BaseThreadInitThunk [0x75FC336A+18]
	RtlInitializeExceptionChain [0x77789902+99]
	RtlInitializeExceptionChain [0x777898D5+54]

https://uberchromegw.corp.google.com/i/chromium.win/builders/Win7%20Tests%20%281%29/builds/75420

Original change's description:
> Reland "Pass network quality estimator to warmup probe URL fetcher"
> 
> Update the tests to ensure that the network
> quality estimator is set on the URLRequestContextGetter that
> is passed to the warmup URL fetcher. This would be used
> in a subsequent CL to compute the fetcher timeout.
> 
> Bug:  760294 
> Change-Id: I78805cb8494aa63bf3dd9d07f6a109f3405818fd
> Reviewed-on: https://chromium-review.googlesource.com/852893
> Reviewed-by: Doug Arnett <dougarnett@chromium.org>
> Reviewed-by: Tarun Bansal <tbansal@chromium.org>
> Commit-Queue: Tarun Bansal <tbansal@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#527419}

TBR=tbansal@chromium.org,dvadym@chromium.org,dougarnett@chromium.org

Change-Id: Icb606b8c667f97c84f5e539369619805e35d3f9b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  760294 
Reviewed-on: https://chromium-review.googlesource.com/853141
Reviewed-by: Mike Wittman <wittman@chromium.org>
Commit-Queue: Mike Wittman <wittman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527463}
[modify] https://crrev.com/d2ae2b45cffbb84d41edd549a06804c0f49c0702/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/d2ae2b45cffbb84d41edd549a06804c0f49c0702/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
[modify] https://crrev.com/d2ae2b45cffbb84d41edd549a06804c0f49c0702/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
[modify] https://crrev.com/d2ae2b45cffbb84d41edd549a06804c0f49c0702/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
[modify] https://crrev.com/d2ae2b45cffbb84d41edd549a06804c0f49c0702/components/data_reduction_proxy/core/browser/warmup_url_fetcher.cc
[modify] https://crrev.com/d2ae2b45cffbb84d41edd549a06804c0f49c0702/components/data_reduction_proxy/core/browser/warmup_url_fetcher_unittest.cc

Project Member

Comment 33 by bugdroid1@chromium.org, Jan 8 2018

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

commit cd5bbc66fa1c412fa3cacc5cb8c01dee1b508a63
Author: Tarun Bansal <tbansal@chromium.org>
Date: Mon Jan 08 19:39:16 2018

Reland "Pass network quality estimator to warmup probe URL fetcher"

Update the tests to ensure that the network
quality estimator is set on the URLRequestContextGetter that
is passed to the warmup URL fetcher. This would be used
in a subsequent CL to compute the fetcher timeout.

Bug:  760294 
Change-Id: Icacf6a68a616a7da7c6f729028f1205d2fb3532f
Reviewed-on: https://chromium-review.googlesource.com/854477
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527713}
[modify] https://crrev.com/cd5bbc66fa1c412fa3cacc5cb8c01dee1b508a63/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/cd5bbc66fa1c412fa3cacc5cb8c01dee1b508a63/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
[modify] https://crrev.com/cd5bbc66fa1c412fa3cacc5cb8c01dee1b508a63/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
[modify] https://crrev.com/cd5bbc66fa1c412fa3cacc5cb8c01dee1b508a63/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
[modify] https://crrev.com/cd5bbc66fa1c412fa3cacc5cb8c01dee1b508a63/components/data_reduction_proxy/core/browser/warmup_url_fetcher.cc
[modify] https://crrev.com/cd5bbc66fa1c412fa3cacc5cb8c01dee1b508a63/components/data_reduction_proxy/core/browser/warmup_url_fetcher_unittest.cc
[modify] https://crrev.com/cd5bbc66fa1c412fa3cacc5cb8c01dee1b508a63/net/nqe/network_quality_estimator_test_util.cc
[modify] https://crrev.com/cd5bbc66fa1c412fa3cacc5cb8c01dee1b508a63/net/nqe/network_quality_estimator_test_util.h

Project Member

Comment 34 by bugdroid1@chromium.org, Jan 17 2018

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

commit ab8f569fd1b236f6ea530c0429b7eff65bdc663c
Author: Tarun Bansal <tbansal@chromium.org>
Date: Wed Jan 17 03:48:01 2018

Update warmup URL fetcher callback to return an enum instead of boolean

The callback is updated to return an enum. This allows data reduction
proxy config class to distinguish between failure and timeout.

Bug:  760294 
Change-Id: I6014a699733ba2d8859e92c2a6354f8b50a2f077
Reviewed-on: https://chromium-review.googlesource.com/865961
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#529583}
[modify] https://crrev.com/ab8f569fd1b236f6ea530c0429b7eff65bdc663c/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
[modify] https://crrev.com/ab8f569fd1b236f6ea530c0429b7eff65bdc663c/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.h
[modify] https://crrev.com/ab8f569fd1b236f6ea530c0429b7eff65bdc663c/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/ab8f569fd1b236f6ea530c0429b7eff65bdc663c/components/data_reduction_proxy/core/browser/warmup_url_fetcher.cc
[modify] https://crrev.com/ab8f569fd1b236f6ea530c0429b7eff65bdc663c/components/data_reduction_proxy/core/browser/warmup_url_fetcher.h
[modify] https://crrev.com/ab8f569fd1b236f6ea530c0429b7eff65bdc663c/components/data_reduction_proxy/core/browser/warmup_url_fetcher_unittest.cc
[modify] https://crrev.com/ab8f569fd1b236f6ea530c0429b7eff65bdc663c/testing/variations/fieldtrial_testing_config.json

Project Member

Comment 35 by bugdroid1@chromium.org, Jan 22 2018

Labels: -M-64 M-65 Merge-Request-65
Requesting M-65 merge for the CL in comment #35. This CL is pretty safe, and the logic is behind a finch trial. This CL is needed to start the experiment for probing data reduction proxies, which would help us in improving the data compression.
Labels: OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows
Project Member

Comment 38 by sheriffbot@chromium.org, Jan 23 2018

Labels: -Merge-Request-65 Merge-Review-65 Hotlist-Merge-Review
This bug requires manual review: Reverts referenced in bugdroid comments after merge request.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Blockedon: 805072
Labels: -Merge-Review-65 Merge-Approved-65
Approving merge for CL listed at #35 to M65 branch 3325 based on comment #36. Please merge ASAP. Thank you.
Project Member

Comment 41 by bugdroid1@chromium.org, Jan 25 2018

Labels: -merge-approved-65 merge-merged-3325
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c9211fedaa91097c95a9cb96f316b193e0d562d5

commit c9211fedaa91097c95a9cb96f316b193e0d562d5
Author: Tarun Bansal <tbansal@chromium.org>
Date: Thu Jan 25 00:20:35 2018

Add timeout fetching of the warmup probe URL

A timeout value is set based on the current network quality.
If the fetch times out, the proxy is marked as bad if the warmup URL
fetcher callback is enabled.

Bug:  760294 
Change-Id: I4dda83ec9adcade69581ec8b097ec980e954fc5a
Reviewed-on: https://chromium-review.googlesource.com/849497
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#530917}(cherry picked from commit 6cf59d5dfa2e3e0f4302dd53d2b9abb948483c46)
Reviewed-on: https://chromium-review.googlesource.com/884982
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#80}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/c9211fedaa91097c95a9cb96f316b193e0d562d5/components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc
[modify] https://crrev.com/c9211fedaa91097c95a9cb96f316b193e0d562d5/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/c9211fedaa91097c95a9cb96f316b193e0d562d5/components/data_reduction_proxy/core/browser/warmup_url_fetcher.cc
[modify] https://crrev.com/c9211fedaa91097c95a9cb96f316b193e0d562d5/components/data_reduction_proxy/core/browser/warmup_url_fetcher.h
[modify] https://crrev.com/c9211fedaa91097c95a9cb96f316b193e0d562d5/components/data_reduction_proxy/core/browser/warmup_url_fetcher_unittest.cc

Blocking: 724704
Blockedon: 806114
Blockedon: 813184
Blockedon: 813186
Project Member

Comment 46 by bugdroid1@chromium.org, Mar 15 2018

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

commit d23ee3c1203ae6c57becbc885b77eee3b23efba9
Author: Tarun Bansal <tbansal@chromium.org>
Date: Thu Mar 15 18:31:13 2018

Add a DCHECK to ensure that probe responses are not checked for explicit bypasses

Add an explicit DCHECK in the data reduction proxy (DRP) to
ensure that we do not check for explicit bypasses
in the responses.

Bug:  760294 
Change-Id: Ifd683efc79d484388f2446757e9025068c1d6fb8
Reviewed-on: https://chromium-review.googlesource.com/963100
Reviewed-by: Doug Arnett <dougarnett@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#543452}
[modify] https://crrev.com/d23ee3c1203ae6c57becbc885b77eee3b23efba9/components/data_reduction_proxy/core/common/data_reduction_proxy_headers.cc

Blockedon: 830088
Blockedon: 839628
Blocking: 841842
tbansal, please update with what work is left since it is not clear based on all of the CLs that have landed.
The CLs have landed, and the experiment is running on M-67 Beta.
Blockedon: 849292
Project Member

Comment 53 by bugdroid1@chromium.org, Jun 18 2018

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

commit 16eaec904492bf1aa0d7a024f0ca62140022060b
Author: Tarun Bansal <tbansal@chromium.org>
Date: Mon Jun 18 18:20:51 2018

Update the warmup timeout params based in data reduction proxy.

These are the params that we have been using in the server-side field trial.

Bug:  760294 
Change-Id: I625e8c41b8acc005edaec8515b617198013b775a
Reviewed-on: https://chromium-review.googlesource.com/1104778
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#568079}
[modify] https://crrev.com/16eaec904492bf1aa0d7a024f0ca62140022060b/components/data_reduction_proxy/core/browser/warmup_url_fetcher.cc
[modify] https://crrev.com/16eaec904492bf1aa0d7a024f0ca62140022060b/components/data_reduction_proxy/core/browser/warmup_url_fetcher_unittest.cc

Refreshed during triage.
Project Member

Comment 55 by bugdroid1@chromium.org, Aug 14

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

commit 27e13f6270e8452df31e0488fd86d761aa559f45
Author: Tarun Bansal <tbansal@chromium.org>
Date: Tue Aug 14 05:24:18 2018

Experiment with changing minimum probe timeout value.

It seems that the current minimum timeout value of 10 seconds
is too low. UMA shows that on 2G connections, of all succeeded
probes, only 88% of them succeed on the first attempt.
9% succeed on second attempt when the timeout is doubled,
and 3% on third attempt when timeout is further doubled.

This CL experiments with increasing the minimum timeout value
to increase the chances of success.

Bug:  760294 
Change-Id: I79eb5addf2258641032b3a92053228d3f15763eb
Reviewed-on: https://chromium-review.googlesource.com/1173157
Reviewed-by: Robert Kaplow (slow) <rkaplow@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582837}
[modify] https://crrev.com/27e13f6270e8452df31e0488fd86d761aa559f45/testing/variations/fieldtrial_testing_config.json

tbansal: Can this bug be closed, since tuning the probe timeout is not a goal of this bug.

I do not see any improvement in DataReductionProxy.WarmupURL.FetchAttemptsBeforeSuccess.Secure.Core with the latest params ?
It seems worse than the previous experiment ?
https://uma.googleplex.com/p/chrome/variations/?sid=6f7005ab55da3473b5ee3b0ca6a89e6c
Summary of results from https://uma.googleplex.com/p/chrome/variations/?sid=dba9d8f1c8eb13d813cdf47a36179200 (28-day data ending Oct 09). Note that results are not statistically significant, but they are all positive.

Note that we are comparing Control with Enabled_NoViaNoBypass_v7:
1. Sum in Net.HttpContentLengthDifferenceWithValidOCL increased by ~3% indicating more data savings for users. This is probably because of not bypassing on via header.
2. PageLoad.Clients.DataReductionProxy.PaintTiming.NavigationToFirstContentfulPaint has 4.9% more counts. This implies more pages loaded via proxy.
3. PageLoad.PaintTiming.NavigationToFirstContentfulPaint has 1.5% more samples.
4. From Net.ErrorCodesForMainFrame4, there is ~2% increase in OK bucket count.
5. MobilePullGestureReload count increased which is bad. However, counts in PageLoad.PaintTiming.NavigationToFirstContentfulPaint increased by 33x that amount. So, I am not too worried about MobilePullGestureReload count increase.


I am planning to enable this by default.
Project Member

Comment 58 by bugdroid1@chromium.org, Oct 18

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

commit 2fac591598af1a5769da0e8096f747a9e118febb
Author: Tarun Bansal <tbansal@chromium.org>
Date: Thu Oct 18 20:08:22 2018

Data reduction proxy: Disable proxies based on probing result

Enable the experiment that probes data saver proxies, and disables
a proxy if the probe to it fails.

Change-Id: I037d000f375d9a1eddf3515e4929948cafbd7d36
Bug:  760294 
Reviewed-on: https://chromium-review.googlesource.com/c/1280142
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Robert Kaplow (sloooow) <rkaplow@chromium.org>
Reviewed-by: rajendrant <rajendrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600880}
[modify] https://crrev.com/2fac591598af1a5769da0e8096f747a9e118febb/chrome/browser/net/spdyproxy/data_reduction_proxy_browsertest.cc
[modify] https://crrev.com/2fac591598af1a5769da0e8096f747a9e118febb/components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_protocol_unittest.cc
[modify] https://crrev.com/2fac591598af1a5769da0e8096f747a9e118febb/components/data_reduction_proxy/core/browser/data_reduction_proxy_bypass_stats_unittest.cc
[modify] https://crrev.com/2fac591598af1a5769da0e8096f747a9e118febb/components/data_reduction_proxy/core/browser/data_reduction_proxy_config_unittest.cc
[modify] https://crrev.com/2fac591598af1a5769da0e8096f747a9e118febb/components/data_reduction_proxy/core/browser/data_reduction_proxy_interceptor_unittest.cc
[modify] https://crrev.com/2fac591598af1a5769da0e8096f747a9e118febb/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc
[modify] https://crrev.com/2fac591598af1a5769da0e8096f747a9e118febb/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.cc
[modify] https://crrev.com/2fac591598af1a5769da0e8096f747a9e118febb/components/data_reduction_proxy/core/browser/data_reduction_proxy_test_utils.h
[modify] https://crrev.com/2fac591598af1a5769da0e8096f747a9e118febb/components/data_reduction_proxy/core/browser/warmup_url_fetcher.cc
[modify] https://crrev.com/2fac591598af1a5769da0e8096f747a9e118febb/components/data_reduction_proxy/core/browser/warmup_url_fetcher_unittest.cc
[modify] https://crrev.com/2fac591598af1a5769da0e8096f747a9e118febb/components/data_reduction_proxy/core/common/data_reduction_proxy_headers.cc
[modify] https://crrev.com/2fac591598af1a5769da0e8096f747a9e118febb/components/data_reduction_proxy/core/common/data_reduction_proxy_headers_unittest.cc
[modify] https://crrev.com/2fac591598af1a5769da0e8096f747a9e118febb/components/data_reduction_proxy/core/common/data_reduction_proxy_params.cc
[modify] https://crrev.com/2fac591598af1a5769da0e8096f747a9e118febb/components/data_reduction_proxy/core/common/data_reduction_proxy_params.h
[modify] https://crrev.com/2fac591598af1a5769da0e8096f747a9e118febb/components/data_reduction_proxy/core/common/data_reduction_proxy_params_unittest.cc
[modify] https://crrev.com/2fac591598af1a5769da0e8096f747a9e118febb/components/data_reduction_proxy/core/common/data_reduction_proxy_switches.cc
[modify] https://crrev.com/2fac591598af1a5769da0e8096f747a9e118febb/components/data_reduction_proxy/core/common/data_reduction_proxy_switches.h
[modify] https://crrev.com/2fac591598af1a5769da0e8096f747a9e118febb/testing/variations/fieldtrial_testing_config.json

Project Member

Comment 59 by bugdroid1@chromium.org, Oct 18

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

commit c752d8a20c37d9641bddcd8261b6fd67a6107b97
Author: Tarun Bansal <tbansal@chromium.org>
Date: Thu Oct 18 23:48:27 2018

Fix flaky data saver warmup browsertest

In local testing, there are two separate reasons for flakes:
(i) Histogram tester does not return correct values even though
the relevant code has already logged the histogram values
correctly.
(ii) The via header value is set too late by the unittest.

The first problem is sort of solved by waiting for the histogram
to populate. The second problem is solved by setting the via header
value in the test constructor.

Bug:  760294 
Change-Id: I9ae23e4b704f6eaa51fd9c37f76491008c6cb1c6
Reviewed-on: https://chromium-review.googlesource.com/c/1289689
Reviewed-by: Clark DuVall <cduvall@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#600967}
[modify] https://crrev.com/c752d8a20c37d9641bddcd8261b6fd67a6107b97/chrome/browser/net/spdyproxy/data_reduction_proxy_browsertest.cc
[modify] https://crrev.com/c752d8a20c37d9641bddcd8261b6fd67a6107b97/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter

Labels: -M-65 M-70
Status: Fixed (was: Started)
Blockedon: 849387

Sign in to add a comment