[Webkit-unassigned] [Bug 38180] Remove unneeded custom code for WebSocket.send

[bugzilla-daemon at webkit.org]

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


Adam Barth <abarth at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mjs at apple.com




--- Comment #7 from Adam Barth <abarth at webkit.org>  2010-04-27 11:16:31 PST ---
(In reply to comment #6)
> > Is that code correct?  Why do we do that here but not everywhere else we
> > convert arguments to strings?
> 
> We do that sometimes. In particular, we do that elsewhere in WebSocket bindings
> code (e.g. constructWebSocket()).
[...]
> Providing better error reporting and catching exceptions from argument
> conversion early both seem to be generally good things to do, although there's
> of course the question of how these affect performance.

If that's the right things to do, then we should do it everywhere.  I don't
know why we have special-case code here.

> > Why do we check args.size() here?  That's very rare in our bindings
> 
> Searching for "if (args.size()" reveals many cases where we check for the
> number of arguments.

Our project-wide policy is not to check args.size() unless we need to.  This
violates the spec, but my understanding is that we do this to make writing JS
wrapper functions easier for library developers.  In any case, we should be
making these decisions on a project-wide basis (e.g., in the code generator)
instead of having randomly different behavior in each location based on who
happened to write the custom bindings code.

> I don't have good answers to those questions. It's not unthinkable that most of
> WebKit code is wrong in this respect. Given that IE and Firefox diverge a lot,
> and WebIDL is just a working draft, it's hard to even define"right" and
> "wrong".

Indeed.  However, consistency is a virtue.  As far as I can tell, this method
is randomly custom for no reason.  It seems much better to have it work the
same way as every other method.

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