BlobDispatcherHost: don't rely on NOTREACHED() checks in ParamTraits::Read

BlobDispatcherHost: don't rely on NOTREACHED() checks in ParamTraits::Read

Explicitly handle all possible storage::DataElement cases, and add comments
explaining the purpose of existing security checks.

BUG=644966
TEST=content_browsertests

Committed: https://crrev.com/87d0b2931a2ab2c4a50d220a51e36aea9a351d44
Cr-Commit-Position: refs/heads/master@{#423621}

[ncarter (slow), 2016-09-28 20:20:24]

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

[commit-bot: I haz the power, 2016-09-28 20:20:46]

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

[ncarter (slow), 2016-09-28 20:22:01]

nick@chromium.org changed reviewers:
+ dcheng@chromium.org

[ncarter (slow), 2016-09-28 20:22:01]

dcheng, PTAL

[commit-bot: I haz the power, 2016-09-28 21:03:33]

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

[commit-bot: I haz the power, 2016-09-28 21:03:34]

Dry run: This issue passed the CQ dry run.

[dcheng, 2016-09-28 23:46:37]

https://codereview.chromium.org/2378253002/diff/1/content/browser/blob_storag...
File content/browser/blob_storage/blob_dispatcher_host.cc (right):

https://codereview.chromium.org/2378253002/diff/1/content/browser/blob_storag...
content/browser/blob_storage/blob_dispatcher_host.cc:190: CHECK(false);  //
Should have been caught by IPC deserialization.
I'm uncertain how I feel about CHECKing this. This is something that the
ParamTraits catches, since we're in an IPC handler, so why not just DCHECK here?

[ncarter (slow), 2016-10-05 23:18:46]

ptal

https://codereview.chromium.org/2378253002/diff/1/content/browser/blob_storag...
File content/browser/blob_storage/blob_dispatcher_host.cc (right):

https://codereview.chromium.org/2378253002/diff/1/content/browser/blob_storag...
content/browser/blob_storage/blob_dispatcher_host.cc:190: CHECK(false);  //
Should have been caught by IPC deserialization.
On 2016/09/28 23:46:36, dcheng wrote:
> I'm uncertain how I feel about CHECKing this. This is something that the
> ParamTraits catches, since we're in an IPC handler, so why not just DCHECK
here?

Done.

[dcheng, 2016-10-05 23:38:53]

lgtm

[ncarter (slow), 2016-10-06 18:12:13]

The CQ bit was checked by nick@chromium.org

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

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-10-06 19:08:35]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-06 19:11:32]

Description was changed from

==========
BlobDispatcherHost: don't rely on NOTREACHED() checks in ParamTraits::Read

Explicitly handle all possible storage::DataElement cases, and add comments
explaining the purpose of existing security checks.

BUG=644966
TEST=content_browsertests
==========

to

==========
BlobDispatcherHost: don't rely on NOTREACHED() checks in ParamTraits::Read

Explicitly handle all possible storage::DataElement cases, and add comments
explaining the purpose of existing security checks.

BUG=644966
TEST=content_browsertests

Committed: https://crrev.com/87d0b2931a2ab2c4a50d220a51e36aea9a351d44
Cr-Commit-Position: refs/heads/master@{#423621}
==========

[commit-bot: I haz the power, 2016-10-06 19:11:34]

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