Changed to use JSInterop

Changed to use JSInterop

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

[terry, 2015-07-08 19:22:10]

terry@google.com changed reviewers:
+ alanknight@google.com, asiva@google.com, vsm@google.com

[terry, 2015-07-08 19:22:11]

Added code to support JS Interop for Dartium dart:xxx libraries (accessing the
DOM).  Currrently, this is under a switch --gen-interop to be used with go.sh
script (not yet enabled).  The Dartium and Dart2js libraries still essentially
generate the same code (sans calls to wrap_jso, wrap_jso_list, unwrap_jso,
etc.).  For dart2js these compile to nothing for Dartium (w/o --gen-interop)
they also do nothing.

Next checkin will be to generate a branch of the sdk/lib with this switch
enabled that will generate a different set of dartium libraries that uses
dart:js

[vsm, 2015-07-08 20:01:58]

https://codereview.chromium.org/1173403004/diff/30001/sdk/lib/indexed_db/dart...
File sdk/lib/indexed_db/dartium/indexed_db_dartium.dart (right):

https://codereview.chromium.org/1173403004/diff/30001/sdk/lib/indexed_db/dart...
sdk/lib/indexed_db/dartium/indexed_db_dartium.dart:94: bool operator ==(other)
=> unwrap_jso(other) == unwrap_jso(this) || identical(this, other);
Where are wrap_jso and unwrap_jso defined?

https://codereview.chromium.org/1173403004/diff/30001/tools/dom/scripts/css_c...
File tools/dom/scripts/css_code_generator.py (right):

https://codereview.chromium.org/1173403004/diff/30001/tools/dom/scripts/css_c...
tools/dom/scripts/css_code_generator.py:327: $if DART2JS
Perhaps hoist JSINTEROP out to get rid of redundant code in the case DART2JS or
!DART2JS && !JSINTEROP?

I.e., can both JSINTEROP and DART2JS be true?

Same thing a couple cases above.

