First try at removing ? from HTML.

First try at removing ? from HTML.

R=blois@google.com

Committed: https://code.google.com/p/dart/source/detail?r=22163

[Andrei Mouravski, 2013-04-24 11:23:20]

If you wouldn't mind, could you try patching this locally and seeing if anything
breaks? This is my first time testing locally built Dartium and I have less than
stellar confidence that I did everything right. :]

Also note, this is the first pass. There are probably more ? in the HTML libs,
but I am too sleepy to try them right now.

[Anton Muhin, 2013-04-25 09:33:34]

https://codereview.chromium.org/14400004/diff/1/sdk/lib/chrome/dart2js/chrome...
File sdk/lib/chrome/dart2js/chrome_dart2js.dart (right):

https://codereview.chromium.org/14400004/diff/1/sdk/lib/chrome/dart2js/chrome...
sdk/lib/chrome/dart2js/chrome_dart2js.dart:401: if (id != null)
that's most probably an error: imagine invocation like foo(null, 1, 2).

You will now treat this invocation as a no-arg as you'll see a null as a 1st
argument.  With nulls you should go opposite direction and check arguments from
the last to the first.

Let's fix it and continue with review.

[Andrei Mouravski, 2013-04-29 22:33:05]

Should be okay like this. PTAL.

https://chromiumcodereview.appspot.com/14400004/diff/1/sdk/lib/chrome/dart2js...
File sdk/lib/chrome/dart2js/chrome_dart2js.dart (right):

https://chromiumcodereview.appspot.com/14400004/diff/1/sdk/lib/chrome/dart2js...
sdk/lib/chrome/dart2js/chrome_dart2js.dart:401: if (id != null)
On 2013/04/25 09:33:34, Anton Muhin wrote:
> that's most probably an error: imagine invocation like foo(null, 1, 2).
> 
> You will now treat this invocation as a no-arg as you'll see a null as a 1st
> argument.  With nulls you should go opposite direction and check arguments
from
> the last to the first.
> 
> Let's fix it and continue with review.

Can you explain this? These are not positional optional arguments, so why does
it matter what order they go in?

Other places, sure, but here?

[blois, 2013-04-30 01:12:19]

https://chromiumcodereview.appspot.com/14400004/diff/5001/sdk/lib/chrome/dart...
File sdk/lib/chrome/dart2js/chrome_dart2js.dart (right):

https://chromiumcodereview.appspot.com/14400004/diff/5001/sdk/lib/chrome/dart...
sdk/lib/chrome/dart2js/chrome_dart2js.dart:256: if (callback != null) {
Was this manually changed or via the generator?

https://chromiumcodereview.appspot.com/14400004/diff/5001/tools/dom/templates...
File tools/dom/templates/html/dart2js/impl_AudioBufferSourceNode.darttemplate
(right):

https://chromiumcodereview.appspot.com/14400004/diff/5001/tools/dom/templates...
tools/dom/templates/html/dart2js/impl_AudioBufferSourceNode.darttemplate:13: if
(grainOffset != null) {
The grainOffset/grainDuration are flipped- probably should not be?

https://chromiumcodereview.appspot.com/14400004/diff/5001/tools/dom/templates...
File tools/dom/templates/html/impl/impl_AudioContext.darttemplate (right):

https://chromiumcodereview.appspot.com/14400004/diff/5001/tools/dom/templates...
tools/dom/templates/html/impl/impl_AudioContext.darttemplate:25: if
(numberOfInputChannels != null) {
Ditto- why flip order here?

[Andrei Mouravski, 2013-04-30 01:26:04]

Okay, everything resolved.

[blois, 2013-04-30 02:02:46]

lgtm

[Andrei Mouravski, 2013-04-30 02:29:40]

Committed patchset #3 manually as r22163 (presubmit successful).

[Anton Muhin, 2013-04-30 12:55:12]

https://chromiumcodereview.appspot.com/14400004/diff/1/sdk/lib/chrome/dart2js...
File sdk/lib/chrome/dart2js/chrome_dart2js.dart (right):

https://chromiumcodereview.appspot.com/14400004/diff/1/sdk/lib/chrome/dart2js...
sdk/lib/chrome/dart2js/chrome_dart2js.dart:401: if (id != null)
Yes, you're right, I was meaning positionals, sorry

On 2013/04/29 22:33:05, Andrei Mouravski wrote:
> On 2013/04/25 09:33:34, Anton Muhin wrote:
> > that's most probably an error: imagine invocation like foo(null, 1, 2).
> > 
> > You will now treat this invocation as a no-arg as you'll see a null as a 1st
> > argument.  With nulls you should go opposite direction and check arguments
> from
> > the last to the first.
> > 
> > Let's fix it and continue with review.
> 
> Can you explain this? These are not positional optional arguments, so why does
> it matter what order they go in?
> 
> Other places, sure, but here?

https://chromiumcodereview.appspot.com/14400004/diff/16001/sdk/lib/web_audio/...
File sdk/lib/web_audio/dart2js/web_audio_dart2js.dart (right):

https://chromiumcodereview.appspot.com/14400004/diff/16001/sdk/lib/web_audio/...
sdk/lib/web_audio/dart2js/web_audio_dart2js.dart:1: library dart.dom.web_audio;
there is no Dartium-specific modifications, is it by design?

https://chromiumcodereview.appspot.com/14400004/diff/16001/tools/dom/scripts/...
File tools/dom/scripts/htmldartgenerator.py (right):

https://chromiumcodereview.appspot.com/14400004/diff/16001/tools/dom/scripts/...
tools/dom/scripts/htmldartgenerator.py:390: for index, param_info in
enumerate(constructor_info.param_infos):
I believe you should revert an order of iteration as was discussed above.

For now we're apparently just lucky that we don't have more than a single
optional in ctors.

[blois, 2013-05-03 15:59:24]

https://chromiumcodereview.appspot.com/14400004/diff/16001/tools/dom/scripts/...
File tools/dom/scripts/htmldartgenerator.py (right):

https://chromiumcodereview.appspot.com/14400004/diff/16001/tools/dom/scripts/...
tools/dom/scripts/htmldartgenerator.py:390: for index, param_info in
enumerate(constructor_info.param_infos):
On 2013/04/30 12:55:13, Anton Muhin wrote:
> I believe you should revert an order of iteration as was discussed above.
> 
> For now we're apparently just lucky that we don't have more than a single
> optional in ctors.

Has this been resolved?