Splitting out the Audio library.

Splitting the Audio types out of the HTML library.


BUG=3108
Committed: https://code.google.com/p/dart/source/detail?r=15529

[Emily Fortuna, 2012-11-20 22:38:16]

lgtm with a naming suggestion

https://codereview.chromium.org/11280103/diff/1001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/11280103/diff/1001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/compiler.dart:496: || libraryName ==
'dart:audio'
should we call this htmlaudio instead? the svg lib is a bit more obviously html
oriented than just "dart:audio"

https://codereview.chromium.org/11280103/diff/1001/sdk/lib/html/scripts/syste...
File sdk/lib/html/scripts/systemhtml.py (right):

https://codereview.chromium.org/11280103/diff/1001/sdk/lib/html/scripts/syste...
sdk/lib/html/scripts/systemhtml.py:210: 'audio': {
:-)

[blois, 2012-11-20 23:46:11]

https://codereview.chromium.org/11280103/diff/1001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/11280103/diff/1001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/compiler.dart:496: || libraryName ==
'dart:audio'
On 2012/11/20 22:38:17, Emily Fortuna wrote:
> should we call this htmlaudio instead? the svg lib is a bit more obviously
html
> oriented than just "dart:audio"

Pretty much all of the APIs are from the 'Web Audio' spec, so web_audio may be
better.

Also, I have AudioElement moved into here as well, though that may be better in
dart:html.

[Emily Fortuna, 2012-11-20 23:59:22]

true. web_audio could be good. 
I'd still keep AudioElement in here where it is, but personal preference.

[Emily Fortuna, 2012-11-21 00:27:08]

https://codereview.chromium.org/11280103/diff/9001/tests/html/htmlaudioelemen...
File tests/html/htmlaudioelement_test.dart (left):

https://codereview.chromium.org/11280103/diff/9001/tests/html/htmlaudioelemen...
tests/html/htmlaudioelement_test.dart:1: library AudioElementTest;
why delete this test? Why don't we just rename it?

[Emily Fortuna, 2012-11-21 00:28:10]

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/html/templates/htm...
File sdk/lib/html/templates/html/dart2js/html_dart2js.darttemplate (right):

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/html/templates/htm...
sdk/lib/html/templates/html/dart2js/html_dart2js.darttemplate:13: import
'dart:web_audio' as audio;
as web_audio?

[blois, 2012-11-21 00:48:54]

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/html/templates/htm...
File sdk/lib/html/templates/html/dart2js/html_dart2js.darttemplate (right):

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/html/templates/htm...
sdk/lib/html/templates/html/dart2js/html_dart2js.darttemplate:13: import
'dart:web_audio' as audio;
On 2012/11/21 00:28:10, Emily Fortuna wrote:
> as web_audio?

It's not actually used at all, just there to ensure no name conflicts. I prefer
audio because prefixes should be simple.

https://codereview.chromium.org/11280103/diff/9001/tests/html/htmlaudioelemen...
File tests/html/htmlaudioelement_test.dart (left):

https://codereview.chromium.org/11280103/diff/9001/tests/html/htmlaudioelemen...
tests/html/htmlaudioelement_test.dart:1: library AudioElementTest;
On 2012/11/21 00:27:08, Emily Fortuna wrote:
> why delete this test? Why don't we just rename it?

Rename it to audioelement_test? I think someone already did that.

This test is almost an exact clone of audioelement_test.

[Emily Fortuna, 2012-11-21 01:48:16]

sgtm. lgtm.

[sra1, 2012-11-21 05:13:25]

lgtm

[Anton Muhin, 2012-11-22 03:22:51]

