Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(237)

Issue 2873793002: Record Data Savings for Client-Side LoFi (Closed)

Created:
3 years, 7 months ago by sclittle
Modified:
3 years, 7 months ago
CC:
chromium-reviews, tyoshino+watch_chromium.org, jam, loading-reviews_chromium.org, dglazkov+blink, darin-cc_chromium.org, gavinp+loader_chromium.org, blink-reviews, loading-reviews+fetch_chromium.org, kinuko+watch, Nate Chapin, tbansal+watch-data-reduction-proxy_chromium.org, blink-reviews-api_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Record Data Savings for Client-Side LoFi This CL causes Chrome to record data savings for Client LoFi when it is used, according to the Content-Range header if it's present (e.g. "Content-Range: bytes 0-2047/10000" implies that the original resource was 10000 bytes). This CL also adds a new PreviewsState bit field, CLIENT_LOFI_AUTO_RELOAD, that is set when a Client LoFi image is auto reloaded because of a decoding error, so that the browser process can count the data used for the reload against the savings. See design doc for more info: https://docs.google.com/document/d/1CoZfTswPq67VaHKsr0OrkpPbWT87dhhvPq9hylOxrlU/edit BUG=605347 Review-Url: https://codereview.chromium.org/2873793002 Cr-Commit-Position: refs/heads/master@{#470850} Committed: https://chromium.googlesource.com/chromium/src/+/afaa2a65a0d50a88e08fd595507668538cb6689e

Patch Set 1 #

Patch Set 2 : Rebased on master #

Total comments: 6

Patch Set 3 : Addressed comments #

Patch Set 4 : fix crash in DRPNetworkDelegate when lofi_ui_service is null. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+390 lines, -83 lines) Patch
M components/data_reduction_proxy/content/browser/content_lofi_decider.h View 1 1 chunk +2 lines, -0 lines 0 comments Download
M components/data_reduction_proxy/content/browser/content_lofi_decider.cc View 1 1 chunk +7 lines, -0 lines 0 comments Download
M components/data_reduction_proxy/content/browser/content_lofi_decider_unittest.cc View 1 1 chunk +28 lines, -0 lines 0 comments Download
M components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc View 1 2 3 4 chunks +48 lines, -16 lines 0 comments Download
M components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate_unittest.cc View 1 2 5 chunks +193 lines, -0 lines 0 comments Download
M components/data_reduction_proxy/core/common/lofi_decider.h View 1 1 chunk +5 lines, -0 lines 0 comments Download
M content/public/common/previews_state.h View 2 chunks +18 lines, -13 lines 0 comments Download
M third_party/WebKit/Source/core/loader/resource/ImageResource.cpp View 1 1 chunk +14 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/loader/resource/ImageResourceTest.cpp View 1 1 chunk +56 lines, -37 lines 0 comments Download
M third_party/WebKit/Source/platform/loader/fetch/Resource.h View 1 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/platform/loader/fetch/Resource.cpp View 1 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/public/platform/WebURLRequest.h View 1 chunk +16 lines, -13 lines 0 comments Download

Messages

Total messages: 30 (16 generated)
sclittle
megjablon: all japhet: third_part/WebKit/* kinuko: content/public/common/previews_state.h Thanks in advance!
3 years, 7 months ago (2017-05-09 22:05:49 UTC) #2
kinuko
lgtm
3 years, 7 months ago (2017-05-10 07:00:46 UTC) #4
Nate Chapin
blink lgtm
3 years, 7 months ago (2017-05-10 17:18:41 UTC) #5
RyanSturm
lgtm % a couple minor nits. I'd be fine with it as is though. https://codereview.chromium.org/2873793002/diff/20001/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc ...
3 years, 7 months ago (2017-05-10 18:14:09 UTC) #7
sclittle
https://codereview.chromium.org/2873793002/diff/20001/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc File components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc (right): https://codereview.chromium.org/2873793002/diff/20001/components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc#newcode133 components/data_reduction_proxy/core/browser/data_reduction_proxy_network_delegate.cc:133: return std::max<int64_t>(0, request.GetTotalReceivedBytes() - On 2017/05/10 18:14:08, RyanSturm wrote: ...
3 years, 7 months ago (2017-05-10 19:56:37 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2873793002/40001
3 years, 7 months ago (2017-05-10 19:58:45 UTC) #11
commit-bot: I haz the power
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_swarming_rel/builds/175175)
3 years, 7 months ago (2017-05-10 20:59:26 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2873793002/40001
3 years, 7 months ago (2017-05-10 21:12:01 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2873793002/60001
3 years, 7 months ago (2017-05-10 21:36:18 UTC) #19
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/450926)
3 years, 7 months ago (2017-05-11 00:00:59 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2873793002/60001
3 years, 7 months ago (2017-05-11 00:17:57 UTC) #23
commit-bot: I haz the power
Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_rel_ng/builds/451157)
3 years, 7 months ago (2017-05-11 03:21:12 UTC) #25
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2873793002/60001
3 years, 7 months ago (2017-05-11 03:25:12 UTC) #27
commit-bot: I haz the power
3 years, 7 months ago (2017-05-11 06:42:15 UTC) #30
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/afaa2a65a0d50a88e08fd5955076...

Powered by Google App Engine
This is Rietveld 408576698