Bug 51546 - WebKit2 needs to mirror the frame tree in the UIProcess
: WebKit2 needs to mirror the frame tree in the UIProcess
Status: RESOLVED FIXED
: WebKit
WebKit2
: 528+ (Nightly build)
: PC Mac OS X 10.5
: P2 Normal
Assigned To:
:
:
:
:
  Show dependency treegraph
 
Reported: 2010-12-23 11:22 PST by
Modified: 2010-12-24 22:11 PST (History)


Attachments
Patch (38.10 KB, patch)
2010-12-23 11:49 PST, Sam Weinig
darin: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-12-23 11:22:42 PST
Unfortunately, it looks like we need to mirror the frame tree in the UIProcess.
------- Comment #1 From 2010-12-23 11:49:58 PST -------
Created an attachment (id=77354) [details]
Patch
------- Comment #2 From 2010-12-23 11:54:47 PST -------
Attachment 77354 [details] did not build on chromium:
Build output: http://queues.webkit.org/results/7322135
------- Comment #3 From 2010-12-23 12:00:16 PST -------
(From update of attachment 77354 [details])
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.
------- Comment #4 From 2010-12-23 12:10:23 PST -------
Landed in r74571.
------- Comment #5 From 2010-12-23 14:23:20 PST -------
(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.
------- Comment #6 From 2010-12-24 14:33:19 PST -------
(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.
------- Comment #7 From 2010-12-24 22:04:28 PST -------
> 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.
------- Comment #8 From 2010-12-24 22:11:31 PST -------
(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.