sorry for late review

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/compiler.dart:498: || libraryName ==
'dart:web_audio') {
just for my education, why <web>_audio?  are there any plans to make it more
generic with different implementation on different platforms?  or it's too early
to talk about that?

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/compiler.dart:498: || libraryName ==
'dart:web_audio') {
it might be time to switch to a list contains, but it's your call

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/native_handler.dart (right):

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/native_handler.dart:351: ||
libraryName == 'dart:web_audio') {
fitto for switching to list and again your call.

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/html/dart2js/html_...
File sdk/lib/html/dart2js/html_dart2js.dart (right):

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/html/dart2js/html_...
sdk/lib/html/dart2js/html_dart2js.dart:6: import 'dart:web_audio' as audio;
nit: as web_audio to follow a simple scheme?

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/html/scripts/dartd...
File sdk/lib/html/scripts/dartdomgenerator.py (right):

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/html/scripts/dartd...
sdk/lib/html/scripts/dartdomgenerator.py:188: if 'htmldart2js' in systems:
again, might be right time to introduce a list of libs to generate and turn
sequences below into a loop.  again, your call

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/html/scripts/htmlr...
File sdk/lib/html/scripts/htmlrenamer.py (right):

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/html/scripts/htmlr...
sdk/lib/html/scripts/htmlrenamer.py:362: if
self._database.HasInterface(idl_type_name):
another option would be to analyse file path of IDL: I hope that most if not all
of those IDLs should reside in Modules/webaudio:
http://trac.webkit.org/browser/trunk/Source/WebCore/Modules/webaudio

Yes, it means more work and can be deferred to a CL on its own, but it might be
helpful if you're planning to keep splitting the library.

https://codereview.chromium.org/11280103/diff/9001/tests/html/audiocontext_te...
File tests/html/audiocontext_test.dart (right):

https://codereview.chromium.org/11280103/diff/9001/tests/html/audiocontext_te...
tests/html/audiocontext_test.dart:4: import 'dart:web_audio';
nit: alphabetic sorting?

[blois, 2012-11-27 00:07:45]

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/compiler.dart:498: || libraryName ==
'dart:web_audio') {
On 2012/11/22 03:22:51, Anton Muhin wrote:
> just for my education, why <web>_audio?  are there any plans to make it more
> generic with different implementation on different platforms?  or it's too
early
> to talk about that?

I assume that there will be an audio for Android or other platforms as well. The
library is essentially entirely the 'Web Audio' API-
https://dvcs.w3.org/hg/audio/raw-file/tip/webaudio/specification.html

Also, my preferred design for audio on other platforms would be that we have a
dart:audio library which is the common subset, but then dart:web_audio would
'extend' that to provide additional functionality.

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/compiler.dart:498: || libraryName ==
'dart:web_audio') {
On 2012/11/22 03:22:51, Anton Muhin wrote:
> it might be time to switch to a list contains, but it's your call

Good point, I'd like to do that as a separate CL (and I have another CL out
which adds another item to this list).

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/native_handler.dart (right):

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/native_handler.dart:351: ||
libraryName == 'dart:web_audio') {
On 2012/11/22 03:22:51, Anton Muhin wrote:
> fitto for switching to list and again your call.

Will address as an independent CL.

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/html/dart2js/html_...
File sdk/lib/html/dart2js/html_dart2js.dart (right):

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/html/dart2js/html_...
sdk/lib/html/dart2js/html_dart2js.dart:6: import 'dart:web_audio' as audio;
On 2012/11/22 03:22:51, Anton Muhin wrote:
> nit: as web_audio to follow a simple scheme?

Since it's not used anywhere I had preferred audio, but changed to web_audio as
this is the second request for it to be web_audio :)

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/html/scripts/dartd...
File sdk/lib/html/scripts/dartdomgenerator.py (right):

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/html/scripts/dartd...
sdk/lib/html/scripts/dartdomgenerator.py:188: if 'htmldart2js' in systems:
On 2012/11/22 03:22:51, Anton Muhin wrote:
> again, might be right time to introduce a list of libs to generate and turn
> sequences below into a loop.  again, your call

Done.

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/html/scripts/htmlr...
File sdk/lib/html/scripts/htmlrenamer.py (right):

https://codereview.chromium.org/11280103/diff/9001/sdk/lib/html/scripts/htmlr...
sdk/lib/html/scripts/htmlrenamer.py:362: if
self._database.HasInterface(idl_type_name):
On 2012/11/22 03:22:51, Anton Muhin wrote:
> another option would be to analyse file path of IDL: I hope that most if not
all
> of those IDLs should reside in Modules/webaudio:
> http://trac.webkit.org/browser/trunk/Source/WebCore/Modules/webaudio
> 
> Yes, it means more work and can be deferred to a CL on its own, but it might
be
> helpful if you're planning to keep splitting the library.

I'll investigate this as part of breaking out the IndexedDB types.

https://codereview.chromium.org/11280103/diff/9001/tests/html/audiocontext_te...
File tests/html/audiocontext_test.dart (right):

https://codereview.chromium.org/11280103/diff/9001/tests/html/audiocontext_te...
tests/html/audiocontext_test.dart:4: import 'dart:web_audio';
On 2012/11/22 03:22:51, Anton Muhin wrote:
> nit: alphabetic sorting?

Done.

[Anton Muhin, 2012-11-27 03:30:37]

lgtm

[ahe, 2012-11-27 07:55:50]

Changes to sdk/lib/_internal/compiler, LGTM

https://codereview.chromium.org/11280103/diff/10010/sdk/lib/html/dart2js/html...
File sdk/lib/html/dart2js/html_dart2js.dart (right):

https://codereview.chromium.org/11280103/diff/10010/sdk/lib/html/dart2js/html...
sdk/lib/html/dart2js/html_dart2js.dart:6: import 'dart:web_audio' as web_audio;
Is this import used?

