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

Issue 775773002: Add data reduction proxy debug info to net-internals#bandwidth (Closed)

Created:
6 years ago by jeremyim
Modified:
6 years ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, eroman, arv+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Add data reduction proxy debug info to net-internals#bandwidth - Save off various events in DataReductionProxyEventStore for later use (last bypass event, data reduction proxy configuration, canary request state) - Plumb DataReductionProxyEventStore through ChromeNetworkDelegate to net_internals_ui.cc for consumption by bandwidth_view.js/html - Update bandwidth_view.* for rendering the DataReductionProxyEventStore information BUG=346356 Committed: https://crrev.com/29b3b28bcd3ea31391f4ff1b1edc43c18d788c98 Cr-Commit-Position: refs/heads/master@{#307981}

Patch Set 1 #

Total comments: 38

Patch Set 2 : #

Total comments: 4

Patch Set 3 : #

Patch Set 4 : Move DRP event store logic to run on UI thread instead of IO thread #

Patch Set 5 : Forgot some IO -> UI method renames #

Patch Set 6 : #

Patch Set 7 : #

Total comments: 30

Patch Set 8 : mmenke CR updates #

Patch Set 9 : #

Patch Set 10 : #

Total comments: 26

Patch Set 11 : Latest CR feedback #

Patch Set 12 : Rebase #

Total comments: 43

Patch Set 13 : bengr CR updates #

Patch Set 14 : Fix test #

Patch Set 15 : More test fixes #

Patch Set 16 : #

Total comments: 18

Patch Set 17 : bengr CR updates #

Unified diffs Side-by-side diffs Delta from patch set Stats (+983 lines, -114 lines) Patch
M android_webview/browser/aw_browser_context.cc View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/profiles/profile_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/resources/net_internals/bandwidth_view.html View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +39 lines, -1 line 0 comments Download
M chrome/browser/resources/net_internals/bandwidth_view.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 5 chunks +387 lines, -4 lines 0 comments Download
M chrome/browser/resources/net_internals/browser_bridge.js View 1 2 3 4 5 6 7 4 chunks +23 lines, -0 lines 0 comments Download
M chrome/browser/resources/net_internals/main.js View 1 2 3 4 5 6 7 8 9 10 2 chunks +3 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/net_internals/net_internals_ui.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 6 chunks +24 lines, -0 lines 0 comments Download
M chrome/browser/ui/webui/net_internals/net_internals_ui_browsertest.cc View 1 5 chunks +18 lines, -0 lines 0 comments Download
M chrome/test/data/webui/net_internals/bandwidth_view.js View 1 2 3 4 5 6 7 8 9 10 5 chunks +123 lines, -2 lines 0 comments Download
M chrome/test/data/webui/net_internals/log_util.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +1 line, -1 line 0 comments Download
M components/data_reduction_proxy.gypi View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +1 line, -0 lines 0 comments Download
M components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +6 lines, -0 lines 0 comments Download
M components/data_reduction_proxy/core/common/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 1 chunk +1 line, -0 lines 0 comments Download
A components/data_reduction_proxy/core/common/data_reduction_proxy_bypass_type_list.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +49 lines, -0 lines 0 comments Download
M components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 4 chunks +75 lines, -16 lines 0 comments Download
M components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 11 chunks +216 lines, -49 lines 0 comments Download
M components/data_reduction_proxy/core/common/data_reduction_proxy_event_store_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 5 chunks +8 lines, -0 lines 0 comments Download
M components/data_reduction_proxy/core/common/data_reduction_proxy_headers.h View 1 2 3 4 5 6 7 8 9 10 11 12 1 chunk +7 lines, -39 lines 0 comments Download

Messages

