Dispatch encoded_data_length separately in content/child

Dispatch encoded_data_length separately in content/child

This CL removes dispatching encoded_data_length from RequestPeer::ReceivedData. Instead,
it is dispatched via RequestPeer::OnTransferSizeUpdated.

BUG=666217
Committed: https://crrev.com/48f3e047d17e5f78543bb0bec1c421ba3a69cb79
Cr-Commit-Position: refs/heads/master@{#437433}

[yhirano, 2016-11-30 08:33:22]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-30 08:33:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-30 09:51:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-30 09:51:09]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[yhirano, 2016-11-30 12:22:23]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-30 12:22:32]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-30 13:44:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-30 13:44:26]

Dry run: This issue passed the CQ dry run.

[yhirano, 2016-12-06 04:28:09]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-06 04:28:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-06 06:18:18]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-06 06:18:20]

Dry run: 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_...)

[yhirano, 2016-12-06 06:53:26]

Description was changed from

==========
[DO NOT COMMIT] Dispatch encoded_data_length separately from didReceiveData /
OnReceivedData

wip, broken

BUG=666227
==========

to

==========
Dispatch encoded_data_length separately (1/2)

This CL removes dispatching encoded_data_length from RequestPeer::ReceivedData.
Instaed,
it is dispatched via RequestPeer::OnTransferSizeUpdated.

BUG=666227
==========

[yhirano, 2016-12-06 06:53:26]

yhirano@chromium.org changed reviewers:
+ tzik@chromium.org

[tzik, 2016-12-06 11:31:19]

lgtm

https://codereview.chromium.org/2540023003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/WebFrameTest.cpp (right):

https://codereview.chromium.org/2540023003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebFrameTest.cpp:10480: int dataLength) {
override?

[kinuko, 2016-12-07 04:06:26]

kinuko@chromium.org changed reviewers:
+ kinuko@chromium.org

[kinuko, 2016-12-07 04:06:27]

lgtm

https://codereview.chromium.org/2540023003/diff/40001/content/public/child/re...
File content/public/child/request_peer.h (right):

https://codereview.chromium.org/2540023003/diff/40001/content/public/child/re...
content/public/child/request_peer.h:88: virtual void OnTransferSizeUpdated(int
transfer_size_diff) = 0;
Should transfer_size_diff be unsigned if it must always be positive?

https://codereview.chromium.org/2540023003/diff/40001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/WebURLLoaderClient.h (right):

https://codereview.chromium.org/2540023003/diff/40001/third_party/WebKit/publ...
third_party/WebKit/public/platform/WebURLLoaderClient.h:85: // HTTP headers is
updated. |transferSizeDiff| is positive.
Should transferSizeDiff be unsigned if it's always positive?

[kinuko, 2016-12-07 04:11:53]

nit: 'Instaed' in the CL description -> Instead

[yhirano, 2016-12-07 12:02:29]

Description was changed from

==========
Dispatch encoded_data_length separately (1/2)

This CL removes dispatching encoded_data_length from RequestPeer::ReceivedData.
Instaed,
it is dispatched via RequestPeer::OnTransferSizeUpdated.

BUG=666227
==========

to

==========
Dispatch encoded_data_length separately in content/child

This CL removes dispatching encoded_data_length from RequestPeer::ReceivedData.
Instaed,
it is dispatched via RequestPeer::OnTransferSizeUpdated.

BUG=666227
==========

[yhirano, 2016-12-07 12:02:49]

Description was changed from

==========
Dispatch encoded_data_length separately in content/child

This CL removes dispatching encoded_data_length from RequestPeer::ReceivedData.
Instaed,
it is dispatched via RequestPeer::OnTransferSizeUpdated.

BUG=666227
==========

to

==========
Dispatch encoded_data_length separately in content/child

This CL removes dispatching encoded_data_length from RequestPeer::ReceivedData.
Instead,
it is dispatched via RequestPeer::OnTransferSizeUpdated.

BUG=666227
==========

[yhirano, 2016-12-07 12:05:19]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-07 12:05:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yhirano, 2016-12-07 12:06:58]

> nit: 'Instaed' in the CL description -> Instead

Done.

https://codereview.chromium.org/2540023003/diff/40001/content/public/child/re...
File content/public/child/request_peer.h (right):

https://codereview.chromium.org/2540023003/diff/40001/content/public/child/re...
content/public/child/request_peer.h:88: virtual void OnTransferSizeUpdated(int
transfer_size_diff) = 0;
On 2016/12/07 04:06:26, kinuko wrote:
> Should transfer_size_diff be unsigned if it must always be positive?

Google c++ style guide says 

