[webkit-reviews] review granted: [Bug 39636] Move functions out of Frame class that were marked "move to Chrome" : [Attachment 58693] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Darin Adler <darin at apple.com>'s
request for review:
Bug 39636: Move functions out of Frame class that were marked "move to Chrome"
https://bugs.webkit.org/show_bug.cgi?id=39636

Attachment 58693: Patch
https://bugs.webkit.org/attachment.cgi?id=58693&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
WebCore/loader/FrameLoader.cpp:683
 +	    if (DOMWindow* window = m_frame->existingDOMWindow()) {
I guess we don't care about the cases when the frame doesn't have a DOMWindow?

WebCore/page/DOMWindow.cpp:767
 +	if (!m_frame)
Don't we test for this above?

WebCore/page/DOMWindow.cpp:803
 +	if (!(page->openedByDOM() || page->getHistoryLength() <= 1 ||
allowScriptsToCloseWindows))
I think this would be clearer if you pushed the ! inside the ||

WebCore/page/DOMWindow.h:418
 +	inline String DOMWindow::status() const
I suspect the "inline" here is redundant.  Also, we commonly put these trivial
accessors on one line.

WebCore/page/DOMWindow.h:423
 +	inline String DOMWindow::defaultStatus() const
ditto

WebKit/win/WebView.cpp: 
 +	    *result = frame->shouldClose() ? TRUE : FALSE;
Do we not need this type conversion?  I don't really understand what Windows
needs here..