[webkit-reviews] review granted: [Bug 24337] Assert when doing XHR for missing file in worker. : [Attachment 28249] Proposed fix.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Mar 4 00:57:40 PST 2009


Alexey Proskuryakov <ap at webkit.org> has granted David Levin
<levin at chromium.org>'s request for review:
Bug 24337: Assert when doing XHR for missing file in worker.
https://bugs.webkit.org/show_bug.cgi?id=24337

Attachment 28249: Proposed fix.
https://bugs.webkit.org/attachment.cgi?id=28249&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
+	     if (window.layoutTestController)
+		 layoutTestController.notifyDone();
+	     log("PASS");

The PASS string is printed after notifyDone(), so I'm very surprised it shows
up in expected results.

+    if (m_sendResourceLoadCallbacks)
+	 return frameLoader()->client()->willCacheResponse(documentLoader(),
identifier(), response);
+    return 0;

Our usual style is to do early return (if (!m_sendResourceLoadCallbacks) return
0;). Some say that's bad for performance though, so we may need to reconsider
it eventually.

I don't think this has much to do with the resource being missing - more
likely, it's just that the response is cacheable, unlike what a CGI from
methods.html sends. It would be nice to make this clearer in ChangeLog and test
comments.

r=me


More information about the webkit-reviews mailing list