Rename RenderViewGone IPC/methods to better reflect reality

Rename RenderViewGone IPC/methods to better reflect reality.

The RenderViewGone IPC and methods are incorrectly named, as those are used when a renderer process has died. Since we are passing in termination status around and usually iterating over all Render{Widget|View}Host in a specific process, it actually applies to the whole process. This CL renames all applicable instances of this method and the IPC message associated with it.

BUG=258087
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=210727

[nasko, 2013-07-08 23:50:00]

Please review this CL, which does a rename of RenderViewGone to
RenderProcessGone.

jam@ - overall CL and content OWNERS review
benm@ - android_webview/ OWNERS review
jochen@ - chrome/ OWNERS review
palmer@ - IPC message review

[nasko, 2013-07-08 23:53:07]

Adding rkc@ for ash/screensaver OWNERS review.

[rkc, 2013-07-08 23:59:49]

lgtm

[jam, 2013-07-09 01:02:40]

lgtm

[joth, 2013-07-09 01:14:59]

a_w/ lgtm rubberstamp


On 8 July 2013 18:02, <jam@chromium.org> wrote:

> lgtm
>
>
>
>
https://codereview.chromium.**org/18339006/<https://codereview.chromium.org/1...
>

[jochen (gone - plz use gerrit), 2013-07-09 07:02:52]

lgtm

[mkosiba (inactive), 2013-07-09 10:12:46]

LGTM for android_webview/

[nasko, 2013-07-09 16:02:05]

Adding creis@ to comment whether
RenderViewHostManager::RenderViewGoneFromRenderManager should also be renamed or
not.

[Charlie Reis, 2013-07-09 16:57:35]

Thanks for cleaning this up.  It will make much more sense when we start talking
about RenderFrames as well.

https://codereview.chromium.org/18339006/diff/4001/chrome/browser/ui/search/i...
File chrome/browser/ui/search/instant_page.h (right):

https://codereview.chromium.org/18339006/diff/4001/chrome/browser/ui/search/i...
chrome/browser/ui/search/instant_page.h:55: // Called when the underlying
RenderView crashed.
nit: RenderView's process

https://codereview.chromium.org/18339006/diff/4001/content/browser/renderer_h...
File content/browser/renderer_host/render_view_host_impl.h (right):

https://codereview.chromium.org/18339006/diff/4001/content/browser/renderer_h...
content/browser/renderer_host/render_view_host_impl.h:651: // initial navigation
of a RenderView and reset when the RenderView is
nit: RenderView's process

https://codereview.chromium.org/18339006/diff/4001/content/browser/web_conten...
File content/browser/web_contents/render_view_host_manager.cc (right):

https://codereview.chromium.org/18339006/diff/4001/content/browser/web_conten...
content/browser/web_contents/render_view_host_manager.cc:742:
delegate_->RenderViewGoneFromRenderManager(render_view_host_);
Sure, might as well change this to RenderProcessGoneFromRenderManager.  Both
this and OnRenderProcessGone end up calling RenderViewTerminated, but at least
it's consistent with OnRenderProcessGone.

