[webkit-reviews] review granted: [Bug 46430] Platform specific ResourceResponse and ResourceRequest can't fully cross threads. : [Attachment 69098] crossThread patch

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has granted Michael Nordman
<michaeln at google.com>'s request for review:
Bug 46430: Platform specific ResourceResponse and ResourceRequest can't fully
cross threads.
https://bugs.webkit.org/show_bug.cgi?id=46430

Attachment 69098: crossThread patch
https://bugs.webkit.org/attachment.cgi?id=69098&action=review

------- Additional Comments from David Levin <levin at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=69098&action=review

Awesome!

> WebCore/platform/network/ResourceRequestBase.h:143
> +	   static bool compare(const ResourceRequest& a, const ResourceRequest&
b);

When I see "compare", I think stpcmp and expect that 0 means equal. How about
isSame or isEqual or equivalent, etc.? Ugh, I see ResourceResponse already has
this. I still hate the name but consistency is good, so nevermind :(


Do this: omit "a" and "b" as they add no information. (Yes,
ResourceResponseBase.h does the same thing but it is wrong and consistency here
adds no value.)

> WebCore/platform/network/android/ResourceRequest.h:58
> +    void doPlatformAdopt(PassOwnPtr<CrossThreadResourceRequestData> data) {
}

Omit data here or else you will get unused variable erros/warnings on some
platforms (throughout).

> WebCore/platform/network/chromium/ResourceRequest.h:93
> +	   PassOwnPtr<CrossThreadResourceRequestData>
doPlatformCopyData(PassOwnPtr<CrossThreadResourceRequestData> data) const;

Omit param name "data" here as it adds no information.

> WebCore/platform/network/chromium/ResourceRequest.h:94
> +	   void doPlatformAdopt(PassOwnPtr<CrossThreadResourceRequestData>
data);

Ditto.