Port sky-scrollable to Dart

Port sky-scrollable to Dart

This CL ports sky-scrollable to the new sky-element.

R=esprehn@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/dff4b5f6f62c9cab61cb3954f0e7fc17e03de75a

[abarth-chromium, 2015-02-21 09:08:00]

abarth@chromium.org changed reviewers:
+ eseidel@chromium.org, ojan@chromium.org

[abarth-chromium, 2015-02-21 09:15:50]

https://codereview.chromium.org/950603002/diff/1/sky/engine/core/dom/Element.idl
File sky/engine/core/dom/Element.idl (right):

https://codereview.chromium.org/950603002/diff/1/sky/engine/core/dom/Element....
sky/engine/core/dom/Element.idl:15: [CustomElementCallbacks, RaisesException]
void setAttributes(sequence<Attr> attributes);
Adding this API seemed easier than writing a loop to call setAttribute.

https://codereview.chromium.org/950603002/diff/1/sky/engine/core/html/parser/...
File sky/engine/core/html/parser/HTMLDocumentParser.cpp (left):

https://codereview.chromium.org/950603002/diff/1/sky/engine/core/html/parser/...
sky/engine/core/html/parser/HTMLDocumentParser.cpp:398:
ASSERT(!isWaitingForScripts());
sky/examples/example-scrollable/index.sky hits this ASSERT.  I need to make a
reduced test case for sky/tests.

https://codereview.chromium.org/950603002/diff/1/sky/examples/example-scrolla...
File sky/examples/example-scrollable/example-scrollable.sky (right):

https://codereview.chromium.org/950603002/diff/1/sky/examples/example-scrolla...
sky/examples/example-scrollable/example-scrollable.sky:31:
parent.appendChild(div);
This code is compensating for lack of template@repeat.

https://codereview.chromium.org/950603002/diff/1/sky/framework/sky-element.sky
File sky/framework/sky-element.sky (right):

https://codereview.chromium.org/950603002/diff/1/sky/framework/sky-element.sk...
sky/framework/sky-element.sky:50:
shadow.appendChild(_cloneDeep(registration.template.content));
Custom elements don't quite work properly with the implicit "adoptNode" call
that existed here previously.  The problem is that the template has a separate
element registry (to avoid inflating custom elements in templates) and when we
adopt the cloned elements, we keep their identity (by mutating their document
field).  That means we end up with elements in the main document that weren't
created with the main document's element registry.

What I've done here is to clone the template manually and create new elements in
the main document, which lets us use the correct element registry.

https://codereview.chromium.org/950603002/diff/1/sky/framework/sky-element.sk...
sky/framework/sky-element.sky:50:
shadow.appendChild(_cloneDeep(registration.template.content));
Custom elements don't quite work properly with the implicit "adoptNode" call
that existed here previously.  The problem is that the template has a separate
element registry (to avoid inflating custom elements in templates) and when we
adopt the cloned elements, we keep their identity (by mutating their document
field).  That means we end up with elements in the main document that weren't
created with the main document's element registry.

What I've done here is to clone the template manually and create new elements in
the main document, which lets us use the correct element registry.

[esprehn, 2015-02-21 09:57:12]

esprehn@chromium.org changed reviewers:
+ esprehn@chromium.org

[esprehn, 2015-02-21 09:57:13]

You don't need setAttributes or the _clone methods, just use importNode.

https://codereview.chromium.org/950603002/diff/1/sky/framework/sky-element.sky
File sky/framework/sky-element.sky (right):

https://codereview.chromium.org/950603002/diff/1/sky/framework/sky-element.sk...
sky/framework/sky-element.sky:50:
shadow.appendChild(_cloneDeep(registration.template.content));
We should use importNode, which clones you into the right document.

https://codereview.chromium.org/950603002/diff/1/sky/framework/sky-element.sk...
sky/framework/sky-element.sky:82: }
This is duplicating what importNode already does.

[abarth-chromium, 2015-02-21 16:00:39]

On 2015/02/21 at 09:57:13, esprehn wrote:
> You don't need setAttributes or the _clone methods, just use importNode.

importNode doesn't work for the reason I explained in the CL comments. 
Specifically, importNode maintains object identity by mutating the internal
|document| property of the elements.  By the time we run importNode, the
runtimeType of the element is already established (i.e., by the element registry
inside the template).  That means the elements importNode returns still have the
same runTime type as they did before.  In particular, the |sky-scrollable|
element has a runtimeType of Element rather than of SkyScrollable.

By contrast, the _clone creates new elements using the document.createElement
factory, which means the |sky-scrollable| element ends up with a runtimeType of
SkyScrollable, which is what we want.

Now, maybe we should change importNode not to maintain object identity, but for
the time being I've taken the more conservative approach of having SkyElement
create the elements it wants explicitly.

[abarth-chromium, 2015-02-21 17:05:31]

This appears to work the same way in the web:

http://jsbin.com/tacepidigo/1/edit?html,console,output

<!DOCTYPE html>
<html>
<body>
<script>
var MyElementProto = Object.create(HTMLElement.prototype);
MyElementProto.hello = function() {
    alert('Hello!');
}

var MyElement = document.registerElement('my-element',  { 
    prototype: MyElementProto,
});
</script>
<my-element id="foo"></my-element>
<template id="bar"><my-element></my-element></template>
<script>
setTimeout(function() {
  var elmt = document.getElementById('foo');
  console.log(elmt.tagName);
  console.log(elmt.hello);
  var tmpl = document.getElementById('bar');
  console.log(tmpl.content.firstChild.tagName);
  console.log(tmpl.content.firstChild.hello);
  var clone = tmpl.content.cloneNode(true).firstChild;
  console.log(clone.tagName);
  console.log(clone.hello);
  document.body.appendChild(clone);
  var adopted = document.body.lastChild;
  console.log(clone == adopted);
  console.log(adopted.tagName);
  console.log(adopted.hello);
  setTimeout(function() {
    console.log(adopted.tagName);
    console.log(adopted.hello);
  });
});
</script>
</body>
</html>

[abarth-chromium, 2015-02-21 17:35:21]

Ah, *importNode*.  Done.  PTAL.

[esprehn, 2015-02-21 17:41:43]

Please remove the Element::setAttributes code before landing, if we want that we
should do it in a separate patch with a test.

Lgtm

[abarth-chromium, 2015-02-21 17:46:40]

New patchsets have been uploaded after l-g-t-m from esprehn@chromium.org

[abarth-chromium, 2015-02-21 17:56:27]

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