[webkit-reviews] review granted: [Bug 22720] Make XMLHttpRequest work in Workers : [Attachment 26847] Addressed comments.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jan 19 12:19:49 PST 2009


Alexey Proskuryakov <ap at webkit.org> has granted David Levin
<levin at chromium.org>'s request for review:
Bug 22720: Make XMLHttpRequest work in Workers
https://bugs.webkit.org/show_bug.cgi?id=22720

Attachment 26847: Addressed comments.
https://bugs.webkit.org/attachment.cgi?id=26847&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
+    switch (destination) {
+    case InspectorControllerDestination:

We indent cases deeper. Also, it will be slightly beneficial to replace break
statements with return ones, and to add ASSERT_NOT_REACHED (gcc checks for
forgotten cases, but only at compile time, not runtime).

+    RefPtr<Task> task;
+    {
+	 String messageCopy = message.copy();
+	 String sourceURLCopy = sourceURL.copy();
+	 task = createCallbackTask(m_thread->messagingProxy(), &addMessageTask,
destination, source, level, messageCopy, lineNumber, sourceURLCopy);
+    }
+    postTaskToParentContext(task.release());

I think this needs a comment explaining that we're avoiding a race condition
with this construct. As discussed on IRC, this can probably be simplified if
you change createCallbackTask to take const references - but you still need an
explanatory comment.

r=me


More information about the webkit-reviews mailing list