Make is and as checks handle JavaScriptFunction correctly.

Make is and as checks handle JavaScriptFunction correctly.

BUG=
R=sigmund@google.com, sra@google.com

Committed: https://github.com/dart-lang/sdk/commit/f9551da8e3f55734442be679369c00b234c19fa5

[Jacob, 2015-10-19 23:17:02]

jacobr@google.com changed reviewers:
+ sigmund@google.com, sra@chromium.org

[Jacob, 2015-10-19 23:17:02]

Fix for:
https://github.com/dart-lang/sdk/issues/24608

https://codereview.chromium.org/1416803002/diff/20001/sdk/lib/_internal/js_ru...
File sdk/lib/_internal/js_runtime/lib/js_helper.dart (right):

https://codereview.chromium.org/1416803002/diff/20001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/js_helper.dart:3182: if
((identical(JS('String', 'typeof #', value), 'object') ||
ran dartfmt on these methods hence the different indentation.

Obviously we should probably standardize on whether identical is used or the
string comparison is performed in JS. Let me know if you want to standardize to
the format used in this method or interceptedTypeCast.

[Siggi Cherem (dart-lang), 2015-10-19 23:27:18]

lgtm

eventually we plan to start using dartfmt, so I'm fine with those changes.

[Siggi Cherem (dart-lang), 2015-10-19 23:34:48]

https://codereview.chromium.org/1416803002/diff/20001/sdk/lib/_internal/js_ru...
File sdk/lib/_internal/js_runtime/lib/js_helper.dart (right):

https://codereview.chromium.org/1416803002/diff/20001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/js_helper.dart:3182: if
((identical(JS('String', 'typeof #', value), 'object') ||
On 2015/10/19 23:17:02, Jacob wrote:
> ran dartfmt on these methods hence the different indentation.
> 
> Obviously we should probably standardize on whether identical is used or the
> string comparison is performed in JS. Let me know if you want to standardize
to
> the format used in this method or interceptedTypeCast.

as for 
identical(JS('String', 'typeof #', value), 'function'

or
JS('bool', 'typeof # === "function"', value)

I slightly prefer the latter

[sra1, 2015-10-20 00:15:55]

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

[sra1, 2015-10-20 00:15:55]

lgtm

https://codereview.chromium.org/1416803002/diff/20001/sdk/lib/_internal/js_ru...
File sdk/lib/_internal/js_runtime/lib/js_helper.dart (right):

https://codereview.chromium.org/1416803002/diff/20001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/js_helper.dart:3182: if
((identical(JS('String', 'typeof #', value), 'object') ||
On 2015/10/19 23:34:48, Siggi Cherem (dart-lang) wrote:
> On 2015/10/19 23:17:02, Jacob wrote:
> > ran dartfmt on these methods hence the different indentation.
> > 
> > Obviously we should probably standardize on whether identical is used or the
> > string comparison is performed in JS. Let me know if you want to standardize
> to
> > the format used in this method or interceptedTypeCast.
> 
> as for 
> identical(JS('String', 'typeof #', value), 'function'
> 
> or
> JS('bool', 'typeof # === "function"', value)
> 
> I slightly prefer the latter

+1

V8 recognizes  typeof x == S  where S is a string literal.
It usually compiles to a few checks rather than a lot if checks to generate a
string followed by a string comparison. Other JS engines likely do similar.

This means the latter is safer since dart2js code generation can't obfuscate it
and confuse the JavaScript optimization by moving parts of the expression
around.

https://codereview.chromium.org/1416803002/diff/20001/sdk/lib/_internal/js_ru...
sdk/lib/_internal/js_runtime/lib/js_helper.dart:3199: JS('bool', '#[#]',
getInterceptor(value), property))) {
+rnystrom@google.com

This indentation is ugly.
Using a +1 indent for parenthesized expression components where possible would
go a long way to fixing it, something that a human might write.

[Jacob, 2015-10-20 00:58:31]

Committed patchset #3 (id:40001) manually as
f9551da8e3f55734442be679369c00b234c19fa5 (presubmit successful).