Adding a mechanism to 'upgrade' the Dart type of elements

Adding a mechanism to 'upgrade' the Dart type of elements

This is to allow custom Dart wrappers for Polymer elements to provide improved JS custom element integration.

The Dartium portion of this is at https://codereview.chromium.org/188083002

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

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

[Jennifer Messerly, 2014-03-07 00:00:48]

LGTM

https://codereview.chromium.org/177113014/diff/20001/tests/html/custom/elemen...
File tests/html/custom/element_upgrade_test.dart (right):

https://codereview.chromium.org/177113014/diff/20001/tests/html/custom/elemen...
tests/html/custom/element_upgrade_test.dart:22: _proxy = new
js.JsObject.fromBrowserObject(this);
I wonder if, eventually, we can find a way to remove the _proxy object. Since we
already have a Dart object (FooElement in this example).

This seems like a good first step though.

https://codereview.chromium.org/177113014/diff/20001/tests/html/custom/elemen...
tests/html/custom/element_upgrade_test.dart:86: test('cannot upgrade
non-matching elements', () {
maybe a test that you can't upgrade a Dart custom element?

https://codereview.chromium.org/177113014/diff/20001/tests/html/custom/elemen...
tests/html/custom/element_upgrade_test.dart:92: test('cannot upgrade
non-matching elements', () {
this seems identical to above test.

https://codereview.chromium.org/177113014/diff/20001/tests/html/html.status
File tests/html/html.status (right):

https://codereview.chromium.org/177113014/diff/20001/tests/html/html.status#n...
tests/html/html.status:36: custom/element_upgrade_test: Skip # Issue 17298
is this fixed by the Dartium side of your change?

https://codereview.chromium.org/177113014/diff/20001/tools/dom/src/dartium_Cu...
File tools/dom/src/dartium_CustomElementSupport.dart (right):

https://codereview.chromium.org/177113014/diff/20001/tools/dom/src/dartium_Cu...
tools/dom/src/dartium_CustomElementSupport.dart:66: Symbol objectName =
reflectClass(Object).qualifiedName;
note: my comments below are before I realized this code was just moved around :)

Still, it might be worth simplifying it up a bit. Up to you.

https://codereview.chromium.org/177113014/diff/20001/tools/dom/src/dartium_Cu...
tools/dom/src/dartium_CustomElementSupport.dart:69: Symbol elementName =
reflectClass(HtmlElement).qualifiedName;
Hmmm. It might be more reliable to compare reflectedType instances for identity.
I'm not sure if qualifiedName's are guaranteed to be unique.

https://codereview.chromium.org/177113014/diff/20001/tools/dom/src/dartium_Cu...
tools/dom/src/dartium_CustomElementSupport.dart:74: while(!isRoot(superClass) &&
!isElement(superClass)) {
for isRoot, you could just walk up superClass == null right?
We used to have issues on dart2js but I think it works correctly everywhere now.
(and this bit of code is only on Dartium anyway)

https://codereview.chromium.org/177113014/diff/20001/tools/dom/src/dartium_Cu...
tools/dom/src/dartium_CustomElementSupport.dart:85: // TODO(vsm): Find a less
hackish way to do this.
ah, I see. I think instances compare correctly in the mirror system. So if you
wanted to check for dart:html you could do this for example:

reflectClass(HtmlElement).owner == cls.owner


Or dart:core:

reflectClass(Object).owner == cls.owner

https://codereview.chromium.org/177113014/diff/20001/tools/dom/templates/html...
File tools/dom/templates/html/impl/impl_HTMLDocument.darttemplate (right):

https://codereview.chromium.org/177113014/diff/20001/tools/dom/templates/html...
tools/dom/templates/html/impl/impl_HTMLDocument.darttemplate:308:
ElementUpgrader createElementUpgrader(Type type, {String extendsTag}) {
Maybe a TODO: we can move this to the registration context object once that API
is available.

[sra1, 2014-03-07 05:18:56]

lgtm

[blois, 2014-04-01 19:52:21]

https://codereview.chromium.org/177113014/diff/20001/tests/html/custom/elemen...
File tests/html/custom/element_upgrade_test.dart (right):

https://codereview.chromium.org/177113014/diff/20001/tests/html/custom/elemen...
tests/html/custom/element_upgrade_test.dart:86: test('cannot upgrade
non-matching elements', () {
On 2014/03/07 00:00:49, John Messerly wrote:
> maybe a test that you can't upgrade a Dart custom element?

Done.

https://codereview.chromium.org/177113014/diff/20001/tests/html/custom/elemen...
tests/html/custom/element_upgrade_test.dart:92: test('cannot upgrade
non-matching elements', () {
On 2014/03/07 00:00:49, John Messerly wrote:
> this seems identical to above test.

Done.

https://codereview.chromium.org/177113014/diff/20001/tests/html/html.status
File tests/html/html.status (right):

https://codereview.chromium.org/177113014/diff/20001/tests/html/html.status#n...
tests/html/html.status:36: custom/element_upgrade_test: Skip # Issue 17298
On 2014/03/07 00:00:49, John Messerly wrote:
> is this fixed by the Dartium side of your change?

Yes, but just want to avoid turning the bots red for a few minutes.

[blois, 2014-04-01 20:17:40]

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

[Jennifer Messerly, 2014-04-04 01:56:50]

https://codereview.chromium.org/177113014/diff/40001/tools/dom/src/dart2js_Cu...
File tools/dom/src/dart2js_CustomElementSupport.dart (right):

https://codereview.chromium.org/177113014/diff/40001/tools/dom/src/dart2js_Cu...
tools/dom/src/dart2js_CustomElementSupport.dart:180: throw new
ArgumentError('element is not subclass of $_nativeType');
I just noticed, this error message is different from the comment 2 lines up :)
... it should probably say 'element is not an instance of type $_nativeType'