Improve process crash handling in RenderViewHost & mock RenderProcessHost.

Improve process crash handling in RenderViewHost & mock RenderProcessHost.

Currently the mock RenderProcessHost always lies about having a connection
to the renderer process. This CL aims to make it more realistic by keeping
track of process initialization and crashing.

Additionally, RenderViewHost can directly monitor the RenderProcessHost
for crashes instead of relying on the RenderFrameHost to notify it.
The CL includes the decoupling of the two.

BUG=357747
Committed: https://crrev.com/954b02627a8c041cc5d3af2ae57593407bd4570b
Cr-Commit-Position: refs/heads/master@{#332462}

[nasko, 2015-05-29 13:45:52]

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

[nasko, 2015-05-29 13:45:52]

Hey Nick,
Can you review this CL for me? It cleans up a bit process crash handling and
makes the mock RPH be closer to the real one.
Thanks in advance!
Nasko

[ncarter (slow), 2015-05-29 17:31:31]

https://codereview.chromium.org/1159183002/diff/20001/ui/views/controls/webvi...
File ui/views/controls/webview/webview_interactive_uitest.cc (right):

https://codereview.chromium.org/1159183002/diff/20001/ui/views/controls/webvi...
ui/views/controls/webview/webview_interactive_uitest.cc:93:
web_contents1->GetRenderViewHost()->GetProcess()->Init();
web_contents1 is created via
[WebView::GetWebContents=>WebView::CreateWebContents=>WebViewTestViewsDelegate::CreateWebContents=>WebContentsTester::CreateTestWebContents=>TestWebContents::Create]

You need the RPH::Init call here because in test_renderer_host.cc you added an
RPH::Init call to RenderViewHostTestHarness::CreateTestWebContents and not to
TestWebContents::Create.

Would it be better to remove the RPH::Init call from
RenderViewHostTestHarness::CreateTestWebContents and add it to
TestWebContents::Create so that it's done automatically and consistently for
tests like this one?

https://codereview.chromium.org/1159183002/diff/20001/ui/views/controls/webvi...
ui/views/controls/webview/webview_interactive_uitest.cc:106:
web_contents2->GetRenderViewHost()->GetProcess()->Init();
Same question here.

[nasko, 2015-06-01 18:58:20]

Patchset #4 (id:60001) has been deleted

[nasko, 2015-06-01 19:00:09]

Unfortunately, the CL this builds on
(https://codereview.chromium.org/1151973005/) was reverted and needs to reland
for this to pass.

In the meantime, I've fixed Nick's comments.

https://codereview.chromium.org/1159183002/diff/20001/ui/views/controls/webvi...
File ui/views/controls/webview/webview_interactive_uitest.cc (right):

https://codereview.chromium.org/1159183002/diff/20001/ui/views/controls/webvi...
ui/views/controls/webview/webview_interactive_uitest.cc:93:
web_contents1->GetRenderViewHost()->GetProcess()->Init();
On 2015/05/29 17:31:31, ncarter wrote:
> web_contents1 is created via
>
[WebView::GetWebContents=>WebView::CreateWebContents=>WebViewTestViewsDelegate::CreateWebContents=>WebContentsTester::CreateTestWebContents=>TestWebContents::Create]
> 
> You need the RPH::Init call here because in test_renderer_host.cc you added an
> RPH::Init call to RenderViewHostTestHarness::CreateTestWebContents and not to
> TestWebContents::Create.
> 
> Would it be better to remove the RPH::Init call from
> RenderViewHostTestHarness::CreateTestWebContents and add it to
> TestWebContents::Create so that it's done automatically and consistently for
> tests like this one?

Indeed it is better. Done!

https://codereview.chromium.org/1159183002/diff/20001/ui/views/controls/webvi...
ui/views/controls/webview/webview_interactive_uitest.cc:106:
web_contents2->GetRenderViewHost()->GetProcess()->Init();
On 2015/05/29 17:31:31, ncarter wrote:
> Same question here.

Done.

[ncarter (slow), 2015-06-01 19:13:20]

lgtm

[nasko, 2015-06-02 17:53:22]

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

[nasko, 2015-06-02 17:53:23]

The patchset sent to the CQ was uploaded after l-g-t-m from nick@chromium.org
Link to the patchset: https://codereview.chromium.org/1159183002/#ps120001
(title: "Revert moving to TestWebContents::Create and fix Mac failures.")

[commit-bot: I haz the power, 2015-06-02 17:53:54]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1159183002/120001

[nasko, 2015-06-02 17:54:43]

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

[nasko, 2015-06-02 17:54:44]

Hey Nick,
As we discussed, I reverted to not using TestWebContents::Create and fixed up
the failing test cases.

Adding sadrul@ for OWNERS review of ui/views/controls/webview/.

[ncarter (slow), 2015-06-02 17:58:39]

lgtm

[nasko, 2015-06-02 18:15:51]

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

[nasko, 2015-06-02 18:15:52]

The patchset sent to the CQ was uploaded after l-g-t-m from nick@chromium.org
Link to the patchset: https://codereview.chromium.org/1159183002/#ps140001
(title: "Fix compile problem in webview_interactive_uitest.cc")

[commit-bot: I haz the power, 2015-06-02 18:16:29]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1159183002/140001

[commit-bot: I haz the power, 2015-06-02 19:48:15]

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

[commit-bot: I haz the power, 2015-06-02 19:48:17]

Dry run: This issue passed the CQ dry run.

[sadrul, 2015-06-02 20:41:27]

lgtm

[nasko, 2015-06-02 20:55:45]

The CQ bit was checked by nasko@chromium.org

[commit-bot: I haz the power, 2015-06-02 20:57:26]

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

[commit-bot: I haz the power, 2015-06-02 21:02:44]

Committed patchset #7 (id:140001)

[commit-bot: I haz the power, 2015-06-02 21:16:11]

Patchset 7 (id:??) landed as
https://crrev.com/954b02627a8c041cc5d3af2ae57593407bd4570b
Cr-Commit-Position: refs/heads/master@{#332462}