"
You should not use the unsigned integer types such as uint32_t, unless there is
a valid reason such as representing a bit pattern rather than a number, or you
need defined overflow modulo 2^N. In particular, do not use unsigned types to
say a number will never be negative. Instead, use assertions for this.
"

https://google.github.io/styleguide/cppguide.html#Integer_Types

https://codereview.chromium.org/2540023003/diff/40001/third_party/WebKit/Sour...
File third_party/WebKit/Source/web/tests/WebFrameTest.cpp (right):

https://codereview.chromium.org/2540023003/diff/40001/third_party/WebKit/Sour...
third_party/WebKit/Source/web/tests/WebFrameTest.cpp:10480: int dataLength) {
On 2016/12/06 11:31:19, tzik wrote:
> override?

Done.

https://codereview.chromium.org/2540023003/diff/40001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/WebURLLoaderClient.h (right):

https://codereview.chromium.org/2540023003/diff/40001/third_party/WebKit/publ...
third_party/WebKit/public/platform/WebURLLoaderClient.h:85: // HTTP headers is
updated. |transferSizeDiff| is positive.
On 2016/12/07 04:06:26, kinuko wrote:
> Should transferSizeDiff be unsigned if it's always positive?

ditto

[yhirano, 2016-12-07 12:07:14]

yhirano@chromium.org changed reviewers:
+ jochen@chromium.org

[yhirano, 2016-12-07 12:07:29]

+jochen@ for chrome/ changes.

[yhirano, 2016-12-07 13:32:15]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-07 13:32:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-07 14:25:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-07 14:25:23]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[jochen (gone - plz use gerrit), 2016-12-07 15:38:37]

chrome/ lgtm

[yhirano, 2016-12-07 15:58:59]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-07 15:59:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yhirano, 2016-12-07 16:02:45]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-07 16:02:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yhirano, 2016-12-07 16:12:30]

+dgozman@ for layout test changes.

With this CL, WebURLLoader dispatches only positive encoded_data_length. I think
it's reasonable because dispatching 0 encoded_data_length is meaningless. But it
breaks inspector/tracing/timeline-network/timeline-network-received-data.html
because resources loaded in the test are fetched as file:// resources and hence
encoded_data_length is always zero. I moved the test to http/tests/inspector to
fix the issue and move timeline-network-resource.js to share the file.

[yhirano, 2016-12-07 16:13:00]

yhirano@chromium.org changed reviewers:
+ dgozman@chromium.org

[yhirano, 2016-12-07 16:13:01]

+dgozman@ for layout test changes.

With this CL, WebURLLoader dispatches only positive encoded_data_length. I think
it's reasonable because dispatching 0 encoded_data_length is meaningless. But it
breaks inspector/tracing/timeline-network/timeline-network-received-data.html
because resources loaded in the test are fetched as file:// resources and hence
encoded_data_length is always zero. I moved the test to http/tests/inspector to
fix the issue and move timeline-network-resource.js to share the file.

[commit-bot: I haz the power, 2016-12-07 17:51:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-07 17:51:18]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[dgozman, 2016-12-07 19:43:49]

> With this CL, WebURLLoader dispatches only positive encoded_data_length. I
think
> it's reasonable because dispatching 0 encoded_data_length is meaningless. But
it
> breaks inspector/tracing/timeline-network/timeline-network-received-data.html
> because resources loaded in the test are fetched as file:// resources and
hence
> encoded_data_length is always zero. I moved the test to http/tests/inspector
to
> fix the issue and move timeline-network-resource.js to share the file.

Sounds reasonable.

https://codereview.chromium.org/2540023003/diff/120001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/TestExpectations (right):

https://codereview.chromium.org/2540023003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/TestExpectations:1625: crbug.com/669357
virtual/mojo-loading/http/tests/inspector/tracing/timeline-network-received-data.html
[ Failure ]
Why does this still fail?

https://codereview.chromium.org/2540023003/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/inspector/tracing/timeline-misc/timeline-receive-response-event-expected.txt
(left):

https://codereview.chromium.org/2540023003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/tracing/timeline-misc/timeline-receive-response-event-expected.txt:5:
ResourceReceivedData
Does the same happen here? Let's move it to http as well?

[yhirano, 2016-12-08 02:23:44]

The CQ bit was checked by yhirano@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-08 02:24:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[yhirano, 2016-12-08 02:24:47]

Description was changed from

==========
Dispatch encoded_data_length separately in content/child

This CL removes dispatching encoded_data_length from RequestPeer::ReceivedData.
Instead,
it is dispatched via RequestPeer::OnTransferSizeUpdated.

BUG=666227
==========

to

==========
Dispatch encoded_data_length separately in content/child

