[webkit-reviews] review denied: [Bug 21807] Add the ability for embedders to use Google-URL without changing shared headers : [Attachment 24564] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Dec 5 09:37:55 PST 2008


Darin Adler <darin at apple.com> has denied Brett Wilson (Google)
<brettw at chromium.org>'s request for review:
Bug 21807: Add the ability for embedders to use Google-URL without changing
shared headers
https://bugs.webkit.org/show_bug.cgi?id=21807

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

------- Additional Comments from Darin Adler <darin at apple.com>
This patch unnecessarily reduces the amount of inlining done in KURL for simple
operations. The non-Google-URL versions of these functions should still be
inlined.

This can be done either by adding a new header for the inline functions that's
included by KURL.h or by adding these function definitions to the end of KURL.h
rather than KURL.cpp.

Or you could prove that inlining of these isn't helpful for performance or code
size and then leave them in the .cpp file.

Otherwise the patch seems OK if still a bit inelegant.

 214	 void parse(const char* url, const String* originalString);  // KURLMac
calls this.
 215	 void copyToBuffer(Vector<char, 512>& buffer) const;  // KURLCFNet uses
this.

These should be outside the #if USE(GOOGLEURL) rather than repeated on both
sides of the if.

review- because of the unnecessary inlining change


More information about the webkit-reviews mailing list