Correctly find root RWHV from a nested WebContents

Correctly find root RWHV from a nested WebContents

CrossProcessFrameConnector::GetRootRenderWidgetHostView() does not
traverse up through a tree of WebContents, which results in infinite
recursion. This adds a check for whether the current WebContents is
nested, and returns the root of the outer WebContents if so.

BUG=548912
R=nasko@chromium.org

Committed: https://crrev.com/29e91242fd0e1db2632ac1a0f83d645080ce20d4
Cr-Commit-Position: refs/heads/master@{#357355}

[kenrb, 2015-10-30 21:20:54]

Description was changed from

==========
Correctly find root RWHV from a nested WebContents

CrossProcessFrameConnector::GetRootRenderWidgetHostView() does not
traverse up through a tree of WebContents, which results in infinite
recursion. This adds a check for whether the current WebContents is
nested, and returns the root of the outer WebContents if so.

BUG=548912
R=nasko@chromium.org
==========

to

==========
Correctly find root RWHV from a nested WebContents

CrossProcessFrameConnector::GetRootRenderWidgetHostView() does not
traverse up through a tree of WebContents, which results in infinite
recursion. This adds a check for whether the current WebContents is
nested, and returns the root of the outer WebContents if so.

BUG=548912
R=nasko@chromium.org
==========

[kenrb, 2015-10-30 21:22:22]

nasko: PTAL?

The configuration of RenderWidgetHostViewChildFrame and
CrossProcessFrameConnector for a nested WebContents is correct, I had just
misunderstood it.

[nasko, 2015-10-30 21:29:12]

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

https://codereview.chromium.org/1420203007/diff/1/content/browser/frame_host/...
content/browser/frame_host/cross_process_frame_connector.cc:235: if
(top_host->frame_tree_node()->render_manager()->ForInnerDelegate()) {
This will just go one level up, right? I'm not sure if we have currently an easy
way to get to multiple levels of nesting in real code, but I think we should
structure the code to handle that. I think just changing the if into a while
should do the trick, right?

[kenrb, 2015-10-30 21:40:40]

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

https://codereview.chromium.org/1420203007/diff/1/content/browser/frame_host/...
content/browser/frame_host/cross_process_frame_connector.cc:235: if
(top_host->frame_tree_node()->render_manager()->ForInnerDelegate()) {
On 2015/10/30 21:29:12, nasko (slow to review) wrote:
> This will just go one level up, right? I'm not sure if we have currently an
easy
> way to get to multiple levels of nesting in real code, but I think we should
> structure the code to handle that. I think just changing the if into a while
> should do the trick, right?

I guess it might be more intuitive to do this. Originally I had thought it would
just jump to the next level up, and then call the appropriate method on that
RenderWidgetHostView, which could proxy again if it happened to be a
doubly-nested WebContents (this is how the bug caused infinite recursion). Since
we don't support more than one layer of WebContents anyway this is hypothetical,
but you're right it is easy to make it return the final target.

[nasko, 2015-10-30 21:45:18]

LGTM

[kenrb, 2015-11-02 14:17:40]

The CQ bit was checked by kenrb@chromium.org

[commit-bot: I haz the power, 2015-11-02 14:18:15]

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

[commit-bot: I haz the power, 2015-11-02 15:20:20]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-11-02 15:21:10]

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