[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