Preparing Blink for cross-process frame tree replication

Preparing Blink for cross-process frame tree replication

This is an updated version of a patch originally written by
nasko@chromium.org.

A forthcoming Chromium CL will cause the entire frame tree of a page to
be replicated across processes when out-of-process iframes are in use.
This patch updates Blink to remove, in some places, the assumption that
top-level frames are in-process. The immediate goal is to prevent crashes,
while allow rendering and input events to work. Other breakages will be
addressed in the future.

BUG=357747
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=178395

[kenrb, 2014-07-16 21:02:32]

Eric or Adam, can one of you please review this?

Daniel: any concerns that you see for site isolation goals?

Nasko posted a rough WIP for this as https://codereview.chromium.org/348833005/.
I have rebased that and minimized it.

[dcheng, 2014-07-16 21:09:08]

https://codereview.chromium.org/397023003/diff/20001/Source/core/page/FocusCo...
File Source/core/page/FocusController.cpp (right):

https://codereview.chromium.org/397023003/diff/20001/Source/core/page/FocusCo...
Source/core/page/FocusController.cpp:308: for (Frame* frame =
m_page->mainFrame()->tree().top(); frame; frame = frame->tree().traverseNext())
{
Can we somehow annotate this as a horrible hack? Perhaps we can collapse this
with the identical loop in WebViewImpl and move it into FrameTree and just call
FrameTree::localRootTemporary in both locations?

https://codereview.chromium.org/397023003/diff/20001/Source/web/WebRemoteFram...
File Source/web/WebRemoteFrameImpl.cpp (left):

https://codereview.chromium.org/397023003/diff/20001/Source/web/WebRemoteFram...
Source/web/WebRemoteFrameImpl.cpp:518: ASSERT_NOT_REACHED();
We're deleting these because we haven't had time to fix the code that's
incorrectly calling these right?

https://codereview.chromium.org/397023003/diff/20001/Source/web/WebRemoteFram...
File Source/web/WebRemoteFrameImpl.cpp (right):

https://codereview.chromium.org/397023003/diff/20001/Source/web/WebRemoteFram...
Source/web/WebRemoteFrameImpl.cpp:15: #include "public/platform/WebLayer.h"
All these includes seem to be unnecessary.

[kenrb, 2014-07-17 14:57:02]

https://codereview.chromium.org/397023003/diff/20001/Source/core/page/FocusCo...
File Source/core/page/FocusController.cpp (right):

https://codereview.chromium.org/397023003/diff/20001/Source/core/page/FocusCo...
Source/core/page/FocusController.cpp:308: for (Frame* frame =
m_page->mainFrame()->tree().top(); frame; frame = frame->tree().traverseNext())
{
On 2014/07/16 21:09:08, dcheng (OOO) wrote:
> Can we somehow annotate this as a horrible hack? Perhaps we can collapse this
> with the identical loop in WebViewImpl and move it into FrameTree and just
call
> FrameTree::localRootTemporary in both locations?

I have added a comment. I don't think it is worth the effort to unify them
because both should be short-lived, and will be removed at different times.

https://codereview.chromium.org/397023003/diff/20001/Source/web/WebRemoteFram...
File Source/web/WebRemoteFrameImpl.cpp (left):

https://codereview.chromium.org/397023003/diff/20001/Source/web/WebRemoteFram...
Source/web/WebRemoteFrameImpl.cpp:518: ASSERT_NOT_REACHED();
On 2014/07/16 21:09:08, dcheng (OOO) wrote:
> We're deleting these because we haven't had time to fix the code that's
> incorrectly calling these right?

Yes, except for WebRemoteFrameImpl::view(), which I think correctly should
return a WebViewImpl.

https://codereview.chromium.org/397023003/diff/20001/Source/web/WebRemoteFram...
File Source/web/WebRemoteFrameImpl.cpp (right):

https://codereview.chromium.org/397023003/diff/20001/Source/web/WebRemoteFram...
Source/web/WebRemoteFrameImpl.cpp:15: #include "public/platform/WebLayer.h"
On 2014/07/16 21:09:08, dcheng (OOO) wrote:
> All these includes seem to be unnecessary.

Apparently so. Removed.

[eseidel, 2014-07-17 16:34:31]

lgtm

Very glad to see this taking shape.

[dcheng, 2014-07-17 17:55:42]

LGTM.

Let's get this in so we can focus on cleaning up the local root hacks =)

[kenrb, 2014-07-17 18:09:49]

The CQ bit was checked by kenrb@chromium.org

[commit-bot: I haz the power, 2014-07-17 18:10:51]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/kenrb@chromium.org/397023003/40001

[commit-bot: I haz the power, 2014-07-17 19:56:33]

Change committed as 178395