[Webkit-unassigned] [Bug 40372] CodeGeneratorJS.pm incorrectly increments $paramIndex when a method is declared with [CallWith]

[bugzilla-daemon at webkit.org]

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





--- Comment #4 from Andrei Popescu <andreip at google.com>  2010-06-10 04:22:56 PST ---
(In reply to comment #3)
> (From update of attachment 58266 [details])
> No show stoppers, but a bunch of things I think could be better/cleaner.
> 
> 
> LayoutTests/storage/indexeddb/script-tests/idb-objectstore-request.js:25
>  +      doCreateOrOpen(function() { return db.createObjectStore('storeName', 'keyPath'); }, createSuccess);
> Why evaluate this as a function?  You might as well just pass the IDBResult into it.  That said, I actually don't think that helper function is worth the code obfuscation.
> 

Done.

> LayoutTests/storage/indexeddb/script-tests/idb-objectstore-request.js:5
>  +  function openSuccess()
> You should put these functions in some sort of order.  RIght now you jump from bottom to top to second from the bottom to second from the top.  Linear is probably easiest to read, though from the bottom up would work as well.
>

Done.

> LayoutTests/storage/indexeddb/script-tests/idb-objectstore-request.js:18
>  +      shouldBeEqualToString("store.keyPath", "keyPath");
> Put fixmes for the other stuff that needs to be done.
>

Added.

> LayoutTests/storage/indexeddb/script-tests/idb-objectstore-request.js:28
>  +  function doCreateOrOpen(func, successHandler)
> Instead of a 'doCreateOrOpen' we really need to clean up the environment before we start.  For now, just put in a 'FIXME' to clean up before we start and put a debug comment explaining that it'll only work once per instance of WebKit.
>

Added FIXME.

> 
> WebCore/bindings/scripts/CodeGeneratorJS.pm:1872
>  +                      my $argsIndex = 0;
> $argsIndex and $paramIndex aren't the most clear names.  Maybe even just $inParamIndex and $outParamIndex would be better.

Well, $paramIndex is not an index for output parameters. It is an index for input parameters, just as argsIndex. However, we already have the convention that "args" / "arguments" denote the parameters passed in from JS (see $argsCount or exec->arguments()). $paramsIndex is only used for the in params to the C++ function, so I think it's all reasonably clear.

 Landing manually.

Thanks,
Andrei

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