Landing Recent QUIC changes until and including Mon Nov 16 14:15:48 2015

Landing Recent QUIC changes until and including Mon Nov 16 14:15:48 2015

Deprecate FLAGS_quic_read_packets_full_recvmmsg.

Merge internal change: 107932700
https://codereview.chromium.org/1467453007/

removing a redundant way of checking if QUIC has pending crypto data.

Merge internal change: 107932405

Make QuicPacketCreator be able to serialize packet itself when it does not have room for next stream frame. No functional change expected.

For QuicPacketCreator, add DelegateInterface class, Flush function, move SerializePacket from public to private.
For QuicPacketGenerator, add OnSerializedPacket function, remove SerializeAndSendPacket.

Merge internal change: 107814957
https://codereview.chromium.org/1459343009/

These packets got reformatted at some point, maybe before the clang-format off annotation was added.

Merge internal change: 107790487

Get rid of unnecessary CompareSpdyHeaderBlocks method, use
operator== instead. No behavior change.

Merge internal change: 107784612

minor changes to batched write to make DCHECK happy.  Behind FLAGS_quic_batch_writes

Also includes a test fix to update bytes on the correct stream to get the no-op
we intended (rather than a no-op we didn't intend)

Merge internal change: 107783952
https://codereview.chromium.org/1472573002/

Deprecate FLAG_reset_cubic_epoch_when_app_limited.

Merge internal change: 107711408
https://codereview.chromium.org/1467453006/

Add ConsumeData method to QuicPacketCreator, ConsumeData wraps both CreateStreamFrame and AddFrame. No functional change expected.

Also move CreateStreamFrame from public to private in QuicPacketCreator.

Merge internal change: 107709890
https://codereview.chromium.org/1464153002/

Add WritePushPromise() method in QuicHeadersStream for server push.

Merge internal change: 107699435
https://codereview.chromium.org/1471583002/

Clear the connection's send alarm when a new ack is received.  Flag protected by FLAGS_quic_respect_send_alarm.

Merge internal change: 107697347
https://codereview.chromium.org/1466153002/

Remove the LOG(DFATAL) from QuicTestClient::address(), since it appears to be implemented.

Changing from QuicPriority to SpdyPriority.

Merge internal change: 107687942
https://codereview.chromium.org/1468773002/

Replacing EffectivePriority with Priority for QUIC.

The only place we had changing priority was "if the response headers were
serialized and not sent"  After we moved to having a separate headers stream
we never utilized the QuicWriter's header buffer, instead passing
headers directly to the QUIC layer which serialized them and sent them with
their own special priority.

Removing the idea of mutable priority enables having the write blocked list
track stream priorty, removing SPDY-specific logic from our shared code and
moving us closer to the new H2 style write scheduler logic.

Merge internal change: 107610692
https://codereview.chromium.org/1471573002/

Let QUIC streams write 16k before ceding. Behind FLAGS_quic_batch_writes.

Changing our round robin logic to prompt a given stream until it has finished
~16k (rounding up to the max quic packet payload).

Merge internal change: 107595145
https://codereview.chromium.org/1472563002/

Removing unused code to convert QUIC from SPDY3 to SPDY4

Now that QUIC version 24 is gone, we shouldn't need to worry about converting
headers from SPDY3 to SPDY4 as it should all be SPDY4

Merge internal change: 107581674
https://codereview.chromium.org/1463303002/

Cleanup: clang-formatting gfe/quic/quic_framer* to comply with Chromium style guide.

 clang-format -i --style="{BasedOnStyle: Chromium, Standard: Cpp11}"
 ./quic_framer*

Merge internal change: 107514106
https://codereview.chromium.org/1466693002/

QUIC proof source: allow construction with an |SSLContext| provider.

To support |SSLContext| swaps, the long-lived |ProofSourceChromium|
object must have a means of retrieving the new context.  The simplest
way to do that seems to be to pass in a getter method: the alternative
of calling |SSLGlobalData::ssl_context| is unavailable due to circular
dependencies.

n/a (Refactor. No functional change expected.)

Merge internal change: 107509393
https://codereview.chromium.org/1463603002/

Convert QuicPathIdPacketNumber struct to std::pair. Not used in production, no behavior change.

Chromium doesn't have the same support for custom hash functions. Makes it hard to port cl/106938391.

Merge internal change: 107505890
https://codereview.chromium.org/1465583002/

Fix bug in ReliableQuicStream::WritevData() where if the write failed and the connection was closed, the stream would attempt to mark itself write blocked leading to a crash in debug mode.

Merge internal change: 107452913
https://codereview.chromium.org/1458003004/

Deprecate FLAGS_quic_count_unfinished_as_open_streams.

Merge internal change: 107422215
https://codereview.chromium.org/1457233003/

Convince ASAN we're really not touching the factory pointers at the same time by adding explicit locks

n/a (test only)

Merge internal change: 107412764
https://codereview.chromium.org/1459163002/

R=rch@chromium.org
BUG=

Committed: https://crrev.com/0888cffc5df1623d928740a4d3a264c16147823e
Cr-Commit-Position: refs/heads/master@{#361380}

[Ryan Hamilton, 2015-11-22 00:57:18]

Please update the CL description to match the format of the final merge CLs.
Also, please add the URLs for the individual chromium CLs.

There were a number of merge conflicts in a few individual CLs have those been
resolved?

[ianswett, 2015-11-23 15:40:42]

Description was changed from

==========
Deprecate FLAGS_quic_read_packets_full_recvmmsg.

Merge internal change: 107932700

removing a redundant way of checking if QUIC has pending crypto data.

Merge internal change: 107932405

Make QuicPacketCreator be able to serialize packet itself when it does not have
room for next stream frame. No functional change expected.

For QuicPacketCreator, add DelegateInterface class, Flush function, move
SerializePacket from public to private.
For QuicPacketGenerator, add OnSerializedPacket function, remove
SerializeAndSendPacket.

Merge internal change: 107814957

These packets got reformatted at some point, maybe before the clang-format off
annotation was added.

Merge internal change: 107790487

Get rid of unnecessary CompareSpdyHeaderBlocks method, use
operator== instead. No behavior change.

Merge internal change: 107784612

minor changes to batched write to make DCHECK happy.  Behind
gfe2_reloadable_flag_quic_batch_writes

Also includes a test fix to update bytes on the correct stream to get the no-op
we intended (rather than a no-op we didn't intend)

Merge internal change: 107783952

Deprecate gfe2_reloadable_flag_reset_cubic_epoch_when_app_limited.

Merge internal change: 107711408

Deprecate FLAG_reset_cubic_epoch_when_app_limited.

Merge internal change: 107711408

Add ConsumeData method to QuicPacketCreator, ConsumeData wraps both
CreateStreamFrame and AddFrame. No functional change expected.

Also move CreateStreamFrame from public to private in QuicPacketCreator.

Merge internal change: 107709890

Add WritePushPromise() method in QuicHeadersStream for server push.

Merge internal change: 107699435

Clear the connection's send alarm when a new ack is received.  Flag protected by
gfe2_reloadable_flag_quic_respect_send_alarm.

Merge internal change: 107697347

Remove the LOG(DFATAL) from QuicTestClient::address(), since it appears to be
implemented.


Changing from QuicPriority to SpdyPriority.

Merge internal change: 107687942

Replacing EffectivePriority with Priority for QUIC.

The only place we had changing priority was "if the response headers were
serialized and not sent"  After we moved to having a separate headers stream
we never utilized the QuicWriter's header buffer, instead passing
headers directly to the QUIC layer which serialized them and sent them with
their own special priority.

Removing the idea of mutable priority enables having the write blocked list
track stream priorty, removing SPDY-specific logic from our shared code and
moving us closer to the new H2 style write scheduler logic.

Merge internal change: 107610692

Let QUIC streams write 16k before ceding. Behind FLAG_quic_batch_writes.

Changing our round robin logic to prompt a given stream until it has finished
~16k (rounding up to the max quic packet payload).

Merge internal change: 107595145

Removing unused code to convert QUIC from SPDY3 to SPDY4

Now that QUIC version 24 is gone, we shouldn't need to worry about converting
headers from SPDY3 to SPDY4 as it should all be SPDY4

Merge internal change: 107581674

Cleanup: clang-formatting gfe/quic/quic_framer* to comply with Chromium style
guide.

 clang-format -i --style="{BasedOnStyle: Chromium, Standard: Cpp11}"
 ./quic_framer*

Merge internal change: 107514106

QUIC proof source: allow construction with an |SSLContext| provider.

To support |SSLContext| swaps, the long-lived |ProofSourceGoogle3|
object must have a means of retrieving the new context.  The simplest
way to do that seems to be to pass in a getter method: the alternative
of calling |SSLGlobalData::ssl_context| is unavailable due to circular
dependencies.

n/a (Refactor. No functional change expected.)

Merge internal change: 107509393

Convert QuicPathIdPacketNumber struct to std::pair. Not used in production, no
behavior change.

Chromium doesn't have the same support for custom hash functions. Makes it hard
to port cl/106938391.

Merge internal change: 107505890

Fix bug in ReliableQuicStream::WritevData() where if the write failed and the
connection was closed, the stream would attempt to mark itself write blocked
leading to a crash in debug mode.

Merge internal change: 107452913

Deprecate FLAGS_quic_count_unfinished_as_open_streams.

Merge internal change: 107422215

Convince ASAN we're really not touching the factory pointers at the same time by
adding explicit locks

n/a (test only)

Merge internal change: 107412764

R=rch@chromium.org
BUG=
==========

to

==========
Landing Recent QUIC changes until and including Mon Nov 16 14:15:48 2015

Deprecate FLAGS_quic_read_packets_full_recvmmsg.

Merge internal change: 107932700
https://codereview.chromium.org/1467453007/

removing a redundant way of checking if QUIC has pending crypto data.

Merge internal change: 107932405

Make QuicPacketCreator be able to serialize packet itself when it does not have
room for next stream frame. No functional change expected.

For QuicPacketCreator, add DelegateInterface class, Flush function, move
SerializePacket from public to private.
For QuicPacketGenerator, add OnSerializedPacket function, remove
SerializeAndSendPacket.

Merge internal change: 107814957
https://codereview.chromium.org/1459343009/

These packets got reformatted at some point, maybe before the clang-format off
annotation was added.

Merge internal change: 107790487

Get rid of unnecessary CompareSpdyHeaderBlocks method, use
operator== instead. No behavior change.

Merge internal change: 107784612

minor changes to batched write to make DCHECK happy.  Behind
FLAGS_quic_batch_writes

Also includes a test fix to update bytes on the correct stream to get the no-op
we intended (rather than a no-op we didn't intend)

Merge internal change: 107783952
https://codereview.chromium.org/1472573002/

Deprecate FLAG_reset_cubic_epoch_when_app_limited.

Merge internal change: 107711408
https://codereview.chromium.org/1467453006/

Add ConsumeData method to QuicPacketCreator, ConsumeData wraps both
CreateStreamFrame and AddFrame. No functional change expected.

Also move CreateStreamFrame from public to private in QuicPacketCreator.

Merge internal change: 107709890
https://codereview.chromium.org/1464153002/

Add WritePushPromise() method in QuicHeadersStream for server push.

Merge internal change: 107699435
https://codereview.chromium.org/1471583002/

Clear the connection's send alarm when a new ack is received.  Flag protected by
FLAGS_quic_respect_send_alarm.

Merge internal change: 107697347
https://codereview.chromium.org/1466153002/

Remove the LOG(DFATAL) from QuicTestClient::address(), since it appears to be
implemented.

Changing from QuicPriority to SpdyPriority.

Merge internal change: 107687942
https://codereview.chromium.org/1468773002/

Replacing EffectivePriority with Priority for QUIC.

The only place we had changing priority was "if the response headers were
serialized and not sent"  After we moved to having a separate headers stream
we never utilized the QuicWriter's header buffer, instead passing
headers directly to the QUIC layer which serialized them and sent them with
their own special priority.

Removing the idea of mutable priority enables having the write blocked list
track stream priorty, removing SPDY-specific logic from our shared code and
moving us closer to the new H2 style write scheduler logic.

Merge internal change: 107610692
https://codereview.chromium.org/1471573002/

Let QUIC streams write 16k before ceding. Behind FLAGS_quic_batch_writes.

Changing our round robin logic to prompt a given stream until it has finished
~16k (rounding up to the max quic packet payload).

Merge internal change: 107595145
https://codereview.chromium.org/1472563002/

Removing unused code to convert QUIC from SPDY3 to SPDY4

Now that QUIC version 24 is gone, we shouldn't need to worry about converting
headers from SPDY3 to SPDY4 as it should all be SPDY4

Merge internal change: 107581674
https://codereview.chromium.org/1463303002/

Cleanup: clang-formatting gfe/quic/quic_framer* to comply with Chromium style
guide.

 clang-format -i --style="{BasedOnStyle: Chromium, Standard: Cpp11}"
 ./quic_framer*

Merge internal change: 107514106
https://codereview.chromium.org/1466693002/

QUIC proof source: allow construction with an |SSLContext| provider.

To support |SSLContext| swaps, the long-lived |ProofSourceChromium|
object must have a means of retrieving the new context.  The simplest
way to do that seems to be to pass in a getter method: the alternative
of calling |SSLGlobalData::ssl_context| is unavailable due to circular
dependencies.

n/a (Refactor. No functional change expected.)

Merge internal change: 107509393
https://codereview.chromium.org/1463603002/

Convert QuicPathIdPacketNumber struct to std::pair. Not used in production, no
behavior change.

Chromium doesn't have the same support for custom hash functions. Makes it hard
to port cl/106938391.

Merge internal change: 107505890
https://codereview.chromium.org/1465583002/

Fix bug in ReliableQuicStream::WritevData() where if the write failed and the
connection was closed, the stream would attempt to mark itself write blocked
leading to a crash in debug mode.

Merge internal change: 107452913
https://codereview.chromium.org/1458003004/

Deprecate FLAGS_quic_count_unfinished_as_open_streams.

Merge internal change: 107422215
https://codereview.chromium.org/1457233003/

Convince ASAN we're really not touching the factory pointers at the same time by
adding explicit locks

n/a (test only)

Merge internal change: 107412764
https://codereview.chromium.org/1459163002/

R=rch@chromium.org
BUG=
==========

[ianswett, 2015-11-23 20:07:19]

I re-uploaded the full patch.  PTAL.

[Ryan Hamilton, 2015-11-23 20:30:08]

It looks like a few of the CL in the description are missing chromium CL links.

https://codereview.chromium.org/1470713003/diff/20001/net/quic/quic_connectio...
File net/quic/quic_connection_test.cc (right):

https://codereview.chromium.org/1470713003/diff/20001/net/quic/quic_connectio...
net/quic/quic_connection_test.cc:5272: .WillOnce(Return(PacketNumberSet()));
This wasn't part of the internal CL 107699435, was it? How come this test needed
to change?

[ianswett, 2015-11-23 20:38:45]

On 2015/11/23 20:30:08, Ryan Hamilton wrote:
> It looks like a few of the CL in the description are missing chromium CL
links.
> 
>
https://codereview.chromium.org/1470713003/diff/20001/net/quic/quic_connectio...
> File net/quic/quic_connection_test.cc (right):
> 
>
https://codereview.chromium.org/1470713003/diff/20001/net/quic/quic_connectio...
> net/quic/quic_connection_test.cc:5272: .WillOnce(Return(PacketNumberSet()));
> This wasn't part of the internal CL 107699435, was it? How come this test
needed
> to change?

Agreed, it will need to be part of the merge CL that feeds back upstream.  It
appears Chrome is much pickier about requiring return values for EXPECT calls?

[Ryan Hamilton, 2015-11-23 20:51:11]

(Still missing some chromium CL links)

https://codereview.chromium.org/1470713003/diff/20001/net/quic/quic_connectio...
File net/quic/quic_connection_test.cc (right):

https://codereview.chromium.org/1470713003/diff/20001/net/quic/quic_connectio...
net/quic/quic_connection_test.cc:5272: .WillOnce(Return(PacketNumberSet()));
On 2015/11/23 20:30:07, Ryan Hamilton wrote:
> This wasn't part of the internal CL 107699435, was it? How come this test
needed
> to change?

Do you understand why it changed as part of the CL in question, though? That CL
didn't touch anything in QuicConnection so I'm really surprised to see a broken
quic_connection_test. Could it be that it happened in an earlier CL?

[ianswett, 2015-11-23 21:27:50]

On 2015/11/23 20:51:11, Ryan Hamilton wrote:
> (Still missing some chromium CL links)
> 
I combined multiple merges in one Chromium CL for a few small changes, so these
should be all the links.

>
https://codereview.chromium.org/1470713003/diff/20001/net/quic/quic_connectio...
> File net/quic/quic_connection_test.cc (right):
> 
>
https://codereview.chromium.org/1470713003/diff/20001/net/quic/quic_connectio...
> net/quic/quic_connection_test.cc:5272: .WillOnce(Return(PacketNumberSet()));
> On 2015/11/23 20:30:07, Ryan Hamilton wrote:
> > This wasn't part of the internal CL 107699435, was it? How come this test
> needed
> > to change?
> 
> Do you understand why it changed as part of the CL in question, though? That
CL
> didn't touch anything in QuicConnection so I'm really surprised to see a
broken
> quic_connection_test. Could it be that it happened in an earlier CL?

The test was added as part of https://codereview.chromium.org/1466153002/, but I
ignored the test failure and moved to the next CL at the time, because I didn't
know how to fix it.  Then I disabled it in a subsequent CL, and then I fixed it
in Final for once and all.

[Ryan Hamilton, 2015-11-23 22:04:53]

lgtm

[Ryan Hamilton, 2015-11-23 22:04:59]

The CQ bit was checked by rch@chromium.org

[commit-bot: I haz the power, 2015-11-23 22:07:43]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1470713003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1470713003/20001

[commit-bot: I haz the power, 2015-11-23 22:31:16]

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

[commit-bot: I haz the power, 2015-11-23 22:31:17]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[ianswett, 2015-11-23 22:52:01]

The CQ bit was checked by ianswett@chromium.org

[ianswett, 2015-11-23 22:52:01]

The patchset sent to the CQ was uploaded after l-g-t-m from rch@chromium.org
Link to the patchset: https://codereview.chromium.org/1470713003/#ps60001
(title: "git cl format net")

[commit-bot: I haz the power, 2015-11-23 22:52:56]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1470713003/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1470713003/60001

[commit-bot: I haz the power, 2015-11-23 23:51:01]

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

[commit-bot: I haz the power, 2015-11-23 23:51:02]

Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Ryan Hamilton, 2015-11-24 00:34:15]

On 2015/11/23 23:51:02, commit-bot: I haz the power wrote:
> Try jobs failed on following builders:
>   win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED,
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

FAILED: ninja -t msvc -e environment.x64 -- E:\b\build\goma/gomacc
"E:\b\depot_tools\win_toolchain\vs2013_files\VC\bin\amd64\cl.exe" /nologo
/showIncludes /FC
@obj\components\update_client\update_client_test_support.test_configurator.obj.rsp
/c ..\..\components\update_client\test_configurator.cc
/Foobj\components\update_client\update_client_test_support.test_configurator.obj
/Fdobj\components\update_client_test_support.cc.pdb 
e:\b\build\slave\win\build\src\net\quic\quic_write_blocked_list.h(84) :error
C2220: warning treated as error - no 'object' file generated
e:\b\build\slave\win\build\src\net\quic\quic_write_blocked_list.h(84) : warning
C4267: '-=' : conversion from 'size_t' to 'int32', possible loss of data
FAILED: ninja -t msvc -e environment.x64 -- E:\b\build\goma/gomacc
"E:\b\depot_tools\win_toolchain\vs2013_files\VC\bin\amd64\cl.exe" /nologo
/showIncludes /FC @obj\device\test\device_unittests.usb_test_gadget_impl.obj.rsp
/c ..\..\device\test\usb_test_gadget_impl.cc
/Foobj\device\test\device_unittests.usb_test_gadget_impl.obj
/Fdobj\device\device_unittests.cc.pdb 
e:\b\build\slave\win\build\src\net\quic\quic_write_blocked_list.h(84) :error
C2220: warning treated as error - no 'object' file generated
e:\b\build\slave\win\build\src\net\quic\quic_write_blocked_list.h(84) : warning
C4267: '-=' : conversion from 'size_t' to 'int32', possible loss of data
FAILED: ninja -t msvc -e environment.x64 -- E:\b\build\goma/gomacc
"E:\b\depot_tools\win_toolchain\vs2013_files\VC\bin\amd64\cl.exe" /nologo
/showIncludes /FC
@obj\components\update_client\update_client_test_support.url_request_post_interceptor.obj.rsp
/c ..\..\components\update_client\url_request_post_interceptor.cc
/Foobj\components\update_client\update_client_test_support.url_request_post_interceptor.obj
/Fdobj\components\update_client_test_support.cc.pdb 
e:\b\build\slave\win\build\src\net\quic\quic_write_blocked_list.h(84) :error
C2220: warning treated as error - no 'object' file generated
e:\b\build\slave\win\build\src\net\quic\quic_write_blocked_list.h(84) : warning
C4267: '-=' : conversion from 'size_t' to 'int32', possible loss of data
ninja: build stopped: subcommand failed.

Looks like a size_t vs int32t issue in quic_write_blocked_list.h

  84       bytes_left_for_batch_write_[last_priority_popped_] -= bytes;

[ianswett, 2015-11-24 02:32:40]

On 2015/11/24 00:34:15, Ryan Hamilton wrote:
> On 2015/11/23 23:51:02, commit-bot: I haz the power wrote:
> > Try jobs failed on following builders:
> >   win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED,
> >
>
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
> 
> FAILED: ninja -t msvc -e environment.x64 -- E:\b\build\goma/gomacc
> "E:\b\depot_tools\win_toolchain\vs2013_files\VC\bin\amd64\cl.exe" /nologo
> /showIncludes /FC
>
@obj\components\update_client\update_client_test_support.test_configurator.obj.rsp
> /c ..\..\components\update_client\test_configurator.cc
>
/Foobj\components\update_client\update_client_test_support.test_configurator.obj
> /Fdobj\components\update_client_test_support.cc.pdb 
> e:\b\build\slave\win\build\src\net\quic\quic_write_blocked_list.h(84) :error
> C2220: warning treated as error - no 'object' file generated
> e:\b\build\slave\win\build\src\net\quic\quic_write_blocked_list.h(84) :
warning
> C4267: '-=' : conversion from 'size_t' to 'int32', possible loss of data
> FAILED: ninja -t msvc -e environment.x64 -- E:\b\build\goma/gomacc
> "E:\b\depot_tools\win_toolchain\vs2013_files\VC\bin\amd64\cl.exe" /nologo
> /showIncludes /FC
@obj\device\test\device_unittests.usb_test_gadget_impl.obj.rsp
> /c ..\..\device\test\usb_test_gadget_impl.cc
> /Foobj\device\test\device_unittests.usb_test_gadget_impl.obj
> /Fdobj\device\device_unittests.cc.pdb 
> e:\b\build\slave\win\build\src\net\quic\quic_write_blocked_list.h(84) :error
> C2220: warning treated as error - no 'object' file generated
> e:\b\build\slave\win\build\src\net\quic\quic_write_blocked_list.h(84) :
warning
> C4267: '-=' : conversion from 'size_t' to 'int32', possible loss of data
> FAILED: ninja -t msvc -e environment.x64 -- E:\b\build\goma/gomacc
> "E:\b\depot_tools\win_toolchain\vs2013_files\VC\bin\amd64\cl.exe" /nologo
> /showIncludes /FC
>
@obj\components\update_client\update_client_test_support.url_request_post_interceptor.obj.rsp
> /c ..\..\components\update_client\url_request_post_interceptor.cc
>
/Foobj\components\update_client\update_client_test_support.url_request_post_interceptor.obj
> /Fdobj\components\update_client_test_support.cc.pdb 
> e:\b\build\slave\win\build\src\net\quic\quic_write_blocked_list.h(84) :error
> C2220: warning treated as error - no 'object' file generated
> e:\b\build\slave\win\build\src\net\quic\quic_write_blocked_list.h(84) :
warning
> C4267: '-=' : conversion from 'size_t' to 'int32', possible loss of data
> ninja: build stopped: subcommand failed.
> 
> Looks like a size_t vs int32t issue in quic_write_blocked_list.h
> 
>   84       bytes_left_for_batch_write_[last_priority_popped_] -= bytes;

Thanks Ryan, cast added.

[ianswett, 2015-11-24 02:32:50]

The CQ bit was checked by ianswett@chromium.org

[ianswett, 2015-11-24 02:32:50]

The patchset sent to the CQ was uploaded after l-g-t-m from rch@chromium.org
Link to the patchset: https://codereview.chromium.org/1470713003/#ps80001
(title: "Fix Windows Build.")

[commit-bot: I haz the power, 2015-11-24 02:40:29]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1470713003/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1470713003/80001

[commit-bot: I haz the power, 2015-11-24 03:47:35]

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

[commit-bot: I haz the power, 2015-11-24 03:47:36]

Try jobs failed on following builders:
  win_chromium_compile_dbg_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[Ryan Hamilton, 2015-11-24 04:02:48]

https://codereview.chromium.org/1470713003/diff/80001/net/quic/quic_packet_cr...
File net/quic/quic_packet_creator.h (right):

https://codereview.chromium.org/1470713003/diff/80001/net/quic/quic_packet_cr...
net/quic/quic_packet_creator.h:33: class DelegateInterface {
Looks like you need NET_EXPORT_PRIVATE here.

[ianswett, 2015-11-24 15:58:59]

On 2015/11/24 04:02:48, Ryan Hamilton wrote:
>
https://codereview.chromium.org/1470713003/diff/80001/net/quic/quic_packet_cr...
> File net/quic/quic_packet_creator.h (right):
> 
>
https://codereview.chromium.org/1470713003/diff/80001/net/quic/quic_packet_cr...
> net/quic/quic_packet_creator.h:33: class DelegateInterface {
> Looks like you need NET_EXPORT_PRIVATE here.

Thanks, NET_EXPORT_PRIVATE added.

[ianswett, 2015-11-24 15:59:11]

The CQ bit was checked by ianswett@chromium.org

[ianswett, 2015-11-24 15:59:12]

The patchset sent to the CQ was uploaded after l-g-t-m from rch@chromium.org
Link to the patchset: https://codereview.chromium.org/1470713003/#ps100001
(title: "Adding NET_EXPORT_PRIVATE to DelegateInterface.")

[commit-bot: I haz the power, 2015-11-24 16:01:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1470713003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1470713003/100001

[commit-bot: I haz the power, 2015-11-24 17:42:25]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-11-24 17:43:20]

Patchset 6 (id:??) landed as
https://crrev.com/0888cffc5df1623d928740a4d3a264c16147823e
Cr-Commit-Position: refs/heads/master@{#361380}

[brucedawson, 2015-11-30 22:53:08]

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

[brucedawson, 2015-11-30 22:53:09]

https://codereview.chromium.org/1470713003/diff/100001/net/quic/quic_write_bl...
File net/quic/quic_write_blocked_list.cc (right):

https://codereview.chromium.org/1470713003/diff/100001/net/quic/quic_write_bl...
net/quic/quic_write_blocked_list.cc:13: memset(batch_write_stream_id_, 0,
This change is fragile and could be made simpler and more stable.

As it stands the two memset calls rely on knowledge of the type of the array
elements in order to verify their correctness. This dependency could be avoided
by replacing the explicit type with array[0]:

  memset(batch_write_stream_id_, 0,
           arraysize(batch_write_stream_id_) *
sizeof(batch_write_stream_id_[0]));

But even better would be this:

  memset(batch_write_stream_id_, 0, sizeof(batch_write_stream_id_));

Since arraysize divides by sizeof element and the lines of code are multiplying
by sizeof element it is simpler and safer to just use sizeof directly.

This was noticed because it triggered a VC++ /analyze warning about multiplying
sizeof() times sizeof() being suspicious.