[webkit-reviews] review requested: [Bug 73379] [chromium] add setOpener method to WebFrame : [Attachment 117478] Patch update

[bugzilla-daemon at webkit.org]

Karl Koscher <supersat at chromium.org> has asked	for review:
Bug 73379: [chromium] add setOpener method to WebFrame
https://bugs.webkit.org/show_bug.cgi?id=73379

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

------- Additional Comments from Karl Koscher <supersat at chromium.org>
(In reply to comment #2)
> (From update of attachment 117080 [details])
> 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

Fixed.
 
> 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).


I have no idea why I put setOpener there. I moved it and made clearOpener 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"

Oops. Old habits. :) Fixed.