Support DDC debugging tools.

Support DDC debugging tools.

BUG=
R=jakemac@google.com

Committed: https://github.com/dart-lang/pub/commit/be3d4d8be30f11fb9188d7f69434e884d4843c68

[Jacob, 2017-05-18 21:58:32]

jacobr@google.com changed reviewers:
+ rnystrom@google.com

[Jacob, 2017-05-18 21:58:32]

The right way to test some of this functionality would be with integration tests
verifying that stack traces in a
running application show Dart and not JS.
Perhaps we should add a package with integration tests.

[jakemac, 2017-05-19 14:38:05]

jakemac@google.com changed reviewers:
+ jakemac@google.com

[jakemac, 2017-05-19 14:38:06]

https://codereview.chromium.org/2893483005/diff/20001/lib/src/dartdevc/dartde...
File lib/src/dartdevc/dartdevc.dart (right):

https://codereview.chromium.org/2893483005/diff/20001/lib/src/dartdevc/dartde...
lib/src/dartdevc/dartdevc.dart:212: el = document.createElement("script");
duplicate code with the line right above :)

https://codereview.chromium.org/2893483005/diff/20001/lib/src/dartdevc/dartde...
lib/src/dartdevc/dartdevc.dart:281: '--source-map',
We should only include these in DEBUG mode, a few lines below this I think I add
--no-source-map in RELEASE mode so you can share logic with that.

https://codereview.chromium.org/2893483005/diff/20001/test/dartdevc/build_tes...
File test/dartdevc/build_test.dart (left):

https://codereview.chromium.org/2893483005/diff/20001/test/dartdevc/build_tes...
test/dartdevc/build_test.dart:71: d.matcherFile('web__main.js', isNot(isEmpty)),
Why is this one removed? It should still exist (it is the entrypoint module for
the web app).

[Jacob, 2017-05-19 16:24:02]

Moved the debugger functionality so it is only present in a debug build. It was
easy enough to refactor and it makes it clearer what functionality is core to
run the app and what is only needed for source map support and the devtools
debugger extension.

https://codereview.chromium.org/2893483005/diff/20001/lib/src/dartdevc/dartde...
File lib/src/dartdevc/dartdevc.dart (right):

https://codereview.chromium.org/2893483005/diff/20001/lib/src/dartdevc/dartde...
lib/src/dartdevc/dartdevc.dart:212: el = document.createElement("script");
On 2017/05/19 14:38:06, jakemac wrote:
> duplicate code with the line right above :)

Done.

https://codereview.chromium.org/2893483005/diff/20001/lib/src/dartdevc/dartde...
lib/src/dartdevc/dartdevc.dart:281: '--source-map',
On 2017/05/19 14:38:06, jakemac wrote:
> We should only include these in DEBUG mode, a few lines below this I think I
add
> --no-source-map in RELEASE mode so you can share logic with that.

Done.

https://codereview.chromium.org/2893483005/diff/20001/test/dartdevc/build_tes...
File test/dartdevc/build_test.dart (left):

https://codereview.chromium.org/2893483005/diff/20001/test/dartdevc/build_tes...
test/dartdevc/build_test.dart:71: d.matcherFile('web__main.js', isNot(isEmpty)),
On 2017/05/19 14:38:06, jakemac wrote:
> Why is this one removed? It should still exist (it is the entrypoint module
for
> the web app).
user error. added back.

[jakemac, 2017-05-19 16:35:59]

https://codereview.chromium.org/2893483005/diff/40001/lib/src/dartdevc/dartde...
File lib/src/dartdevc/dartdevc.dart (right):

https://codereview.chromium.org/2893483005/diff/40001/lib/src/dartdevc/dartde...
lib/src/dartdevc/dartdevc.dart:307: if (mode == BarbackMode.RELEASE) {
nit: maybe just use the `debugMode` bool you already have?

[Jacob, 2017-05-19 16:54:26]

https://codereview.chromium.org/2893483005/diff/40001/lib/src/dartdevc/dartde...
File lib/src/dartdevc/dartdevc.dart (right):

https://codereview.chromium.org/2893483005/diff/40001/lib/src/dartdevc/dartde...
lib/src/dartdevc/dartdevc.dart:307: if (mode == BarbackMode.RELEASE) {
On 2017/05/19 16:35:59, jakemac wrote:
> nit: maybe just use the `debugMode` bool you already have?

That debugMode var was in a different method. Switched this case to also check
for == DEBUG first as that is a little more consistent.

This is a different function. Added another
debugMode bool to this method and used it in this method.

[jakemac, 2017-05-19 19:33:02]

lgtm

[Jacob, 2017-05-19 19:40:19]

Description was changed from

==========
Support DDC debugging tools.

BUG=
==========

to

==========
Support DDC debugging tools.

BUG=
R=jakemac@google.com

Committed:
https://github.com/dart-lang/pub/commit/be3d4d8be30f11fb9188d7f69434e884d4843c68
==========

[Jacob, 2017-05-19 19:40:19]

Committed patchset #5 (id:80001) manually as
be3d4d8be30f11fb9188d7f69434e884d4843c68 (presubmit successful).

[Bob Nystrom, 2017-05-22 20:21:44]

Sorry I didn't get a chance to review this before you landed it, but if you
wouldn't mind a follow-up CL, I would really appreciate it.

https://codereview.chromium.org/2893483005/diff/80001/lib/src/dartdevc/dartde...
File lib/src/dartdevc/dartdevc.dart (right):

https://codereview.chromium.org/2893483005/diff/80001/lib/src/dartdevc/dartde...
lib/src/dartdevc/dartdevc.dart:23: // JavaScript snippet to determine the
directory a script was run from.
Nit: "///".

https://codereview.chromium.org/2893483005/diff/80001/lib/src/dartdevc/dartde...
lib/src/dartdevc/dartdevc.dart:95: var debugMode = mode == BarbackMode.DEBUG;
Nit: I think "isDebug" would make it a little clearer that it's a Boolean
variable.

https://codereview.chromium.org/2893483005/diff/80001/lib/src/dartdevc/dartde...
lib/src/dartdevc/dartdevc.dart:130: var modulePaths = new Map<String, String>();
We have map literals.

var modulePaths = {
  appModulePath: appModulePath,
  'dart_sdk': 'dart_sdk'
};

https://codereview.chromium.org/2893483005/diff/80001/lib/src/dartdevc/dartde...
lib/src/dartdevc/dartdevc.dart:180: var customModulePaths = new Map<String,
String>();
<String, String>{}

https://codereview.chromium.org/2893483005/diff/80001/lib/src/dartdevc/dartde...
lib/src/dartdevc/dartdevc.dart:246: ''');
What's going on here? Does it write this JS twice if debugMode is true?

https://codereview.chromium.org/2893483005/diff/80001/lib/src/dartdevc/dartde...
File lib/src/dartdevc/dartdevc_environment.dart (right):

https://codereview.chromium.org/2893483005/diff/80001/lib/src/dartdevc/dartde...
lib/src/dartdevc/dartdevc_environment.dart:211: 
Doc comment?