[webkit-reviews] review denied: [Bug 39456] [chromium] WebFrame::contentAsText should not return the text from hidden frames : [Attachment 58707] Fixed ChangeLog

[bugzilla-daemon at webkit.org]

Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Jay Civelli
<jcivelli at chromium.org>'s request for review:
Bug 39456: [chromium] WebFrame::contentAsText should not return the text from
hidden frames
https://bugs.webkit.org/show_bug.cgi?id=39456

Attachment 58707: Fixed ChangeLog
https://bugs.webkit.org/attachment.cgi?id=58707&action=review

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
WebKit/chromium/public/WebCString.h:72
 +	bool lessThan(const WebCString& other) const;
needs the WEBKIT_API prefix

WebKit/chromium/src/WebCString.cpp:44
 +  bool WebCString::lessThan(const WebCString& other) const
why not implement 'int compare(other)' instead?  given that you
are using strncmp, you basically get compare for free.

WebKit/chromium/tests/WebFrameTest.cpp:60
 +	    webkit_support::RegisterMockedURL(url, response,
WebString::fromUTF8(filePath.c_str()));
nit: no need for the .c_str()

WebKit/chromium/tests/WebFrameTest.cpp:77
 +	response.setMIMEType(WebString::fromUTF8("text/html"));
nit: you can just type response.setMIMEType("text/html"), and the right thing
will happen.

WebKit/chromium/tests/WebFrameTest.cpp:82
 +	    WebURL webURL = GURL(rootURL + files[i]);
we should really have a constructor for WebURL that parses its input.
that doesn't have to be part of this patch.