Frame Refactor: Move focusWindow/unfocusWindow/shouldClose/scheduleClose to Chrome

• Component: WebKit Misc.
• Status: NEW ---

[Holger Freyther]

Move the above named methods from WebCore::Frame to WebCore::Chrome.

[Holger Freyther]

Created attachment 26859 [details]
Carry out the move.

The statusbar methods were left in Frame. They require more research, e.g. if the per frame statusbar text needs to be safed.

[Nikolas Zimmermann]

Comment on attachment 26859 [details]
Carry out the move.

Looks fine, long live refactoring, r=me :-)

[Holger Freyther]

Created attachment 27496 [details]
Carry out the move

Add null check for Frame::page in the FrameLoaderClient. shouldClose will be true when we have no page or chrome()->shouldClose will return true. This should resemble the current state with Frame::shouldClose.

[Holger Freyther]

Created attachment 27497 [details]
Move stuff to Chrome..

Unmangled version?

[Darin Adler]

Comment on attachment 27497 [details]
Move stuff to Chrome..

My intent here for these functions was to have the top-frame check be in DOMWindow, and the functions in Page not take a Frame*. I don't think it makes sense to "close" a non-main Frame. Could you make that change?

[Holger Freyther]

(In reply to comment #5)
> (From update of attachment 27497 [details] [review])
> My intent here for these functions was to have the top-frame check be in
> DOMWindow, and the functions in Page not take a Frame*. I don't think it makes
> sense to "close" a non-main Frame. Could you make that change?
> 

Sure.

[Adam Roben (:aroben)]

Comment on attachment 27497 [details]
Move stuff to Chrome..

> +        Like the many other Chrome methods take a Frame* pointer. Update the
> +        FrameLoader and the DOMWindow call site. In the FrameLoader it is assumed
> +        that the page is around...

This last sentence doesn't seem necessary anymore -- there's no code in FrameLoader in this patch that assumes there's a Page.

You need to patch WebView::shouldClose in WebKit/win/WebView.cpp as well.

r=me

[Darin Adler]

Comment on attachment 27497 [details]
Move stuff to Chrome..

I'm going to change this back to review- since I've requested a change.

[Eric Seidel (no email)]

Comment on attachment 26859 [details]
Carry out the move.

Clearing review flag to remove it from the commit queue.

[Eric Seidel (no email)]

Comment on attachment 26859 [details]
Carry out the move.

Clearing review flag to remove it from the commit queue.