[Webkit-unassigned] [Bug 50187] implement onload events for <link rel=prefetch>

Sat Dec 4 01:05:30 PST 2010

https://bugs.webkit.org/show_bug.cgi?id=50187

--- Comment #9 from Adam Barth <abarth at webkit.org>  2010-12-04 01:05:30 PST ---
(From update of attachment 75598)
View in context: https://bugs.webkit.org/attachment.cgi?id=75598&action=review

This is looks to be correct and a nice approach.  I have a couple questions before, mostly for my education.

> LayoutTests/ChangeLog:8
> +        Implement onload events for WebKit
> +
> +        implement onload events for <link rel=prefetch>
> +        https://bugs.webkit.org/show_bug.cgi?id=50187

One of these lines seems redundant.  :)

> LayoutTests/fast/dom/HTMLLinkElement/link-and-subresource-test.html:16
> +    ++nick_load_count

missing ;

> LayoutTests/fast/dom/HTMLLinkElement/prefetch-onload.html:15
> +                if (window.layoutTestController) {
> +                    layoutTestController.notifyDone();
> +                }

No braces needed.

> LayoutTests/fast/dom/HTMLLinkElement/prefetch-onload.html:22
> +	</script>

bad indent

> LayoutTests/fast/dom/HTMLLinkElement/prefetch.html:-7
> -    setTimeout("layoutTestController.notifyDone()",50);

Yay!

> LayoutTests/http/tests/misc/prefetch-purpose.html:-17
> -<body onload="setTimeout('finishUp()', 50);">

yay!

> WebCore/html/HTMLLinkElement.cpp:376
> +#if ENABLE(LINK_PREFETCH)
> +void HTMLLinkElement::onloadTimerFired(Timer<HTMLLinkElement>*)
> +{

We usually have an ASSERT_UNUSED that we're being called back with the right timer.

> WebCore/html/HTMLLinkElement.cpp:377
> +    m_loading = false;

This line surprises me.  I'm not quite sure I understand it.

> WebCore/html/HTMLLinkElement.cpp:382
> +void HTMLLinkElement::notifyFinished(CachedResource*)
> +{

Again, an ASSERT here would be nice.

> WebCore/html/HTMLLinkElement.cpp:383
> +    m_onloadTimer.startOneShot(0);

Should we remove ourselves as a client of the prefetch at this point?

> WebCore/loader/cache/CachedResource.cpp:118
> +    setLoading(false);

I'm not sure I quite understand this line either.  Would you be willing to explain what it does?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.