[webkit-reviews] review denied: [Bug 38665] [chromium] Update chromium port to send/receive cached metadata : [Attachment 55596] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue May 11 15:53:09 PDT 2010


Darin Fisher (:fishd, Google) <fishd at chromium.org> has denied Tony Gentilcore
<tonyg at chromium.org>'s request for review:
Bug 38665: [chromium] Update chromium port to send/receive cached metadata
https://bugs.webkit.org/show_bug.cgi?id=38665

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

------- Additional Comments from Darin Fisher (:fishd, Google)
<fishd at chromium.org>
WebKit/chromium/public/WebKitClient.h:196
 +	virtual void cacheMetadata(const WebURL&, long long, const char*,
size_t) { }
please document the non-obvious parameters by giving them a name :-)

virtual void cacheMetadata(const WebURL&, long long responseTime, const char*
data, size_t dataLength) { }

ordinarily, we use 'double' to represent timestamps in webkit.	is there a
reason for using 'long long' instead of 'double'?  i realize that chromium uses
long long.

WebKit/chromium/public/WebURLLoaderClient.h:60
 +	virtual void didReceiveCachedMetadata(WebURLLoader*, const char* data,
int dataLength) { }
i presume it is promised that didReceiveCachedMetadata will not be called after
didFinishLoading or didFail, right?

WebKit/chromium/public/WebURLResponse.h:75
 +	WEBKIT_API long long responseTime() const;
same comment about 'long long' versus 'double'

WebKit/chromium/src/ResourceHandle.cpp:176
 +  void ResourceHandleInternal::didReceiveCachedMetadata(
no line break here in webkit style


More information about the webkit-reviews mailing list