[webkit-reviews] review denied: [Bug 3652] add support for link prefetching : [Attachment 60782] Remove tabs from changelog from last patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Jul 7 15:20:58 PDT 2010
Adam Barth <abarth at webkit.org> has denied Gavin Peters <gavinp at chromium.org>'s
request for review:
Bug 3652: add support for link prefetching
https://bugs.webkit.org/show_bug.cgi?id=3652
Attachment 60782: Remove tabs from changelog from last patch
https://bugs.webkit.org/attachment.cgi?id=60782&action=review
------- Additional Comments from Adam Barth <abarth at webkit.org>
Thanks, this is looking much better. Mostly questions and a few nits:
WebCore/html/HTMLLinkElement.h:56
+ { };
These shoudn't be indented quite as much and { and } should be on separate
lines. Also, the ; is superfluous.
WebCore/loader/DocLoader.cpp:234
+ case CachedResource::LinkPrefetch:
I don't think a link prefetch can corrupt a frame's pixels. Maybe we need to
add a new category here?
+ ENABLE_DEVICE_ORIENTATION = ;
Device orientation?
+ ENABLE_IMAGE_RESIZER = ;
??
LayoutTests/fast/dom/HTMLLinkElement/prefetch.html:7
+ setTimeout("layoutTestController.notifyDone()",50);
Boo
LayoutTests/fast/dom/HTMLLinkElement/prefetch.html:6
+ layoutTestController.dumpResourceResponseMIMETypes();
Why doesn't the show up in the results? Maybe you need to use something with a
known file extension, like HTML?
LayoutTests/platform/mac/Skipped:285
+ # Link prefetch is disabled by default
What platform will run the test? Why are we putting this behind a #define? We
should just implement the feature, no?
More information about the webkit-reviews
mailing list