Remove CHECKs from WebContentsImpl::OnDocumentLoadedInFrame

Remove CHECKs from WebContentsImpl::OnDocumentLoadedInFrame

BUG=451932
Committed: https://crrev.com/fbd033399d9beaa6a5b1abccba920baf7e52b1a5
Cr-Commit-Position: refs/heads/master@{#313208}

[nasko, 2015-01-27 00:06:05]

nasko@chromium.org changed reviewers:
+ creis@chromium.org

[nasko, 2015-01-27 00:06:06]

Hey Charlie,
Can you review this CL for me?
Thanks in advance!
Nasko

[Charlie Reis, 2015-01-27 00:18:03]

Thanks, that seems right.  Two comments below and we can land it.

https://codereview.chromium.org/883503003/diff/1/content/browser/web_contents...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/883503003/diff/1/content/browser/web_contents...
content/browser/web_contents/web_contents_impl.cc:2767:
RecordAction(base::UserMetricsAction("BadMessageTerminate_RVD2"));
Why RVD2?  I thought we usually abbreviated the class name that was triggering
this.  (I'm wondering if we should fix the case below as well or just leave
it...)

https://codereview.chromium.org/883503003/diff/1/content/browser/web_contents...
content/browser/web_contents/web_contents_impl.cc:2771: // happen.
I don't think this TODO is necessary.  "!render_frame_message_source_" implies
that render_view_message_source_ is valid (due to the DCHECK at the start of
OnMessageReceived), so the message would have come from the RVH.  That implies
that GetRenderProcessHost is the right thing to kill.

We can probably just drop the comment.

[nasko, 2015-01-27 01:29:36]

nasko@chromium.org changed reviewers:
+ isherman@chromium.org

[nasko, 2015-01-27 01:29:37]

Adding isherman@ for OWNER review of tools/metrics/actions/actions.xml

https://codereview.chromium.org/883503003/diff/1/content/browser/web_contents...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/883503003/diff/1/content/browser/web_contents...
content/browser/web_contents/web_contents_impl.cc:2767:
RecordAction(base::UserMetricsAction("BadMessageTerminate_RVD2"));
On 2015/01/27 00:18:03, Charlie Reis (slow until 1-29) wrote:
> Why RVD2?  I thought we usually abbreviated the class name that was triggering
> this.  (I'm wondering if we should fix the case below as well or just leave
> it...)

Fixed both. I forgot it on the first iteration.

https://codereview.chromium.org/883503003/diff/1/content/browser/web_contents...
content/browser/web_contents/web_contents_impl.cc:2771: // happen.
On 2015/01/27 00:18:03, Charlie Reis (slow until 1-29) wrote:
> I don't think this TODO is necessary.  "!render_frame_message_source_" implies
> that render_view_message_source_ is valid (due to the DCHECK at the start of
> OnMessageReceived), so the message would have come from the RVH.  That implies
> that GetRenderProcessHost is the right thing to kill.
> 
> We can probably just drop the comment.

Done.

[Charlie Reis, 2015-01-27 01:43:33]

LGTM

[Ilya Sherman, 2015-01-27 02:09:11]

LGTM, though I'd recommend using a histogram so that you have a built-in
baseline to compare against.

[nasko, 2015-01-27 02:15:28]

The CQ bit was checked by nasko@chromium.org

[commit-bot: I haz the power, 2015-01-27 02:16:46]

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

[commit-bot: I haz the power, 2015-01-27 02:35:21]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-01-27 02:36:33]

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