content: Change DCHECKs to CHECKs in resource_buffer.cc.

content: Change DCHECKs to CHECKs in resource_buffer.cc.

I have reason to believe that these assertions are being violated in release
builds. See the associated crbug for more details.

BUG=527588
Committed: https://crrev.com/497946f7be738258e0aaaa14dd279130c6d06393
Cr-Commit-Position: refs/heads/master@{#347094}

[erikchen, 2015-09-02 23:22:54]

erikchen@chromium.org changed reviewers:
+ avi@chromium.org

[erikchen, 2015-09-02 23:22:54]

avi: Please review.

[erikchen, 2015-09-03 00:11:17]

erikchen@chromium.org changed reviewers:
+ mmenke@chromium.org
- avi@chromium.org

[erikchen, 2015-09-03 00:11:17]

-avi, +mmenke as a more specialized OWNER.

[Avi (use Gerrit), 2015-09-03 00:36:45]

On 2015/09/03 00:11:17, erikchen wrote:
> -avi, +mmenke as a more specialized OWNER.

I'm not clear from the bug. The browser process is sending bogus info? How will
switching to CHECKs help you figure out where the fault is?

[erikchen, 2015-09-03 00:46:18]

On 2015/09/03 00:36:45, Avi wrote:
> On 2015/09/03 00:11:17, erikchen wrote:
> > -avi, +mmenke as a more specialized OWNER.
> 
> I'm not clear from the bug. The browser process is sending bogus info? How
will
> switching to CHECKs help you figure out where the fault is?

AsyncResourceHandler::OnReadCompleted() calls:

""" 
int data_offset = buffer_->GetLastAllocationOffset();
...
filter->Send(new ResourceMsg_DataReceived(
      GetRequestID(), data_offset, bytes_read, encoded_data_length));
"""

Right now, this hits a CHECK in the receiving process, which sees that
data_offset > 512k. The possibilities are: an error in the IPC stack, or the
variable passed in is larger than 512k. After spending a lot of time looking at
this, I suspect that the latter is happening. 

I read through the code in resource_buffer.cc, and its correct, but relies on
the DCHECKs to maintain correctness. In a release build, a caller can corrupt
the internal state of resource_buffer.cc in several different ways. The easiest
way to get non-deterministic behavior is to call
buffer_->ShrinkLastAllocation(bytes_read); with bytes_read > allocation_size_
but there are other possibilities as well. This CL will catch the the first
corruption introduced to resource_buffer.cc, which will give us a hint about the
where the code is broken.

[Avi (use Gerrit), 2015-09-03 00:47:59]

On 2015/09/03 00:46:18, erikchen wrote:
> On 2015/09/03 00:36:45, Avi wrote:
> > On 2015/09/03 00:11:17, erikchen wrote:
> > > -avi, +mmenke as a more specialized OWNER.
> > 
> > I'm not clear from the bug. The browser process is sending bogus info? How
> will
> > switching to CHECKs help you figure out where the fault is?
> 
> AsyncResourceHandler::OnReadCompleted() calls:
> 
> """ 
> int data_offset = buffer_->GetLastAllocationOffset();
> ...
> filter->Send(new ResourceMsg_DataReceived(
>       GetRequestID(), data_offset, bytes_read, encoded_data_length));
> """
> 
> Right now, this hits a CHECK in the receiving process, which sees that
> data_offset > 512k. The possibilities are: an error in the IPC stack, or the
> variable passed in is larger than 512k. After spending a lot of time looking
at
> this, I suspect that the latter is happening. 
> 
> I read through the code in resource_buffer.cc, and its correct, but relies on
> the DCHECKs to maintain correctness. In a release build, a caller can corrupt
> the internal state of resource_buffer.cc in several different ways. The
easiest
> way to get non-deterministic behavior is to call
> buffer_->ShrinkLastAllocation(bytes_read); with bytes_read > allocation_size_
> but there are other possibilities as well. This CL will catch the the first
> corruption introduced to resource_buffer.cc, which will give us a hint about
the
> where the code is broken.

OK. If it helps us figure out real fixes, lgtm.

[erikchen, 2015-09-03 00:49:31]

The CQ bit was checked by erikchen@chromium.org

[commit-bot: I haz the power, 2015-09-03 00:50:11]

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

[erikchen, 2015-09-03 01:59:02]

On 2015/09/03 00:50:11, commit-bot: I haz the power wrote:
> CQ is trying da patch. Follow status at
>  https://chromium-cq-status.appspot.com/patch-status/1325153002/20001
> View timeline at
>  https://chromium-cq-status.appspot.com/patch-timeline/1325153002/20001

Hmm, perhaps I should be using DumpWithoutCrashing() so as to not take down the
browser process.

[commit-bot: I haz the power, 2015-09-03 02:07:50]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-09-03 02:08:33]

Patchset 2 (id:??) landed as
https://crrev.com/497946f7be738258e0aaaa14dd279130c6d06393
Cr-Commit-Position: refs/heads/master@{#347094}