[webkit-reviews] review denied: [Bug 22720] Make XMLHttpRequest work in Workers : [Attachment 27070] Part 5a: Add the GenericWorkTask infrastructure needed for WorkerThreadableLoader.

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has denied 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 27070: Part 5a: Add the GenericWorkTask infrastructure needed for
WorkerThreadableLoader.
https://bugs.webkit.org/attachment.cgi?id=27070&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
I don't think that relying on IsInteger is the right approach - e.g. it
unnecessarily omits floating point values. I'll e-mail you some ideas to
discuss.

+	WebCore/dom/CrossThreadAdapter.h \

The name CrossThreadAdapter is not sufficiently descriptive, it says nothing
about the purpose or behavior of this class (not to mention that it does not
match Adapter pattern).

+++ b/WebCore/dom/CrossThreadAdapter.cpp
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2005, 2006, 2007, 2008 Apple Inc. All rights reserved.

I don't see any old Apple code here. Also, did you use LGPL intentionally? I
think that for other submissions, you used a BSD license.

+	 typedef typename ComplexCopyAvoider<P1>::Type InParam1;

AFAIK, "Avoider" is not a word. But I'm also not sure how this is different
from CrossThreadAdapter::ReturnType - can these be merged?

+    template<typename T> struct ComplexCopyAvoider<T*> : public PtrType<T*> {
+    };
+    template<typename T> struct ComplexCopyAvoider<T* const> : public
PtrType<T*> {

As you are removing const, you could also remove volative, as in
<http://www.boost.org/doc/libs/1_37_0/libs/type_traits/doc/html/boost_typetrait
s/reference/remove_cv.html>.

I think that behavior-wise, this is great, but the C++ magic needs another
iteration or two to get it in a more maintainable shape.