Add CHECK's to debug QuicStreamSequencerBuffer in weird state. Add a destruction indicator to detec…

Add CHECK's to debug QuicStreamSequencerBuffer in weird state. Add a destruction indicator to detect free after use.

Also remove some tests because CHECK's prevent some corner case from
happening.

R=rch@chromium.org
BUG=664200
Committed: https://crrev.com/0f23a5372fa884f61204f980fa057c168c6288eb
Cr-Commit-Position: refs/heads/master@{#434659}

[Ryan Hamilton, 2016-11-23 20:42:40]

Thanks for doing this. I'm guessing this is going to cause quite a few crashes
to be ready to roll it back :)

https://codereview.chromium.org/2519333006/diff/1/net/quic/core/quic_stream_s...
File net/quic/core/quic_stream_sequencer_buffer.cc (right):

https://codereview.chromium.org/2519333006/diff/1/net/quic/core/quic_stream_s...
net/quic/core/quic_stream_sequencer_buffer.cc:56: CHECK(blocks_count_ > 1) <<
"blocks_count_ = " << blocks_count_
nit: CHECK_GT()

https://codereview.chromium.org/2519333006/diff/1/net/quic/core/quic_stream_s...
net/quic/core/quic_stream_sequencer_buffer.cc:306:
base::debug::StackTrace().ToString().c_str());
I'm not sure that sending stack traces to google is something we're allowed to
do from a privacy perspective.

https://codereview.chromium.org/2519333006/diff/1/net/quic/core/quic_stream_s...
File net/quic/core/quic_stream_sequencer_buffer.h (right):

https://codereview.chromium.org/2519333006/diff/1/net/quic/core/quic_stream_s...
net/quic/core/quic_stream_sequencer_buffer.h:252: // in destructor. As long as
it's not 123456, this means use after free.
Either use after free, or memory corruption.

[danzh1, 2016-11-23 21:39:24]

PTAL

https://codereview.chromium.org/2519333006/diff/1/net/quic/core/quic_stream_s...
File net/quic/core/quic_stream_sequencer_buffer.cc (right):

https://codereview.chromium.org/2519333006/diff/1/net/quic/core/quic_stream_s...
net/quic/core/quic_stream_sequencer_buffer.cc:56: CHECK(blocks_count_ > 1) <<
"blocks_count_ = " << blocks_count_
On 2016/11/23 20:42:40, Ryan Hamilton wrote:
> nit: CHECK_GT()

Done.

https://codereview.chromium.org/2519333006/diff/1/net/quic/core/quic_stream_s...
net/quic/core/quic_stream_sequencer_buffer.cc:306:
base::debug::StackTrace().ToString().c_str());
On 2016/11/23 20:42:40, Ryan Hamilton wrote:
> I'm not sure that sending stack traces to google is something we're allowed to
> do from a privacy perspective.

removed as I switch to crashing right above.

https://codereview.chromium.org/2519333006/diff/1/net/quic/core/quic_stream_s...
File net/quic/core/quic_stream_sequencer_buffer.h (right):

https://codereview.chromium.org/2519333006/diff/1/net/quic/core/quic_stream_s...
net/quic/core/quic_stream_sequencer_buffer.h:252: // in destructor. As long as
it's not 123456, this means use after free.
On 2016/11/23 20:42:40, Ryan Hamilton wrote:
> Either use after free, or memory corruption.

Done.

[danzh1, 2016-11-23 21:39:31]

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

[commit-bot: I haz the power, 2016-11-23 21:40:07]

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

[Ryan Hamilton, 2016-11-23 21:54:39]

lgtm

[commit-bot: I haz the power, 2016-11-23 23:42:25]

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

[commit-bot: I haz the power, 2016-11-23 23:42:26]

Dry run: Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)

[danzh1, 2016-11-28 14:25:23]

The CQ bit was checked by danzh@chromium.org

[commit-bot: I haz the power, 2016-11-28 14:25:39]

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-28 15:20:41]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1480343123500590,
"parent_rev": "f06b2c83f28e3fc2bf110e651c0c52fb1a3504fd", "commit_rev":
"b89b0434cce300e61f948ed0e6c3cd8f89d6dd4b"}

[commit-bot: I haz the power, 2016-11-28 15:21:06]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-28 15:22:51]

Description was changed from

==========
Add CHECK's to debug QuicStreamSequencerBuffer in weird state. Add a destruction
indicator to detect free after use.

Also remove some tests because CHECK's prevent some corner case from
happening.

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

to

==========
Add CHECK's to debug QuicStreamSequencerBuffer in weird state. Add a destruction
indicator to detect free after use.

Also remove some tests because CHECK's prevent some corner case from
happening.

R=rch@chromium.org
BUG=664200

Committed: https://crrev.com/0f23a5372fa884f61204f980fa057c168c6288eb
Cr-Commit-Position: refs/heads/master@{#434659}
==========

[commit-bot: I haz the power, 2016-11-28 15:22:52]

Patchset 3 (id:??) landed as
https://crrev.com/0f23a5372fa884f61204f980fa057c168c6288eb
Cr-Commit-Position: refs/heads/master@{#434659}