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

Issue 23696002: Propagate the encoded data length through OnDownloadData delegate method (Closed)

Created:
7 years, 3 months ago by yusukesuzuki
Modified:
7 years, 3 months ago
CC:
chromium-reviews, extensions-reviews_chromium.org, jam, joi+watch-content_chromium.org, feature-media-reviews_chromium.org, darin-cc_chromium.org, chromium-apps-reviews_chromium.org, pfeldman
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Visibility:
Public.

Description

Propagate the encoded data length through OnDownloadData delegate method Since the encoded data length is intended to be used for the Inspector, it will only be valid while devtools are attached. Otherwise it becomes -1, -1 represents that the encoded data length is unknown. To propagate the encoded data length to Blink's didDownloadData delegate method, the IPC dispatcher provides it to Blink. Blink-side CL: https://chromiumcodereview.appspot.com/23632004/ After this change is landed, we'll remove the stub code in Blink-side. BUG=269055 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=221024

Patch Set 1 #

Total comments: 7

Patch Set 2 : #

Total comments: 2

Patch Set 3 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+37 lines, -21 lines) Patch
M chrome/renderer/extensions/extension_localization_peer.h View 1 chunk +1 line, -1 line 0 comments Download
M chrome/renderer/extensions/extension_localization_peer_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/renderer/security_filter_peer.h View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/loader/async_resource_handler.cc View 1 chunk +4 lines, -1 line 0 comments Download
M content/child/resource_dispatcher.h View 1 chunk +2 lines, -1 line 0 comments Download
M content/child/resource_dispatcher.cc View 2 chunks +3 lines, -2 lines 0 comments Download
M content/child/resource_dispatcher_unittest.cc View 3 chunks +3 lines, -3 lines 0 comments Download
M content/common/resource_messages.h View 1 chunk +3 lines, -2 lines 0 comments Download
M content/renderer/media/buffered_resource_loader.h View 1 chunk +2 lines, -1 line 0 comments Download
M content/renderer/media/buffered_resource_loader.cc View 1 chunk +2 lines, -1 line 0 comments Download
M content/renderer/pepper/pepper_url_loader_host.h View 1 chunk +2 lines, -1 line 0 comments Download
M content/renderer/pepper/pepper_url_loader_host.cc View 1 chunk +2 lines, -1 line 0 comments Download
M webkit/child/resource_loader_bridge.h View 1 2 1 chunk +7 lines, -2 lines 0 comments Download
M webkit/child/weburlloader_impl.cc View 2 chunks +4 lines, -3 lines 0 comments Download

Messages

