[webkit-reviews] review denied: [Bug 38995] CSS link onload events : [Attachment 55967] Re-done layout test in right place

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri May 14 20:15:28 PDT 2010


Alexey Proskuryakov <ap at webkit.org> has denied Leon Clarke
<leonclarke at google.com>'s request for review:
Bug 38995: CSS link onload events
https://bugs.webkit.org/show_bug.cgi?id=38995

Attachment 55967: Re-done layout test in right place
https://bugs.webkit.org/attachment.cgi?id=55967&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
+void HTMLLinkElement::notifyFinished(CachedResource*)
+{
+    if (!m_timer.isActive())
+	 m_timer.startOneShot(0);
+}

I doubt that a timer is needed here. HTML5 spec probably says to queue a task
to dispatch an event, but that usually translates as "dispatch the event
immediately" in browser code.

+    if (!m_loading) {
	 c->setCSSStyleSheet(m_url, m_response.url(),
m_decoder->encoding().name(), this);
+	 c->notifyFinished(this);
+    }

I'm not quite sure about the design here, but it seems that setCSSStyleSheet
should go into notifyFinished() now. Please compare to what other
CachedResource subclasses do.

r-, because I'm fairly sure that adding a timer is wrong.


More information about the webkit-reviews mailing list