Change IsolateNatives.computeThisScript to use captured value of document.currentScript.

Change IsolateNatives.computeThisScript to use captured value of document.currentScript.

This should work on all browsers, including IE9. The problem with IE9 is that it doesn't define the "stack" property on exceptions.

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

[ngeoffray, 2013-02-19 15:22:26]

LGTM

https://codereview.chromium.org/12288047/diff/1/dart/sdk/lib/_internal/compil...
File dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart
(right):

https://codereview.chromium.org/12288047/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:582: //  
 document && (document.currentScript ||
Don't you need to use globalThis.document in the first check? I recall some
resolution failures on some engines if document was not defined.

https://codereview.chromium.org/12288047/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:590:
js['document']['scripts']['length'] - 1]))),
So this works on all browsers now?

https://codereview.chromium.org/12288047/diff/1/dart/sdk/lib/_internal/compil...
File dart/sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart
(right):

https://codereview.chromium.org/12288047/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart:425: //
property was not provided until IE10.
But this code is just relevant to command line engines, according to your
comment above.

https://codereview.chromium.org/12288047/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart:433: //
This pattern matches V8, Chrome, and Internet Explorer stack
Update comment to not reference browsers.

https://codereview.chromium.org/12288047/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart:445: //
methodName@URI:LINE
ditto

[ahe, 2013-02-19 15:50:37]

Thank you, submitting with a few tweaks.

https://codereview.chromium.org/12288047/diff/1/dart/sdk/lib/_internal/compil...
File dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart
(right):

https://codereview.chromium.org/12288047/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:582: //  
 document && (document.currentScript ||
On 2013/02/19 15:22:26, ngeoffray wrote:
> Don't you need to use globalThis.document in the first check? I recall some
> resolution failures on some engines if document was not defined.

I have changed this to (typeof document == 'object').

https://codereview.chromium.org/12288047/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/js_backend/emitter.dart:590:
js['document']['scripts']['length'] - 1]))),
On 2013/02/19 15:22:26, ngeoffray wrote:
> So this works on all browsers now?

As far as I can tell.

https://codereview.chromium.org/12288047/diff/1/dart/sdk/lib/_internal/compil...
File dart/sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart
(right):

https://codereview.chromium.org/12288047/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart:425: //
property was not provided until IE10.
On 2013/02/19 15:22:26, ngeoffray wrote:
> But this code is just relevant to command line engines, according to your
> comment above.

This comment is correct, and should help people understand the problem if they
try to restore this code rather than moving it to a helper library.

https://codereview.chromium.org/12288047/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart:433: //
This pattern matches V8, Chrome, and Internet Explorer stack
On 2013/02/19 15:22:26, ngeoffray wrote:
> Update comment to not reference browsers.

Why? The comment is correct.

https://codereview.chromium.org/12288047/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart:445: //
methodName@URI:LINE
On 2013/02/19 15:22:26, ngeoffray wrote:
> ditto

The comment is correct, and relevant for jsshell.

Generally, I think it is better to refer to the browser rather than the command
line version of the engine. Names like "d8", "jsshell", and "cscript" are
obscure.

[ngeoffray, 2013-02-19 16:00:01]

Take this as FYI, it's fine by me if you don't change the code.

https://codereview.chromium.org/12288047/diff/1/dart/sdk/lib/_internal/compil...
File dart/sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart
(right):

https://codereview.chromium.org/12288047/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart:425: //
property was not provided until IE10.
On 2013/02/19 15:50:37, ahe wrote:
> On 2013/02/19 15:22:26, ngeoffray wrote:
> > But this code is just relevant to command line engines, according to your
> > comment above.
> 
> This comment is correct, and should help people understand the problem if they
> try to restore this code rather than moving it to a helper library.

Even if it's correct, it is confusing, because it's code that will not be used
for browsers.

https://codereview.chromium.org/12288047/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart:445: //
methodName@URI:LINE
On 2013/02/19 15:50:37, ahe wrote:
> On 2013/02/19 15:22:26, ngeoffray wrote:
> > ditto
> 
> The comment is correct, and relevant for jsshell.
> 
> Generally, I think it is better to refer to the browser rather than the
command
> line version of the engine. Names like "d8", "jsshell", and "cscript" are
> obscure.

But you just used them line 417, which makes them less obscure.

[ahe, 2013-02-19 16:33:35]

https://codereview.chromium.org/12288047/diff/1/dart/sdk/lib/_internal/compil...
File dart/sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart
(right):

https://codereview.chromium.org/12288047/diff/1/dart/sdk/lib/_internal/compil...
dart/sdk/lib/_internal/compiler/implementation/lib/isolate_helper.dart:425: //
property was not provided until IE10.
On 2013/02/19 16:00:02, ngeoffray wrote:
> On 2013/02/19 15:50:37, ahe wrote:
> > On 2013/02/19 15:22:26, ngeoffray wrote:
> > > But this code is just relevant to command line engines, according to your
> > > comment above.
> > 
> > This comment is correct, and should help people understand the problem if
they
> > try to restore this code rather than moving it to a helper library.
> 
> Even if it's correct, it is confusing, because it's code that will not be used
> for browsers.

Now I have a problem. I certainly don't want to create confusion, but I really
think it is simpler to refer to the browser name (that are well-known to most),
rather than code names of engines and command lines.

What is worse, "cscript" and "wscript" are really not JavaScript engines. They
are general programs that allow you to execute scripts in many languages
(including Visual Basic and JavaScript).