Total messages: 19 (0 generated)
yusukesuzuki
7 years, 3 months ago (2013-08-28 10:33:31 UTC) #1
Avi (use Gerrit)
I don't know if AsyncResourceHandler::OnDataDownloaded is correct (out of my expertise; ask someone else) but ...
7 years, 3 months ago (2013-08-28 14:27:19 UTC) #2
yusukesuzuki
On 2013/08/28 14:27:19, Avi wrote: > I don't know if AsyncResourceHandler::OnDataDownloaded is correct (out of ...
7 years, 3 months ago (2013-08-29 02:12:28 UTC) #3
kinuko
I'm probably not a good reviewer for this either, but anyway lgtm
7 years, 3 months ago (2013-08-29 06:03:42 UTC) #4
kinuko
or just would like to defer to michaeln@ or someone who knows this part better
7 years, 3 months ago (2013-08-29 06:04:39 UTC) #5
yusukesuzuki
On 2013/08/29 06:04:39, kinuko wrote: > or just would like to defer to michaeln@ or ...
7 years, 3 months ago (2013-08-29 06:24:00 UTC) #6
Nico
../../webkit/child/weburlloader_impl.cc:676:44:error: too many arguments to function call, expected 2, have 3 client_->didDownloadData(loader_, len, encoded_data_length); ~~~~~~~~~~~~~~~~~~~~~~~~ ...
7 years, 3 months ago (2013-08-29 14:13:35 UTC) #7
michaeln
pending green'ness lgtm https://codereview.chromium.org/23696002/diff/1/content/browser/loader/async_resource_handler.cc File content/browser/loader/async_resource_handler.cc (right): https://codereview.chromium.org/23696002/diff/1/content/browser/loader/async_resource_handler.cc#newcode288 content/browser/loader/async_resource_handler.cc:288: DevToolsNetLogObserver::GetAndResetEncodedDataLength(request_); I'm not familiar with the ...
7 years, 3 months ago (2013-08-29 20:19:04 UTC) #8
yusukesuzuki
On 2013/08/29 14:13:35, Nico wrote: > ../../webkit/child/weburlloader_impl.cc:676:44:error: too many arguments to > function call, expected ...
7 years, 3 months ago (2013-08-30 01:08:10 UTC) #9
yusukesuzuki
https://chromiumcodereview.appspot.com/23696002/diff/1/webkit/child/weburlloader_impl.cc File webkit/child/weburlloader_impl.cc (right): https://chromiumcodereview.appspot.com/23696002/diff/1/webkit/child/weburlloader_impl.cc#newcode676 webkit/child/weburlloader_impl.cc:676: client_->didDownloadData(loader_, len, encoded_data_length); On 2013/08/29 20:19:05, michaeln wrote: > ...
7 years, 3 months ago (2013-08-30 01:13:23 UTC) #10
yusukesuzuki
Blink is rolled and the patch becomes green.
7 years, 3 months ago (2013-08-30 06:17:55 UTC) #11
Nico
https://chromiumcodereview.appspot.com/23696002/diff/1/content/browser/loader/async_resource_handler.cc File content/browser/loader/async_resource_handler.cc (right): https://chromiumcodereview.appspot.com/23696002/diff/1/content/browser/loader/async_resource_handler.cc#newcode288 content/browser/loader/async_resource_handler.cc:288: DevToolsNetLogObserver::GetAndResetEncodedDataLength(request_); On 2013/08/29 20:19:05, michaeln wrote: > I'm not ...
7 years, 3 months ago (2013-08-31 20:49:46 UTC) #12
yusukesuzuki
https://chromiumcodereview.appspot.com/23696002/diff/1/content/browser/loader/async_resource_handler.cc File content/browser/loader/async_resource_handler.cc (right): https://chromiumcodereview.appspot.com/23696002/diff/1/content/browser/loader/async_resource_handler.cc#newcode288 content/browser/loader/async_resource_handler.cc:288: DevToolsNetLogObserver::GetAndResetEncodedDataLength(request_); On 2013/08/31 20:49:47, Nico wrote: > On 2013/08/29 ...
7 years, 3 months ago (2013-09-02 02:05:09 UTC) #13
Nico
https://chromiumcodereview.appspot.com/23696002/diff/23001/webkit/child/resource_loader_bridge.h File webkit/child/resource_loader_bridge.h (right): https://chromiumcodereview.appspot.com/23696002/diff/23001/webkit/child/resource_loader_bridge.h#newcode158 webkit/child/resource_loader_bridge.h:158: // gzipped content), or -1 if if unknown. s/if ...
7 years, 3 months ago (2013-09-02 03:50:25 UTC) #14
yusukesuzuki
https://chromiumcodereview.appspot.com/23696002/diff/23001/webkit/child/resource_loader_bridge.h File webkit/child/resource_loader_bridge.h (right): https://chromiumcodereview.appspot.com/23696002/diff/23001/webkit/child/resource_loader_bridge.h#newcode158 webkit/child/resource_loader_bridge.h:158: // gzipped content), or -1 if if unknown. On ...
7 years, 3 months ago (2013-09-02 04:10:14 UTC) #15
yusukesuzuki
7 years, 3 months ago (2013-09-02 04:40:09 UTC) #16
Tom Sepez
Messages LGTM
7 years, 3 months ago (2013-09-03 17:04:26 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/yusukesuzuki@chromium.org/23696002/28001
7 years, 3 months ago (2013-09-03 17:47:31 UTC) #18
commit-bot: I haz the power
7 years, 3 months ago (2013-09-03 21:18:49 UTC) #19
Message was sent while issue was closed.
Change committed as 221024

Powered by Google App Engine
This is Rietveld 408576698