Total messages: 26 (3 generated)
jeremyim
PTAL =) bengr -> components/data_reduction_proxy/* mef -> chrome_network_delegate.* mmenke -> profile_impl_io_data.cc, net_internals*
6 years ago (2014-12-02 21:46:33 UTC) #2
bengr
https://codereview.chromium.org/775773002/diff/1/chrome/browser/net/chrome_network_delegate.cc File chrome/browser/net/chrome_network_delegate.cc (right): https://codereview.chromium.org/775773002/diff/1/chrome/browser/net/chrome_network_delegate.cc#newcode382 chrome/browser/net/chrome_network_delegate.cc:382: base::DictionaryValue* proxy_info_dict = new base::DictionaryValue(); proxy_info_dict -> data_reduction_proxy_info https://codereview.chromium.org/775773002/diff/1/chrome/browser/net/chrome_network_delegate.cc#newcode385 ...
6 years ago (2014-12-02 23:55:33 UTC) #3
jeremyim
https://codereview.chromium.org/775773002/diff/1/chrome/browser/net/chrome_network_delegate.cc File chrome/browser/net/chrome_network_delegate.cc (right): https://codereview.chromium.org/775773002/diff/1/chrome/browser/net/chrome_network_delegate.cc#newcode382 chrome/browser/net/chrome_network_delegate.cc:382: base::DictionaryValue* proxy_info_dict = new base::DictionaryValue(); On 2014/12/02 23:55:32, bengr ...
6 years ago (2014-12-03 08:10:37 UTC) #4
mmenke
Sorry for slowness, haven't yet had the time to do a thorough review. https://codereview.chromium.org/775773002/diff/20001/chrome/browser/ui/webui/net_internals/net_internals_ui.cc File ...
6 years ago (2014-12-03 18:58:46 UTC) #5
jeremyim
https://codereview.chromium.org/775773002/diff/20001/chrome/browser/ui/webui/net_internals/net_internals_ui.cc File chrome/browser/ui/webui/net_internals/net_internals_ui.cc (right): https://codereview.chromium.org/775773002/diff/20001/chrome/browser/ui/webui/net_internals/net_internals_ui.cc#newcode1306 chrome/browser/ui/webui/net_internals/net_internals_ui.cc:1306: base::Value* data_reduction_proxy_info = NULL; On 2014/12/03 18:58:46, mmenke wrote: ...
6 years ago (2014-12-03 20:57:26 UTC) #6
mmenke
On 2014/12/03 20:57:26, jeremyim wrote: > https://codereview.chromium.org/775773002/diff/20001/chrome/browser/ui/webui/net_internals/net_internals_ui.cc > File chrome/browser/ui/webui/net_internals/net_internals_ui.cc (right): > > https://codereview.chromium.org/775773002/diff/20001/chrome/browser/ui/webui/net_internals/net_internals_ui.cc#newcode1306 > ...
6 years ago (2014-12-03 21:07:43 UTC) #7
jeremyim
On 2014/12/03 21:07:43, mmenke wrote: > On 2014/12/03 20:57:26, jeremyim wrote: > > > https://codereview.chromium.org/775773002/diff/20001/chrome/browser/ui/webui/net_internals/net_internals_ui.cc ...
6 years ago (2014-12-03 22:53:39 UTC) #8
jeremyim
Updated the DRP event store to run on the UI thread, instead of IO thread, ...
6 years ago (2014-12-04 23:55:16 UTC) #10
jeremyim
sgurun => android_webview/browser/aw_browser_context.cc
6 years ago (2014-12-04 23:56:01 UTC) #11
mmenke
https://codereview.chromium.org/775773002/diff/120001/chrome/browser/resources/net_internals/bandwidth_view.js File chrome/browser/resources/net_internals/bandwidth_view.js (right): https://codereview.chromium.org/775773002/diff/120001/chrome/browser/resources/net_internals/bandwidth_view.js#newcode88 chrome/browser/resources/net_internals/bandwidth_view.js:88: onDataReductionProxyInfoChanged: function(info) { I'm not going to review all ...
6 years ago (2014-12-05 16:18:25 UTC) #12
jeremyim
https://codereview.chromium.org/775773002/diff/120001/chrome/browser/resources/net_internals/bandwidth_view.js File chrome/browser/resources/net_internals/bandwidth_view.js (right): https://codereview.chromium.org/775773002/diff/120001/chrome/browser/resources/net_internals/bandwidth_view.js#newcode88 chrome/browser/resources/net_internals/bandwidth_view.js:88: onDataReductionProxyInfoChanged: function(info) { On 2014/12/05 16:18:25, mmenke wrote: > ...
6 years ago (2014-12-05 21:31:12 UTC) #13
mmenke
https://codereview.chromium.org/775773002/diff/180001/chrome/browser/resources/net_internals/bandwidth_view.js File chrome/browser/resources/net_internals/bandwidth_view.js (right): https://codereview.chromium.org/775773002/diff/180001/chrome/browser/resources/net_internals/bandwidth_view.js#newcode21 chrome/browser/resources/net_internals/bandwidth_view.js:21: // This map should match the DataReductionProxyBypassType enum. Sorry ...
6 years ago (2014-12-09 21:58:53 UTC) #14
jeremyim
https://codereview.chromium.org/775773002/diff/180001/chrome/browser/resources/net_internals/bandwidth_view.js File chrome/browser/resources/net_internals/bandwidth_view.js (right): https://codereview.chromium.org/775773002/diff/180001/chrome/browser/resources/net_internals/bandwidth_view.js#newcode21 chrome/browser/resources/net_internals/bandwidth_view.js:21: // This map should match the DataReductionProxyBypassType enum. On ...
6 years ago (2014-12-09 23:14:29 UTC) #15
mmenke
LGTM. I have not done an in-depth review of the 400 lines of js to ...
6 years ago (2014-12-10 21:09:43 UTC) #16
sgurun-gerrit only
On 2014/12/10 21:09:43, mmenke wrote: > LGTM. I have not done an in-depth review of ...
6 years ago (2014-12-10 22:17:51 UTC) #17
sgurun-gerrit only
On 2014/12/10 21:09:43, mmenke wrote: > LGTM. I have not done an in-depth review of ...
6 years ago (2014-12-10 22:17:52 UTC) #18
sgurun-gerrit only
On 2014/12/10 21:09:43, mmenke wrote: > LGTM. I have not done an in-depth review of ...
6 years ago (2014-12-10 22:17:55 UTC) #19
bengr
https://codereview.chromium.org/775773002/diff/220001/components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h File components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h (right): https://codereview.chromium.org/775773002/diff/220001/components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h#newcode19 components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h:19: #include "components/data_reduction_proxy/core/common/data_reduction_proxy_event_store.h" Why do you need the include now? ...
6 years ago (2014-12-11 00:16:07 UTC) #20
jeremyim
https://codereview.chromium.org/775773002/diff/220001/chrome/browser/resources/net_internals/bandwidth_view.js File chrome/browser/resources/net_internals/bandwidth_view.js (right): https://codereview.chromium.org/775773002/diff/220001/chrome/browser/resources/net_internals/bandwidth_view.js#newcode463 chrome/browser/resources/net_internals/bandwidth_view.js:463: return true; On 2014/12/10 21:09:43, mmenke wrote: > nit: ...
6 years ago (2014-12-11 05:24:23 UTC) #21
bengr
lgtm, with a bunch of nits. https://codereview.chromium.org/775773002/diff/300001/components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h File components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h (right): https://codereview.chromium.org/775773002/diff/300001/components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h#newcode184 components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h:184: DataReductionProxyEventStore* GetEventStore() const ...
6 years ago (2014-12-11 19:25:30 UTC) #22
jeremyim
Thanks everybody for looking at this! https://codereview.chromium.org/775773002/diff/300001/components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h File components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h (right): https://codereview.chromium.org/775773002/diff/300001/components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h#newcode184 components/data_reduction_proxy/core/browser/data_reduction_proxy_settings.h:184: DataReductionProxyEventStore* GetEventStore() const ...
6 years ago (2014-12-11 20:14:12 UTC) #24
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/775773002/320001
6 years ago (2014-12-11 20:15:00 UTC) #25
commit-bot: I haz the power
6 years ago (2014-12-11 23:08:51 UTC) #26
Message was sent while issue was closed.
Patchset 17 (id:??) landed as
https://crrev.com/29b3b28bcd3ea31391f4ff1b1edc43c18d788c98
Cr-Commit-Position: refs/heads/master@{#307981}

Powered by Google App Engine
This is Rietveld 408576698