[Webkit-unassigned] [Bug 25435] Dedicated workers do not support sending MessagePorts in messages

[bugzilla-daemon at webkit.org]

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


eric at webkit.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #29947|review?                     |review-
               Flag|                            |




------- Comment #3 from eric at webkit.org  2009-05-22 07:18 PDT -------
(From update of attachment 29947)
Is this safe/correct?  args.size() < 1?
 88     String message = args.at(exec, 0).toString(exec);

Sad that JSWorker and JSWorkerContext can't share more code...

style:
7     if (proxy) {
 58         context = proxy->workerContext();
 59     } else {


We eventually need to write a throwOrReturn(ec, value) helper function to git
rid of this idiom:
67     if (ec)
 68         return throwError(ec);
 69 
 70     return v8::Undefined();

return throwOrReturn(ec, v8::Undefined())

Or better yet, a special case for there of:

return throwOrReturnUndefined(ec);

static v8::Handle<Value> throwOrReturn(ExceptionCode ec, v8::Handle<Value>
value)
{
   if (ec)
       return throwError(ec);
   return value;
}

that would clean up many of these bindings think.  Even just using it in these
two new places might be worth the win :)

We only name arguments when the names provide clarity beyond the typename:
 83         void postMessage(const String& message, MessagePort* port,
ExceptionCode&);
 84         void postMessage(const String& message, ExceptionCode& ec);

again:
 56         virtual void postMessageToWorkerContext(const String&,
MessagePort*, ExceptionCode& ec) = 0;

Overall this looks sane to me, but I'm not the right reviewer here.  Alexey is
your man I think.

r- for the style issues.  Re-post and Alexey can review for content.


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