Use our own DiagnosticHandler for dart2js.

Use our own DiagnosticHandler for dart2js.

Instead of implementing SourceFileProvider and using dart2js's
built in DiagnosticHandler, we just provide our own
CompilerInputProvider and DiagnosticHandler functions. This avoids
implementing stuff in SourceFileProvider and, more importantly,
lets us handle missing source files better.

The DiagnosticHandler in dart2js just barfs and trashes the original
error if it can't find a source file to associate with it. The windows
bots are currently failing on an error I can't see because it doesn't
have a source file.

BUG=
R=nweiz@google.com

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

[nweiz, 2013-10-03 21:39:14]

lgtm

I'm not thrilled with the amount of duplication here -- we run a real risk of
breaking logging in subtle untested ways if dart2js makes some internal changes.
I don't think there's a better way to do this, though. Maybe file a bug against
them to add a better-factored interface?

https://codereview.chromium.org/25540004/diff/1/sdk/lib/_internal/pub/lib/src...
File sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart (right):

https://codereview.chromium.org/25540004/diff/1/sdk/lib/_internal/pub/lib/src...
sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart:85: /// copied
from dart2js. The primary difference is that it uses barback's
Copied from where in dart2js in particular?

https://codereview.chromium.org/25540004/diff/1/sdk/lib/_internal/pub/lib/src...
sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart:92: var
_isAborting = false;
Document this. Just saying it's copied from dart2js would be fine. Same goes for
other undocumented variables below.

https://codereview.chromium.org/25540004/diff/1/sdk/lib/_internal/pub/lib/src...
sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart:168: throw
'Unknown kind: $kind (${kind.ordinal})';
Don't throw a raw string.

https://codereview.chromium.org/25540004/diff/1/sdk/lib/_internal/pub/lib/src...
File sdk/lib/_internal/pub/lib/src/dart.dart (right):

https://codereview.chromium.org/25540004/diff/1/sdk/lib/_internal/pub/lib/src...
sdk/lib/_internal/pub/lib/src/dart.dart:51: assert(diagnosticHandler != null);
Throw an ArgumentError here.

[Bob Nystrom, 2013-10-03 21:48:51]

On 2013/10/03 21:39:14, nweiz wrote:
> lgtm
> 
> I'm not thrilled with the amount of duplication here -- we run a real risk of
> breaking logging in subtle untested ways if dart2js makes some internal
changes.

I think we generally do want to handle our own error reporting here. If nothing
else, to correctly go through barback's logger. But, yeah, reusing more dart2js
code would be good.

> I don't think there's a better way to do this, though. Maybe file a bug
against
> them to add a better-factored interface?

Bug #13671 is relevant to this.

> 
>
https://codereview.chromium.org/25540004/diff/1/sdk/lib/_internal/pub/lib/src...
> File sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart (right):
> 
>
https://codereview.chromium.org/25540004/diff/1/sdk/lib/_internal/pub/lib/src...
> sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart:85: /// copied
> from dart2js. The primary difference is that it uses barback's
> Copied from where in dart2js in particular?
> 
>
https://codereview.chromium.org/25540004/diff/1/sdk/lib/_internal/pub/lib/src...
> sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart:92: var
> _isAborting = false;
> Document this. Just saying it's copied from dart2js would be fine. Same goes
for
> other undocumented variables below.
> 
>
https://codereview.chromium.org/25540004/diff/1/sdk/lib/_internal/pub/lib/src...
> sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart:168: throw
> 'Unknown kind: $kind (${kind.ordinal})';
> Don't throw a raw string.
> 
>
https://codereview.chromium.org/25540004/diff/1/sdk/lib/_internal/pub/lib/src...
> File sdk/lib/_internal/pub/lib/src/dart.dart (right):
> 
>
https://codereview.chromium.org/25540004/diff/1/sdk/lib/_internal/pub/lib/src...
> sdk/lib/_internal/pub/lib/src/dart.dart:51: assert(diagnosticHandler != null);
> Throw an ArgumentError here.

[Bob Nystrom, 2013-10-03 21:48:57]

https://codereview.chromium.org/25540004/diff/1/sdk/lib/_internal/pub/lib/src...
File sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart (right):

https://codereview.chromium.org/25540004/diff/1/sdk/lib/_internal/pub/lib/src...
sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart:85: /// copied
from dart2js. The primary difference is that it uses barback's
On 2013/10/03 21:39:14, nweiz wrote:
> Copied from where in dart2js in particular?

Mentioned class.

https://codereview.chromium.org/25540004/diff/1/sdk/lib/_internal/pub/lib/src...
sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart:92: var
_isAborting = false;
On 2013/10/03 21:39:14, nweiz wrote:
> Document this. Just saying it's copied from dart2js would be fine. Same goes
for
> other undocumented variables below.

Done.

https://codereview.chromium.org/25540004/diff/1/sdk/lib/_internal/pub/lib/src...
sdk/lib/_internal/pub/lib/src/barback/dart2js_transformer.dart:168: throw
'Unknown kind: $kind (${kind.ordinal})';
On 2013/10/03 21:39:14, nweiz wrote:
> Don't throw a raw string.

Done.

https://codereview.chromium.org/25540004/diff/1/sdk/lib/_internal/pub/lib/src...
File sdk/lib/_internal/pub/lib/src/dart.dart (right):

https://codereview.chromium.org/25540004/diff/1/sdk/lib/_internal/pub/lib/src...
sdk/lib/_internal/pub/lib/src/dart.dart:51: assert(diagnosticHandler != null);
On 2013/10/03 21:39:14, nweiz wrote:
> Throw an ArgumentError here.

Done.

[Bob Nystrom, 2013-10-03 21:49:21]

Committed patchset #2 manually as r28246 (presubmit successful).