https://codereview.chromium.org/18339006/diff/4001/content/browser/web_conten...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/18339006/diff/4001/content/browser/web_conten...
content/browser/web_contents/web_contents_impl.cc:2874: void
WebContentsImpl::RenderViewTerminated(RenderViewHost* rvh,
I'm debating whether we should change this to RenderProcessTerminated, to be
consistent.

Suppose there are two RVHs in this WebContents that are in the same process
(e.g., because we're over the limit).  Does this get called for each of them? 
If so, then I guess it could make sense to keep the RenderViewTerminated name. 
(Either way, we'll do something sensible below by returning early if it's not
our current RVH.)

[palmer, 2013-07-09 17:47:14]

LGTM with a nit

https://codereview.chromium.org/18339006/diff/4001/content/browser/renderer_h...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/18339006/diff/4001/content/browser/renderer_h...
content/browser/renderer_host/render_process_host_impl.cc:1621:
static_cast<int>(status),
Nit: Whitespace.

https://codereview.chromium.org/18339006/diff/4001/content/common/view_messag...
File content/common/view_messages.h (right):

https://codereview.chromium.org/18339006/diff/4001/content/common/view_messag...
content/common/view_messages.h:1412: int, /* this really is
base::TerminationStatus */
Use that type, then? In a subsequent CL. :)

[nasko, 2013-07-09 19:12:47]

Fixes for reviews by creis@ and palmer@.

https://codereview.chromium.org/18339006/diff/4001/chrome/browser/ui/search/i...
File chrome/browser/ui/search/instant_page.h (right):

https://codereview.chromium.org/18339006/diff/4001/chrome/browser/ui/search/i...
chrome/browser/ui/search/instant_page.h:55: // Called when the underlying
RenderView crashed.
On 2013/07/09 16:57:37, creis wrote:
> nit: RenderView's process

Done.

https://codereview.chromium.org/18339006/diff/4001/content/browser/renderer_h...
File content/browser/renderer_host/render_process_host_impl.cc (right):

https://codereview.chromium.org/18339006/diff/4001/content/browser/renderer_h...
content/browser/renderer_host/render_process_host_impl.cc:1621:
static_cast<int>(status),
On 2013/07/09 17:47:15, Chromium Palmer wrote:
> Nit: Whitespace.

Done.

https://codereview.chromium.org/18339006/diff/4001/content/browser/renderer_h...
File content/browser/renderer_host/render_view_host_impl.h (right):

https://codereview.chromium.org/18339006/diff/4001/content/browser/renderer_h...
content/browser/renderer_host/render_view_host_impl.h:651: // initial navigation
of a RenderView and reset when the RenderView is
On 2013/07/09 16:57:37, creis wrote:
> nit: RenderView's process

Done.

https://codereview.chromium.org/18339006/diff/4001/content/browser/web_conten...
File content/browser/web_contents/render_view_host_manager.cc (right):

https://codereview.chromium.org/18339006/diff/4001/content/browser/web_conten...
content/browser/web_contents/render_view_host_manager.cc:742:
delegate_->RenderViewGoneFromRenderManager(render_view_host_);
On 2013/07/09 16:57:37, creis wrote:
> Sure, might as well change this to RenderProcessGoneFromRenderManager.  Both
> this and OnRenderProcessGone end up calling RenderViewTerminated, but at least
> it's consistent with OnRenderProcessGone.

Done.

https://codereview.chromium.org/18339006/diff/4001/content/browser/web_conten...
File content/browser/web_contents/web_contents_impl.cc (right):

https://codereview.chromium.org/18339006/diff/4001/content/browser/web_conten...
content/browser/web_contents/web_contents_impl.cc:2874: void
WebContentsImpl::RenderViewTerminated(RenderViewHost* rvh,
On 2013/07/09 16:57:37, creis wrote:
> I'm debating whether we should change this to RenderProcessTerminated, to be
> consistent.
> 
> Suppose there are two RVHs in this WebContents that are in the same process
> (e.g., because we're over the limit).  Does this get called for each of them? 
> If so, then I guess it could make sense to keep the RenderViewTerminated name.

> (Either way, we'll do something sensible below by returning early if it's not
> our current RVH.)

This is called for each RenderViewHost, so let's keep it as is for now.

https://codereview.chromium.org/18339006/diff/4001/content/common/view_messag...
File content/common/view_messages.h (right):

https://codereview.chromium.org/18339006/diff/4001/content/common/view_messag...
content/common/view_messages.h:1412: int, /* this really is
base::TerminationStatus */
On 2013/07/09 17:47:15, Chromium Palmer wrote:
> Use that type, then? In a subsequent CL. :)

Sounds better in a separate CL.

[Charlie Reis, 2013-07-09 20:12:43]

Thanks!  LGTM.

[commit-bot: I haz the power, 2013-07-09 21:34:12]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/nasko@chromium.org/18339006/18002

[commit-bot: I haz the power, 2013-07-10 02:40:56]

Change committed as 210727