[blois, 2012-11-27 17:14:55]

https://codereview.chromium.org/11280103/diff/10010/sdk/lib/html/dart2js/html...
File sdk/lib/html/dart2js/html_dart2js.dart (right):

https://codereview.chromium.org/11280103/diff/10010/sdk/lib/html/dart2js/html...
sdk/lib/html/dart2js/html_dart2js.dart:6: import 'dart:web_audio' as web_audio;
On 2012/11/27 07:55:50, ahe wrote:
> Is this import used?

It's implicitly used in that dart:html can create audio objects via new
Element.tag() or access the types via document.query.

The import here basically ensures that the types are present so when an audio
object is returned from these methods then it works properly.

We have ambitions to remove these imports and have some form of predictable
behavior when the types are encountered but never included. This is essentially
an aggressive form of tree-shaking, but we need to quantify the benefits because
it's going to take some work to do, and might be a bit unexpected for users.

[ahe, 2012-11-27 19:01:22]

https://codereview.chromium.org/11280103/diff/10010/sdk/lib/html/dart2js/html...
File sdk/lib/html/dart2js/html_dart2js.dart (right):

https://codereview.chromium.org/11280103/diff/10010/sdk/lib/html/dart2js/html...
sdk/lib/html/dart2js/html_dart2js.dart:6: import 'dart:web_audio' as web_audio;
On 2012/11/27 17:14:55, blois wrote:
> On 2012/11/27 07:55:50, ahe wrote:
> > Is this import used?
> 
> It's implicitly used in that dart:html can create audio objects via new
> Element.tag() or access the types via document.query.
> 
> The import here basically ensures that the types are present so when an audio
> object is returned from these methods then it works properly.
> 
> We have ambitions to remove these imports and have some form of predictable
> behavior when the types are encountered but never included. This is
essentially
> an aggressive form of tree-shaking, but we need to quantify the benefits
because
> it's going to take some work to do, and might be a bit unexpected for users.

Thank you for the explanation.  Add a TODO?

[blois, 2012-11-27 20:49:41]

On 2012/11/27 19:01:22, ahe wrote:
>
https://codereview.chromium.org/11280103/diff/10010/sdk/lib/html/dart2js/html...
> File sdk/lib/html/dart2js/html_dart2js.dart (right):
> 
>
https://codereview.chromium.org/11280103/diff/10010/sdk/lib/html/dart2js/html...
> sdk/lib/html/dart2js/html_dart2js.dart:6: import 'dart:web_audio' as
web_audio;
> On 2012/11/27 17:14:55, blois wrote:
> > On 2012/11/27 07:55:50, ahe wrote:
> > > Is this import used?
> > 
> > It's implicitly used in that dart:html can create audio objects via new
> > Element.tag() or access the types via document.query.
> > 
> > The import here basically ensures that the types are present so when an
audio
> > object is returned from these methods then it works properly.
> > 
> > We have ambitions to remove these imports and have some form of predictable
> > behavior when the types are encountered but never included. This is
> essentially
> > an aggressive form of tree-shaking, but we need to quantify the benefits
> because
> > it's going to take some work to do, and might be a bit unexpected for users.
> 
> Thank you for the explanation.  Add a TODO?

Added a comment mentioning what these are for, but removing them is a feature
that we aren't sure if we can/will do at all.

[sra1, 2012-12-04 01:30:48]

https://chromiumcodereview.appspot.com/11280103/diff/10010/sdk/lib/html/dart2...
File sdk/lib/html/dart2js/html_dart2js.dart (right):

https://chromiumcodereview.appspot.com/11280103/diff/10010/sdk/lib/html/dart2...
sdk/lib/html/dart2js/html_dart2js.dart:6: import 'dart:web_audio' as web_audio;
On 2012/11/27 19:01:23, ahe wrote:
> On 2012/11/27 17:14:55, blois wrote:
> > On 2012/11/27 07:55:50, ahe wrote:
> > > Is this import used?
> > 
> > It's implicitly used in that dart:html can create audio objects via new
> > Element.tag() or access the types via document.query.
> > 
> > The import here basically ensures that the types are present so when an
audio
> > object is returned from these methods then it works properly.
> > 
> > We have ambitions to remove these imports and have some form of predictable
> > behavior when the types are encountered but never included. This is
> essentially
> > an aggressive form of tree-shaking, but we need to quantify the benefits
> because
> > it's going to take some work to do, and might be a bit unexpected for users.
> 
> Thank you for the explanation.  Add a TODO?

I think it makes sense for AudioElement to be part of dart:html.
It is not really part of WebAudio, and is used independent of the AudioNode
stuff.
It is a thin subclass of MediaElement which can play videos.

The classes that are marked in the IDL with Conditional=WEB_AUDIO belong here.