Move the frame tree into the embedder.

Move the frame tree into the embedder.

Frame ownership was previously transferred into the embedder layer. As
a result, we can also get rid of the RefPtr<>'s that were previously
used in the frame tree structure to keep frames alive, as the embedder
will only release its reference to the frame when it is removed from
the frame tree.

Several utility methods remain behind in FrameTree since they make more
sense there, but the actual tree structure has been lifted up into
WebFrameImpl.

BUG=340002
R=eseidel@chromium.org

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

[dcheng, 2014-02-07 18:35:57]

Eric, mind taking an initial pass at this CL? There's one unit test failure and
<10 layout test failures to resolve, but I think this is a good starting point
to review =)

[dcheng, 2014-02-12 17:32:26]

Ping?

[eseidel, 2014-02-13 23:49:39]

https://codereview.chromium.org/156123004/diff/90001/Source/core/page/FrameTr...
File Source/core/page/FrameTree.cpp (right):

https://codereview.chromium.org/156123004/diff/90001/Source/core/page/FrameTr...
Source/core/page/FrameTree.cpp:80: if (!m_thisFrame->loader().client())
I would have added a private loaderClient() or client() method given how often
you type this. :)

https://codereview.chromium.org/156123004/diff/90001/Source/core/page/FrameTr...
Source/core/page/FrameTree.cpp:151: name.appendNumber(childCount() - 1);
Why?

https://codereview.chromium.org/156123004/diff/90001/Source/core/page/FrameTr...
File Source/core/page/FrameTree.h (left):

https://codereview.chromium.org/156123004/diff/90001/Source/core/page/FrameTr...
Source/core/page/FrameTree.h:86: RefPtr<Frame> m_nextSibling;
So how does ownership work now?

https://codereview.chromium.org/156123004/diff/90001/Source/web/WebFrameImpl.cpp
File Source/web/WebFrameImpl.cpp (right):

https://codereview.chromium.org/156123004/diff/90001/Source/web/WebFrameImpl....
Source/web/WebFrameImpl.cpp:696: // FIXME: I am a bad person.
This is not particularly helpful. :)  It's not clear that these should be const
in the first place.

[dcheng, 2014-02-14 20:29:28]

https://codereview.chromium.org/156123004/diff/90001/Source/core/page/FrameTr...
File Source/core/page/FrameTree.cpp (right):

https://codereview.chromium.org/156123004/diff/90001/Source/core/page/FrameTr...
Source/core/page/FrameTree.cpp:151: name.appendNumber(childCount() - 1);
On 2014/02/13 23:49:40, eseidel wrote:
> Why?

This was to minimize the amount of text rebaselines that would be needed.

Previously, unique name generation worked like this:
- WebFrameImpl creates the child WebCore::Frame. At this point, the child is
only linked one way, from the child back to the parent.
- WebFrameImpl sets a name for the frame, which triggers unique name generation.
- WebFrameImpl finishes linking the child WebCore::Frame into the frame tree.

Now, we fully link the child into the frame tree before calling Frame::setName.
As a result, this shifted a lot of unique names for frames by 1, since
childCount() is no longer childCount() pre-child insertion.

https://codereview.chromium.org/156123004/diff/90001/Source/core/page/FrameTr...
File Source/core/page/FrameTree.h (left):

https://codereview.chromium.org/156123004/diff/90001/Source/core/page/FrameTr...
Source/core/page/FrameTree.h:86: RefPtr<Frame> m_nextSibling;
On 2014/02/13 23:49:40, eseidel wrote:
> So how does ownership work now?

I've updated the CL description to cover more of how this works.

https://codereview.chromium.org/156123004/diff/90001/Source/web/WebFrameImpl.cpp
File Source/web/WebFrameImpl.cpp (right):

https://codereview.chromium.org/156123004/diff/90001/Source/web/WebFrameImpl....
Source/web/WebFrameImpl.cpp:696: // FIXME: I am a bad person.
On 2014/02/13 23:49:40, eseidel wrote:
> This is not particularly helpful. :)  It's not clear that these should be
const
> in the first place.

The original versions were const. Making these non-const would have ripple
effects elsewhere. I've removed the comment for now.

[eseidel, 2014-02-14 21:04:09]

lgtm

[commit-bot: I haz the power, 2014-02-14 21:05:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dcheng@chromium.org/156123004/150001

[commit-bot: I haz the power, 2014-02-14 22:02:05]

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

[commit-bot: I haz the power, 2014-02-14 22:02:06]

Retried try job too often on mac_layout for step(s) webkit_lint
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_layout...

[dcheng, 2014-02-14 23:16:49]

The CQ bit was checked by dcheng@chromium.org

[commit-bot: I haz the power, 2014-02-14 23:19:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dcheng@chromium.org/156123004/360001

[commit-bot: I haz the power, 2014-02-15 03:29:48]

Change committed as 167231