Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2687193006:
DRP: Do not fetch warm up URL on offline connections (Closed)
DescriptionDRP: Do not fetch warm up URL on offline connections
This CL makes a change to not fetch preconnect / warmup URL for data
reduction proxy (DRP) if the connectivity is unavailable.
BUG=691620
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
Review-Url: https://codereview.chromium.org/2687193006
Cr-Commit-Position: refs/heads/master@{#450589}
Committed: https://chromium.googlesource.com/chromium/src/+/73319e0970ff6421af9ebd06e6cef8de27a8d42b
Patch Set 1 : ps #Patch Set 2 : rebased #
Total comments: 2
Patch Set 3 : ryansturm comments #
Messages
Total messages: 27 (22 generated)
Description was changed from ========== Do not fetch warm up URL on offline connections BUG= ========== to ========== Do not fetch warm up URL on offline connections This CL makes a change to not fetch warm up URL if connectivity is unavailable. BUG=672334 ==========
Description was changed from ========== Do not fetch warm up URL on offline connections This CL makes a change to not fetch warm up URL if connectivity is unavailable. BUG=672334 ========== to ========== DRP: Do not fetch warm up URL on offline connections This CL makes a change to not fetch preconnect / warmup URL for data reduction proxy if the connectivity is unavailable. BUG=691620 ==========
Patchset #2 (id:20001) has been deleted
Patchset #1 (id:1) has been deleted
Description was changed from ========== DRP: Do not fetch warm up URL on offline connections This CL makes a change to not fetch preconnect / warmup URL for data reduction proxy if the connectivity is unavailable. BUG=691620 ========== to ========== DRP: Do not fetch warm up URL on offline connections This CL makes a change to not fetch preconnect / warmup URL for data reduction proxy if the connectivity is unavailable. BUG=691620 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester ==========
Description was changed from ========== DRP: Do not fetch warm up URL on offline connections This CL makes a change to not fetch preconnect / warmup URL for data reduction proxy if the connectivity is unavailable. BUG=691620 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester ========== to ========== DRP: Do not fetch warm up URL on offline connections This CL makes a change to not fetch preconnect / warmup URL for data reduction proxy (DRP) if the connectivity is unavailable. BUG=691620 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester ==========
tbansal@chromium.org changed reviewers: + ryansturm@chromium.org
ryansturm: ptal. Thanks.
The CQ bit was checked by tbansal@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by tbansal@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm % nit. If you decide to change, tell me to look again. https://codereview.chromium.org/2687193006/diff/60001/components/data_reducti... File components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc (right): https://codereview.chromium.org/2687193006/diff/60001/components/data_reducti... components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc:558: if (connection_type_ != connection_type_last_quality_check_) { nit: Can you simplify all this logic with two ConnectionType members to a bool: bool network_type_changed = network_type_changed_since_last_check_; network_type_changed_since_last_check_ = false; in OnConnectionTypeChanged: network_type_changed_since_last_check_ = true; I think that approach catches WiFi -> other WiFi connection switches as well which the current approach misses. You might have to store connection_type_last_quality_check_ still either way for the things that use it.
Patchset #3 (id:80001) has been deleted
ryansturm: ptal. Thanks. https://codereview.chromium.org/2687193006/diff/60001/components/data_reducti... File components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc (right): https://codereview.chromium.org/2687193006/diff/60001/components/data_reducti... components/data_reduction_proxy/core/browser/data_reduction_proxy_config.cc:558: if (connection_type_ != connection_type_last_quality_check_) { On 2017/02/14 18:25:57, Ryan Sturm wrote: > nit: > Can you simplify all this logic with two ConnectionType members to a bool: > bool network_type_changed = network_type_changed_since_last_check_; > network_type_changed_since_last_check_ = false; > > in OnConnectionTypeChanged: > network_type_changed_since_last_check_ = true; > > I think that approach catches WiFi -> other WiFi connection switches as well > which the current approach misses. You might have to store > connection_type_last_quality_check_ still either way for the things that use it. Done.
The CQ bit was checked by tbansal@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by tbansal@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from ryansturm@chromium.org Link to the patchset: https://codereview.chromium.org/2687193006/#ps100001 (title: "ryansturm comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 100001, "attempt_start_ts": 1487133831762280,
"parent_rev": "0e0444a4895f138db12a1722679d8a6be17fad11", "commit_rev":
"73319e0970ff6421af9ebd06e6cef8de27a8d42b"}
Message was sent while issue was closed.
Description was changed from ========== DRP: Do not fetch warm up URL on offline connections This CL makes a change to not fetch preconnect / warmup URL for data reduction proxy (DRP) if the connectivity is unavailable. BUG=691620 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester ========== to ========== DRP: Do not fetch warm up URL on offline connections This CL makes a change to not fetch preconnect / warmup URL for data reduction proxy (DRP) if the connectivity is unavailable. BUG=691620 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester Review-Url: https://codereview.chromium.org/2687193006 Cr-Commit-Position: refs/heads/master@{#450589} Committed: https://chromium.googlesource.com/chromium/src/+/73319e0970ff6421af9ebd06e6ce... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:100001) as https://chromium.googlesource.com/chromium/src/+/73319e0970ff6421af9ebd06e6ce... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