This CL removes dispatching encoded_data_length from RequestPeer::ReceivedData.
Instead,
it is dispatched via RequestPeer::OnTransferSizeUpdated.

BUG=666217
==========

[commit-bot: I haz the power, 2016-12-08 03:55:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-08 03:55:43]

Dry run: 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_...)

[yhirano, 2016-12-08 13:38:42]

https://codereview.chromium.org/2540023003/diff/120001/third_party/WebKit/Lay...
File third_party/WebKit/LayoutTests/TestExpectations (right):

https://codereview.chromium.org/2540023003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/TestExpectations:1625: crbug.com/669357
virtual/mojo-loading/http/tests/inspector/tracing/timeline-network-received-data.html
[ Failure ]
On 2016/12/07 19:43:49, dgozman wrote:
> Why does this still fail?

This CL has no behavior change on the mojo-loading implementation. I added the
entry because the test is now running under virtual/mojo-loading settings.

https://codereview.chromium.org/2540023003/diff/120001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/inspector/tracing/timeline-misc/timeline-receive-response-event-expected.txt
(left):

https://codereview.chromium.org/2540023003/diff/120001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/inspector/tracing/timeline-misc/timeline-receive-response-event-expected.txt:5:
ResourceReceivedData
On 2016/12/07 19:43:49, dgozman wrote:
> Does the same happen here? Let's move it to http as well?

Done.

[dgozman, 2016-12-08 19:37:53]

lgtm

https://codereview.chromium.org/2540023003/diff/140001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/inspector/tracing/timeline-receive-response-event-expected.txt
(right):

https://codereview.chromium.org/2540023003/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/inspector/tracing/timeline-receive-response-event-expected.txt:6:
ResourceReceivedData
Why there are two now?

[yhirano, 2016-12-09 02:02:54]

https://codereview.chromium.org/2540023003/diff/140001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/http/tests/inspector/tracing/timeline-receive-response-event-expected.txt
(right):

https://codereview.chromium.org/2540023003/diff/140001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/http/tests/inspector/tracing/timeline-receive-response-event-expected.txt:6:
ResourceReceivedData
On 2016/12/08 19:37:53, dgozman wrote:
> Why there are two now?

Because the resource is fetched from the network now. If this change causes
flakiness, I will fix the test later.

[yhirano, 2016-12-09 02:03:01]

The CQ bit was checked by yhirano@chromium.org

[yhirano, 2016-12-09 02:03:02]

The patchset sent to the CQ was uploaded after l-g-t-m from kinuko@chromium.org,
tzik@chromium.org, jochen@chromium.org
Link to the patchset: https://codereview.chromium.org/2540023003/#ps140001
(title: "fix")

[commit-bot: I haz the power, 2016-12-09 02:03:56]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-09 02:19:21]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1481248981725280,
"parent_rev": "6f7d387fbdbb8f11860184ae6a40bf4e6c5533ac", "commit_rev":
"5fd8e27bc6621e766d6f6dbcd5cea902945b1c1e"}

[commit-bot: I haz the power, 2016-12-09 02:19:46]

Description was changed from

==========
Dispatch encoded_data_length separately in content/child

This CL removes dispatching encoded_data_length from RequestPeer::ReceivedData.
Instead,
it is dispatched via RequestPeer::OnTransferSizeUpdated.

BUG=666217
==========

to

==========
Dispatch encoded_data_length separately in content/child

This CL removes dispatching encoded_data_length from RequestPeer::ReceivedData.
Instead,
it is dispatched via RequestPeer::OnTransferSizeUpdated.

BUG=666217
==========

[commit-bot: I haz the power, 2016-12-09 02:19:47]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-12-09 02:22:58]

Description was changed from

==========
Dispatch encoded_data_length separately in content/child

This CL removes dispatching encoded_data_length from RequestPeer::ReceivedData.
Instead,
it is dispatched via RequestPeer::OnTransferSizeUpdated.

BUG=666217
==========

to

==========
Dispatch encoded_data_length separately in content/child

This CL removes dispatching encoded_data_length from RequestPeer::ReceivedData.
Instead,
it is dispatched via RequestPeer::OnTransferSizeUpdated.

BUG=666217

Committed: https://crrev.com/48f3e047d17e5f78543bb0bec1c421ba3a69cb79
Cr-Commit-Position: refs/heads/master@{#437433}
==========

[commit-bot: I haz the power, 2016-12-09 02:22:59]

Patchset 8 (id:??) landed as
https://crrev.com/48f3e047d17e5f78543bb0bec1c421ba3a69cb79
Cr-Commit-Position: refs/heads/master@{#437433}