https://codereview.chromium.org/1173403004/diff/30001/tools/dom/scripts/css_c...
tools/dom/scripts/css_code_generator.py:333: class CssStyleDeclarationBase {
Keep class abstract?

https://codereview.chromium.org/1173403004/diff/30001/tools/dom/scripts/gener...
File tools/dom/scripts/generator.py (right):

https://codereview.chromium.org/1173403004/diff/30001/tools/dom/scripts/gener...
tools/dom/scripts/generator.py:523: Notice we loo for any class that starts with
HtmlNNNN e.g., HtmlDocument, etc. """
s/loo/look/

https://codereview.chromium.org/1173403004/diff/30001/tools/dom/scripts/gener...
tools/dom/scripts/generator.py:534: if not type_registry:
You seem to have a bigger indent here (4) vs the rest of the file (2).

https://codereview.chromium.org/1173403004/diff/30001/tools/dom/scripts/syste...
File tools/dom/scripts/systemhtml.py (right):

https://codereview.chromium.org/1173403004/diff/30001/tools/dom/scripts/syste...
tools/dom/scripts/systemhtml.py:580: if (not(isinstance(self._backend,
Dart2JSBackend)) and
indent looks off

https://codereview.chromium.org/1173403004/diff/30001/tools/dom/templates/htm...
File tools/dom/templates/html/dartium/html_dartium.darttemplate (right):

https://codereview.chromium.org/1173403004/diff/30001/tools/dom/templates/htm...
tools/dom/templates/html/dartium/html_dartium.darttemplate:40: import
'dart:indexed_db' show indexed_dbBlinkFunctionMap;
I don't see a corresponding generated html_dartium.dart change.  Does this
include a go/sh run?

[terry, 2015-07-13 16:31:22]

PTAL

https://codereview.chromium.org/1173403004/diff/30001/sdk/lib/indexed_db/dart...
File sdk/lib/indexed_db/dartium/indexed_db_dartium.dart (right):

https://codereview.chromium.org/1173403004/diff/30001/sdk/lib/indexed_db/dart...
sdk/lib/indexed_db/dartium/indexed_db_dartium.dart:94: bool operator ==(other)
=> unwrap_jso(other) == unwrap_jso(this) || identical(this, other);
On 2015/07/08 20:01:58, vsm wrote:
> Where are wrap_jso and unwrap_jso defined?

There is dart:html (emitted from the html_dartium.darttemplate) dart2js
generated html dart libraries have no reference to wrap_jso only dartium
libraries.

https://codereview.chromium.org/1173403004/diff/30001/tools/dom/scripts/css_c...
File tools/dom/scripts/css_code_generator.py (right):

https://codereview.chromium.org/1173403004/diff/30001/tools/dom/scripts/css_c...
tools/dom/scripts/css_code_generator.py:333: class CssStyleDeclarationBase {
On 2015/07/08 20:01:58, vsm wrote:
> Keep class abstract?

Can't do it I need to construct CssStyleDeclaration and can't if the
getPropertyValue and setProperty are not implemented because it's used as a
mixin (I hope that calling super on a mixin will solve this problem in Dart). 
After checkin will talk to SRA we might be able to remove this mixin (SRA seemed
to suggest that was possible for dart2js too).

https://codereview.chromium.org/1173403004/diff/30001/tools/dom/scripts/gener...
File tools/dom/scripts/generator.py (right):

https://codereview.chromium.org/1173403004/diff/30001/tools/dom/scripts/gener...
tools/dom/scripts/generator.py:523: Notice we loo for any class that starts with
HtmlNNNN e.g., HtmlDocument, etc. """
On 2015/07/08 20:01:58, vsm wrote:
> s/loo/look/

Done.

https://codereview.chromium.org/1173403004/diff/30001/tools/dom/scripts/gener...
tools/dom/scripts/generator.py:534: if not type_registry:
On 2015/07/08 20:01:58, vsm wrote:
> You seem to have a bigger indent here (4) vs the rest of the file (2).

Done.

https://codereview.chromium.org/1173403004/diff/30001/tools/dom/scripts/syste...
File tools/dom/scripts/systemhtml.py (right):

https://codereview.chromium.org/1173403004/diff/30001/tools/dom/scripts/syste...
tools/dom/scripts/systemhtml.py:580: if (not(isinstance(self._backend,
Dart2JSBackend)) and
On 2015/07/08 20:01:58, vsm wrote:
> indent looks off

Done.

https://codereview.chromium.org/1173403004/diff/30001/tools/dom/templates/htm...
File tools/dom/templates/html/dartium/html_dartium.darttemplate (right):

https://codereview.chromium.org/1173403004/diff/30001/tools/dom/templates/htm...
tools/dom/templates/html/dartium/html_dartium.darttemplate:40: import
'dart:indexed_db' show indexed_dbBlinkFunctionMap;
On 2015/07/08 20:01:58, vsm wrote:
> I don't see a corresponding generated html_dartium.dart change.  Does this
> include a go/sh run?

You won't in this checkin we gen'd the sdk/lib w/o the --gen-interop switch so
the imports aren't there because its under $if INTEROP once flipped on then
you'll see this change.  I'll then run go.sh with the switch post this checkin
and I'll create another CL for the new sdk/lib under a branch that the dartium
DEPS file will point to then I'll build the new Dartium.

[terry, 2015-07-13 16:38:13]

PTAL

https://codereview.chromium.org/1173403004/diff/30001/tools/dom/scripts/css_c...
File tools/dom/scripts/css_code_generator.py (right):

https://codereview.chromium.org/1173403004/diff/30001/tools/dom/scripts/css_c...
tools/dom/scripts/css_code_generator.py:327: $if DART2JS
On 2015/07/08 20:01:58, vsm wrote:
> Perhaps hoist JSINTEROP out to get rid of redundant code in the case DART2JS
or
> !DART2JS && !JSINTEROP?
> 
> I.e., can both JSINTEROP and DART2JS be true?
> 
> Same thing a couple cases above.

Must have lost his comment (forgot to hit save).

In general now but right now I allow 2 types of Dartium C++ and dart:js
(interop) sdk/lib to be generated.  Soon I'll be removing that and the $else
part of the conditional in all templates and python files.

[terry, 2015-07-13 19:38:29]

Committed patchset #7 (id:60001) manually as
4947e570d47bb72a057a2069e83b4e80f48468a3 (presubmit successful).

[Paul Berry, 2015-07-14 06:09:25]

On 2015/07/13 19:38:29, terry wrote:
> Committed patchset #7 (id:60001) manually as
> 4947e570d47bb72a057a2069e83b4e80f48468a3 (presubmit successful).

This commit appears to be the source of analysis server errors, e.g.: python
tools/test.py -mrelease -cdart2analyzer -rnone --write-debug-log
--write-test-outcome-log -t240 co19/LayoutTests/fast/xpath/attr-namespace_t02

Which produces the following output:

Test configuration: dart2analyzer_none_release_ia32

FAILED: dart2analyzer-none release_ia32
co19/LayoutTests/fast/xpath/attr-namespace_t02
Expected: Pass 
Actual: StaticWarning
CommandOutput[dart2analyzer]:

stderr:
WARNING|STATIC_WARNING|NON_ABSTRACT_CLASS_INHERITS_ABSTRACT_MEMBER_TWO|/Users/paulberry/dart-new/sdk/tests/co19/src/LayoutTests/fast/xpath/attr-namespace_t02.dart|14|7|17|Missing
concrete implementation of setter 'XPathNSResolver.blink_jsObject' and getter
'XPathNSResolver.blink_jsObject'


Command[dart2analyzer]: DART_CONFIGURATION=ReleaseIA32 sdk/bin/dartanalyzer
--ignore-unrecognized-flags --show-package-warnings --enable-async
--package-root=xcodebuild/ReleaseIA32/packages/ --machine --no-hints
/Users/paulberry/dart-new/sdk/tests/co19/src/LayoutTests/fast/xpath/attr-namespace_t02.dart
Took 0:00:05.488000

Short reproduction command (experimental):
    python tools/test.py -mrelease -cdart2analyzer -rnone --write-debug-log
--write-test-outcome-log -t240 co19/LayoutTests/fast/xpath/attr-namespace_t02


It appears that this co19 test relies on being able to implement the interface
of XPathNSResolver, which is broken by the addition of the field blink_jsObject.


A total of 9 tests are currently failing, all with similar error messages.  I'm
not familiar enough with the code to know whether these are genuine problems or
whether the tests are bad, so I would appreciate your opinion.  You can see the
full list here:
http://build.chromium.org/p/client.dart/builders/analyzer_experimental-linux-...