Shutdown RenderWidgetHost owned by RenderFrameHost when destroying the RenderFrameHost.

Shutdown RenderWidgetHost owned by RenderFrameHost when destroying the RenderFrameHost.

This is needed because the RenderThread in the renderer process owns a reference to the RenderWidget, and that reference is only released when the RenderWidgetHost sends ViewMsg_Close, which happens on RenderWidgetHost::Shutdown.

BUG=464764
Committed: https://crrev.com/6a3da17d3e84b2a1150bd3fe301586b9960b5d44
Cr-Commit-Position: refs/heads/master@{#327331}

[lfg, 2015-04-22 19:36:29]

lfg@chromium.org changed reviewers:
+ kenrb@chromium.org

[lfg, 2015-04-22 19:36:29]

Hi Ken, please take a look.

[kenrb, 2015-04-23 15:56:27]

I see that this is necessary, and overall the change looks fine, but am I right
in thinking that this still leaves a problem of raciness between destruction of
the RenderFrame and the RenderWidget in the renderer process?

[lfg, 2015-04-23 19:06:23]

On 2015/04/23 15:56:27, kenrb wrote:
> I see that this is necessary, and overall the change looks fine, but am I
right
> in thinking that this still leaves a problem of raciness between destruction
of
> the RenderFrame and the RenderWidget in the renderer process?

Yes, that issue still has to be solved, but the raciness is between
WebLocalFrame and WebFrameWidget -- I'm not sure if the fix should be in Blink
or content, but I'm leaning towards fixing it in Blink. Currently (and with this
patch), it's not an issue because we don't destroy the LocalFrame/RenderFrame in
site-per-process.

[kenrb, 2015-04-23 21:39:18]

Okay. lgtm

You might try Nasko for owner review.

https://codereview.chromium.org/1052223008/diff/1/content/browser/frame_host/...
File content/browser/frame_host/render_frame_host_impl.cc (right):

https://codereview.chromium.org/1052223008/diff/1/content/browser/frame_host/...
content/browser/frame_host/render_frame_host_impl.cc:218:
render_widget_host_->Shutdown();
Worthwhile to add a comment that the RenderWidgetHost deletes itself on this
call.

[lfg, 2015-04-23 22:19:54]

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

[lfg, 2015-04-23 22:19:54]

+nasko: please take a look.

https://codereview.chromium.org/1052223008/diff/1/content/browser/frame_host/...
File content/browser/frame_host/render_frame_host_impl.cc (right):

https://codereview.chromium.org/1052223008/diff/1/content/browser/frame_host/...
content/browser/frame_host/render_frame_host_impl.cc:218:
render_widget_host_->Shutdown();
On 2015/04/23 21:39:18, kenrb wrote:
> Worthwhile to add a comment that the RenderWidgetHost deletes itself on this
> call.

Done.

[nasko, 2015-04-28 00:02:51]

LGTM

[lfg, 2015-04-28 17:20:57]

The CQ bit was checked by lfg@chromium.org

[lfg, 2015-04-28 17:20:57]

The patchset sent to the CQ was uploaded after l-g-t-m from kenrb@chromium.org
Link to the patchset: https://codereview.chromium.org/1052223008/#ps20001
(title: "addressing Ken's comment.")

[commit-bot: I haz the power, 2015-04-28 17:21:57]

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

[commit-bot: I haz the power, 2015-04-28 18:23:39]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-04-28 18:25:46]

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