[webkit-reviews] review denied: [Bug 73379] [chromium] add setOpener method to WebFrame : [Attachment 117080] Patch

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Karl Koscher
<supersat at chromium.org>'s request for review:
Bug 73379: [chromium] add setOpener method to WebFrame
https://bugs.webkit.org/show_bug.cgi?id=73379

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

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=117080&action=review


> Source/WebKit/chromium/public/WebFrame.h:221
> +    virtual void setOpener(const WebFrame*) = 0;

nit: please preserve two new lines above section header

note: we already have opener() and clearOpener().  it seems like setOpener()
should be
listed next to those.  also, perhaps we should delete clearOpener() in favor of
setOpener(0).
you could at least implement clearOpener() as an inline call to setOpener(0).

> Source/WebKit/chromium/src/WebFrameImpl.cpp:768
> +	   static_cast<const WebFrameImpl *>(frame)->m_frame : 0);

nit: no space after "WebFrameImpl"