Start using RenderFrameProxyHost objects.

Start using RenderFrameProxyHost objects.

Instantiate RenderFrameProxyHost objects for remote frames when using
--site-per-process.

BUG=357747, 399709, 399775, 400594
TBR=kenrb, creis, kalman

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

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

[ncarter (slow), 2014-08-05 00:36:35]

Reviewers -- this is ken's LGTM'ed patch from
https://codereview.chromium.org/404613005/ , with one more test disabled. I am
TBRing you via a new issue, since Ken is out camping.

[ncarter (slow), 2014-08-05 00:36:43]

The CQ bit was checked by nick@chromium.org

[commit-bot: I haz the power, 2014-08-05 00:38:18]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/nick@chromium.org/444503002/20001

[commit-bot: I haz the power, 2014-08-05 03:09:47]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  mac_chromium_rel on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel/...)

[commit-bot: I haz the power, 2014-08-05 04:10:39]

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

[commit-bot: I haz the power, 2014-08-05 04:10:40]

Try jobs failed on following builders:
  mac_chromium_rel on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel/...)

[ncarter (slow), 2014-08-05 17:26:23]

The CQ bit was checked by nick@chromium.org

[commit-bot: I haz the power, 2014-08-05 17:28:12]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/nick@chromium.org/444503002/20001

[commit-bot: I haz the power, 2014-08-05 17:30:01]

Change committed as 287557

[robliao, 2014-08-05 21:44:33]

A revert of this CL has been created in
https://codereview.chromium.org/438393004/ by robliao@chromium.org.

The reason for reverting is: Appears to have destabilized
RenderFrameHostManagerTest.AllowTargetedNavigationsAfterSwap.

[robliao, 2014-08-05 21:50:18]

On 2014/08/05 21:44:33, robliao wrote:
> A revert of this CL has been created in
> https://codereview.chromium.org/438393004/ by mailto:robliao@chromium.org.
> 
> The reason for reverting is: Appears to have destabilized
> RenderFrameHostManagerTest.AllowTargetedNavigationsAfterSwap.

Here's the first failing bot:
http://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%281%29/buil...

[ncarter (slow), 2014-08-06 00:37:38]

Reopening this issue. The patch was reverted due to a failure in
RenderFrameHostManagerTest.AllowTargetedNavigationsAfterSwap, which is not (!!)
a --site-per-process test.

I've debugged this and there are two levels of failure. The first level of
failure is that this change apparently results in reuse of RenderViewHosts (in a
normal, non-oopif operation) that previously weren't reused. Precisely, what
happens is that we're navigating a window from site A to site B, then back to
site A, and finally to site B. During the second navigation to B, sometimes the
first RenderFrameHost for B is still around, and as a result, its RenderViewHost
is reused by the new code in RenderFrameHostManager::CreateRenderFrameHost.

This reuse violates an invariant that we seem to have had when in normal (not
--site-per-process) mode, namely: RenderViewHosts and RenderFrameHosts should be
one-to-one. When this invariant is broken, any uses of
RenderViewHostImpl::main_frame_routing_id() can result in bugs, since there's
now a chance that it's not the frame we're looking for.

Which brings us to the second layer of the bug, which is a what was causing the
test to fail: we get and process an OnCrossSiteResponse that's directed to a
RenderFrameHost on the RFHM's pending_delete_hosts_ list. This navigation was
initiated by the RFH with routing_id 7, but its response is processed on the RFH
with routing_id 4.

The knowledge of the originating RFH seems to be lost on the following line in
the RFHM code:

      pending_render_frame_host_->render_view_host()->
          SetHasPendingCrossSiteRequest(true);

Making that be per-frame will probably help fix some bugs with oopif, but I also
think we need to revisit the changes to the RVH reuse logic in this patch, as
that wasn't an intended consequence of this patch.

[ncarter (slow), 2014-08-06 21:36:22]

Patch set 5, assuming it passes the trybots, is what I hope to submit.

[Charlie Reis, 2014-08-06 22:01:46]

On 2014/08/06 21:36:22, ncarter wrote:
> Patch set 5, assuming it passes the trybots, is what I hope to submit.

LGTM.  We should always create a new RVH for main frames, and there should
always be an existing one for subframes (since we walk the frame tree starting
from the root when we create proxies).  I like having it enforced with a CHECK.

[ncarter (slow), 2014-08-06 22:03:15]

The CQ bit was checked by nick@chromium.org

[commit-bot: I haz the power, 2014-08-06 22:04:24]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/nick@chromium.org/444503002/80001

[ncarter (slow), 2014-08-06 22:25:30]

The CQ bit was unchecked by nick@chromium.org

[ncarter (slow), 2014-08-06 23:45:37]

The CQ bit was checked by nick@chromium.org

[commit-bot: I haz the power, 2014-08-06 23:48:48]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/nick@chromium.org/444503002/80001

[commit-bot: I haz the power, 2014-08-07 03:30:18]

Change committed as 287954