Associate RenderWidgetHostView for new widget with correct process

Associate RenderWidgetHostView for new widget with correct process

Creating a select element dropdown in an out-of-process iframe is
crashing the browser process because the RenderWidgetHostView for the
new widget becomes associated with the top-level frame's renderer
process, creating a routing ID conflict. This CL initializes
the new RenderWidgetHostView with the correct RenderProcessHost.

BUG=582511
Committed: https://crrev.com/a26d15094453cbd05d2506c55fe4ccc7e8112ed6
Cr-Commit-Position: refs/heads/master@{#374740}

[kenrb, 2016-02-10 15:43:07]

Description was changed from

==========
Associate RenderWidgetHostView for new widget with correct process

Creating a select element dropdown in an out-of-process iframe is
crashing the browser process because the RenderWidgetHostView for the
new widget becomes associated with the top-level frame's renderer
process, creating a routing ID conflict. This CL initializes
the new RenderWidgetHostView with the correct RenderProcessHost.

BUG=582511
==========

to

==========
Associate RenderWidgetHostView for new widget with correct process

Creating a select element dropdown in an out-of-process iframe is
crashing the browser process because the RenderWidgetHostView for the
new widget becomes associated with the top-level frame's renderer
process, creating a routing ID conflict. This CL initializes
the new RenderWidgetHostView with the correct RenderProcessHost.

BUG=582511
==========

[kenrb, 2016-02-10 15:43:08]

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

[kenrb, 2016-02-10 15:45:53]

creis: PTAL?

[Charlie Reis, 2016-02-10 17:20:36]

Yikes, that's certainly wrong today!  Fix looks good with a nit.

Is this something easy to test so we can catch regressions?  Looks like
SitePerProcessBrowsertest has some helpers for input events if needed.

Also looks like GetRenderProcessHost uses in WebContents are a good way of
finding OOPIF bugs.  I'll file a bug to fix the others.

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

https://codereview.chromium.org/1685133003/diff/1/content/browser/web_content...
content/browser/web_contents/web_contents_impl.cc:1963: RenderProcessHost* rph =
RenderProcessHost::FromID(render_process_id);
We're also grabbing it here.  Let's just write it once, updating line 1959
instead.

[kenrb, 2016-02-10 18:25:48]

I can add a test now or when I make popups actually work, but in either case it
isn't clear to me how to hook that. WebContentsDelegate doesn't have any useful
methods. I could use a BrowserMessageFilter to catch the incoming IPCs but that
doesn't really verify that they are being correctly handled.

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

https://codereview.chromium.org/1685133003/diff/1/content/browser/web_content...
content/browser/web_contents/web_contents_impl.cc:1963: RenderProcessHost* rph =
RenderProcessHost::FromID(render_process_id);
On 2016/02/10 17:20:36, Charlie Reis wrote:
> We're also grabbing it here.  Let's just write it once, updating line 1959
> instead.

Done.

[Charlie Reis, 2016-02-10 18:30:29]

Thanks!

On 2016/02/10 18:25:48, kenrb wrote:
> I can add a test now or when I make popups actually work, but in either case
it
> isn't clear to me how to hook that. WebContentsDelegate doesn't have any
useful
> methods. I could use a BrowserMessageFilter to catch the incoming IPCs but
that
> doesn't really verify that they are being correctly handled.

Ok, let's add the test with the popup CL if you can.  I'd like to get this fix
landed.

LGTM.

[Charlie Reis, 2016-02-10 18:45:54]

The CQ bit was checked by creis@chromium.org

[commit-bot: I haz the power, 2016-02-10 18:48:30]

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

[commit-bot: I haz the power, 2016-02-10 22:00:35]

Description was changed from

==========
Associate RenderWidgetHostView for new widget with correct process

Creating a select element dropdown in an out-of-process iframe is
crashing the browser process because the RenderWidgetHostView for the
new widget becomes associated with the top-level frame's renderer
process, creating a routing ID conflict. This CL initializes
the new RenderWidgetHostView with the correct RenderProcessHost.

BUG=582511
==========

to

==========
Associate RenderWidgetHostView for new widget with correct process

Creating a select element dropdown in an out-of-process iframe is
crashing the browser process because the RenderWidgetHostView for the
new widget becomes associated with the top-level frame's renderer
process, creating a routing ID conflict. This CL initializes
the new RenderWidgetHostView with the correct RenderProcessHost.

BUG=582511
==========

[commit-bot: I haz the power, 2016-02-10 22:00:36]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-02-16 22:31:29]

Description was changed from

==========
Associate RenderWidgetHostView for new widget with correct process

Creating a select element dropdown in an out-of-process iframe is
crashing the browser process because the RenderWidgetHostView for the
new widget becomes associated with the top-level frame's renderer
process, creating a routing ID conflict. This CL initializes
the new RenderWidgetHostView with the correct RenderProcessHost.

BUG=582511
==========

to

==========
Associate RenderWidgetHostView for new widget with correct process

Creating a select element dropdown in an out-of-process iframe is
crashing the browser process because the RenderWidgetHostView for the
new widget becomes associated with the top-level frame's renderer
process, creating a routing ID conflict. This CL initializes
the new RenderWidgetHostView with the correct RenderProcessHost.

BUG=582511

Committed: https://crrev.com/a26d15094453cbd05d2506c55fe4ccc7e8112ed6
Cr-Commit-Position: refs/heads/master@{#374740}
==========

[commit-bot: I haz the power, 2016-02-16 22:31:30]

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