[webkit-reviews] review denied: [Bug 25064] Implement a content sniffer : [Attachment 30080] Step 2: Hook sniffer into network stack (ready for review)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri May 22 21:42:40 PDT 2009


Maciej Stachowiak <mjs at apple.com> has denied Adam Barth <abarth at webkit.org>'s
request for review:
Bug 25064: Implement a content sniffer
https://bugs.webkit.org/show_bug.cgi?id=25064

Attachment 30080: Step 2: Hook sniffer into network stack (ready for review)
https://bugs.webkit.org/attachment.cgi?id=30080&action=review

------- Additional Comments from Maciej Stachowiak <mjs at apple.com>
The low-level details of the code look ok, but I'm not entirely happy with the
design approach. It seems like overkill to create a generic forwarding resource
handle client just to attach some built-in behavior to didReceiveResponse. I
think it would make more sense to implement an appropriate method in
ResourceHandleBase and require ResourceHandle subclasses to call that method at
the appropriate point.

Also:

This adds a call to shouldSniffMIMEType, but no call to sniffMIMEType. Part 3
doesn't call it either, so how does any actual sniffing ever happen?

It might actually be simpler to combine this and part 1 with the patch to add
the sniffer logic. Seems like it would be easier to review it all as a unit so
that the reviewer can follow the logic.

r- for the above.


More information about the webkit-reviews mailing list