Bug 51546 – WebKit2 needs to mirror the frame tree in the UIProcess

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit2

[Sam Weinig]

Unfortunately, it looks like we need to mirror the frame tree in the UIProcess.

[Sam Weinig]

Created attachment 77354 [details]
Patch

[WebKit Review Bot]

Attachment 77354 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7322135

[Darin Adler]

Comment on attachment 77354 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=77354&action=review

> WebKit2/UIProcess/WebFrameProxy.cpp:184
> +    ASSERT(child->page() == page());

Assert that child->m_parentFrame is 0?
Assert that child->m_nextSibling is 0?
Assert that child->m_previousSibling is 0?

> WebKit2/UIProcess/WebFrameProxy.cpp:193
> +        oldLast->m_nextSibling = child;

Assert that oldLast->m_nextSibling was 0?

> WebKit2/UIProcess/WebFrameProxy.cpp:205
> +    std::swap(newLocationForNext, child->m_nextSibling);
> +    std::swap(newLocationForPrevious, child->m_previousSibling);

How about "using namespace std" at the top instead?

> WebKit2/UIProcess/WebFrameProxy.cpp:226
> +void WebFrameProxy::dumpFrameTree(unsigned indent)

It would be nicer if the name made it clear it’s a stdout-print type of dump. I like leaving this kind of function in the code as long as it’s obvious how to invoke it. If only someone had left an easy string-printing function in WTF::String!!!

> WebKit2/UIProcess/WebFrameProxy.h:136
> +    void removeAllChildFrames();

You mentioned we should remove this declaration for a function that is never used or defined.

[Sam Weinig]

Landed in r74571.

[Adam Barth]

(In reply to comment #0)
> Unfortunately, it looks like we need to mirror the frame tree in the UIProcess.

I'm curious why you need to mirror the Frame tree.  Chrome gets away without doing that.  The equivalent of the UIProcess only understands the concept of a WebView.

[Sam Weinig]

(In reply to comment #5)
> (In reply to comment #0)
> > Unfortunately, it looks like we need to mirror the frame tree in the UIProcess.
> 
> I'm curious why you need to mirror the Frame tree.  Chrome gets away without doing that.  The equivalent of the UIProcess only understands the concept of a WebView.

Safari uses the information in its Activity Window.  There are other ways to implement it however, so I will be trying to figure out if we can do something else efficiently enough that we can remove this.  I really want to remove this.  It sucks.

[Adam Barth]

> Safari uses the information in its Activity Window.  There are other ways to implement it however, so I will be trying to figure out if we can do something else efficiently enough that we can remove this.  I really want to remove this.  It sucks.

Ah, that makes sense.  Yeah, Chrome addresses this use case with the Web Inspector.  One of the design pressures we try to push back against is having the browser process (UIProcess) learn too much about WebKit internals.  Considerations might well be different in WebKit2, though, depending on how general a client you'd like to support.  Thanks for satisfying my curiosity.

[Sam Weinig]

(In reply to comment #7)
> > Safari uses the information in its Activity Window.  There are other ways to implement it however, so I will be trying to figure out if we can do something else efficiently enough that we can remove this.  I really want to remove this.  It sucks.
> 
> Ah, that makes sense.  Yeah, Chrome addresses this use case with the Web Inspector.  One of the design pressures we try to push back against is having the browser process (UIProcess) learn too much about WebKit internals.  Considerations might well be different in WebKit2, though, depending on how general a client you'd like to support.  Thanks for satisfying my curiosity.

That is a goal of WebKit2 as well.