<marquee> in <template> should not crash

<marquee> in <template> should not crash

Currently the following HTML crashes because HTMLMarqueeElement's constructor fails to get a frame and thus cannot install a private script. Note that a template element does not have a frame (a template element just has a document fragment).

<template><marquee>

This CL defers the installation of the private script to the point where we get a frame.

BUG=421002

[haraken, 2014-10-16 07:56:59]

haraken@chromium.org changed reviewers:
+ tasak@google.com

[haraken, 2014-10-16 07:57:05]

PTAL

[tasak, 2014-10-16 08:10:05]

lgtm

[haraken, 2014-10-16 08:10:37]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2014-10-16 08:11:06]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/661693002/60001

[commit-bot: I haz the power, 2014-10-16 09:10:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-10-16 09:10:45]

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/2...)

[adamk, 2014-10-16 16:38:56]

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

[adamk, 2014-10-16 16:38:57]

Can you explain what effect delaying the installation of the private script has?
Does this affect the behavior/availability of methods/attributes on the instance
while it's in a <template>?

[haraken, 2014-10-17 00:52:14]

On 2014/10/16 16:38:57, adamk wrote:
> Can you explain what effect delaying the installation of the private script
has?
> Does this affect the behavior/availability of methods/attributes on the
instance
> while it's in a <template>?

There is no observable effect, because a <marquee> element doesn't do anything
(it's not rendered) while it's in a <template> element. Once the document
fragment of the <template> element is used somewhere, the <marquee> element
starts working. And at that point, the private script is lazily installed.

[adamk, 2014-10-17 16:24:44]

On 2014/10/17 at 00:52:14, haraken wrote:
> On 2014/10/16 16:38:57, adamk wrote:
> > Can you explain what effect delaying the installation of the private script
has?
> > Does this affect the behavior/availability of methods/attributes on the
instance
> > while it's in a <template>?
> 
> There is no observable effect, because a <marquee> element doesn't do anything
(it's not rendered) while it's in a <template> element. Once the document
fragment of the <template> element is used somewhere, the <marquee> element
starts working. And at that point, the private script is lazily installed.

I mean observable from JavaScript. That is, if I have:

<template><marquee></marquee></template>
<script>
var marquee = document.querySelector('template').content.firstChild;
if ('start' in marquee') {
  ... // does this run?
}
marquee.loop = 5; // does this set the 'loop' IDL attribute?
</script>

[haraken, 2014-10-20 06:03:00]

Adamk: Updated the CL, PTAL.

On 2014/10/17 16:24:44, adamk wrote:
> On 2014/10/17 at 00:52:14, haraken wrote:
> > On 2014/10/16 16:38:57, adamk wrote:
> > > Can you explain what effect delaying the installation of the private
script
> has?
> > > Does this affect the behavior/availability of methods/attributes on the
> instance
> > > while it's in a <template>?
> > 
> > There is no observable effect, because a <marquee> element doesn't do
anything
> (it's not rendered) while it's in a <template> element. Once the document
> fragment of the <template> element is used somewhere, the <marquee> element
> starts working. And at that point, the private script is lazily installed.
> 
> I mean observable from JavaScript. That is, if I have:
> 
> <template><marquee></marquee></template>
> <script>
> var marquee = document.querySelector('template').content.firstChild;
> if ('start' in marquee') {
>   ... // does this run?
> }
> marquee.loop = 5; // does this set the 'loop' IDL attribute?
> </script>

Sorry about the confusion -- it just works.

It just works because installPrivateScript is called when any method implemented
in JS is going to be invoked at the first time. This means that we don't need to
call installPrivateScript in HTMLMarqueeElement.cpp at all. What we need in
HTMLMarqueeElement.cpp is just to delay calling createdCallback(). The latest CL
does this.

You still cannot call marquee.start() (nothing will happen), but this behavior
makes sense because the marquee element is not yet associated with any frame.

In other words, as long as the marquee element is in a template element, it
doesn't need to animate. Once the marquee element is associated with a frame, it
should start to animate. This corresponds to the latest implementation where
createdCallback() is invoked when the marquee element is associated with a
frame.

[adamk, 2014-10-20 17:51:59]

On 2014/10/20 at 06:03:00, haraken wrote:
> Adamk: Updated the CL, PTAL.

Where did the layout test go?

> On 2014/10/17 16:24:44, adamk wrote:
> > On 2014/10/17 at 00:52:14, haraken wrote:
> > > On 2014/10/16 16:38:57, adamk wrote:
> > > > Can you explain what effect delaying the installation of the private
script
> > has?
> > > > Does this affect the behavior/availability of methods/attributes on the
> > instance
> > > > while it's in a <template>?
> > > 
> > > There is no observable effect, because a <marquee> element doesn't do
anything
> > (it's not rendered) while it's in a <template> element. Once the document
> > fragment of the <template> element is used somewhere, the <marquee> element
> > starts working. And at that point, the private script is lazily installed.
> > 
> > I mean observable from JavaScript. That is, if I have:
> > 
> > <template><marquee></marquee></template>
> > <script>
> > var marquee = document.querySelector('template').content.firstChild;
> > if ('start' in marquee') {
> >   ... // does this run?
> > }
> > marquee.loop = 5; // does this set the 'loop' IDL attribute?
> > </script>
> 
> Sorry about the confusion -- it just works.
> 
> It just works because installPrivateScript is called when any method
implemented in JS is going to be invoked at the first time. This means that we
don't need to call installPrivateScript in HTMLMarqueeElement.cpp at all. What
we need in HTMLMarqueeElement.cpp is just to delay calling createdCallback().
The latest CL does this.

Ah, this sounds similar to what Polymer calls the "ready" callback (it's only
called for elements that are actually part of a non-template document).

But I'm not sure your insertedInto code is correct. It can result in the
createdCallback not being called before attachedCallback. The following test
case would result in that:

<template><marquee></marquee></template>
<script>
var template = document.querySelector('template');
var templateDoc = template.content.ownerDocument;
templateDoc.appendChild(template.content.firstChild);
</script>

in this case, <marquee> is in a document, but that document still does not have
a frame.

> You still cannot call marquee.start() (nothing will happen), but this behavior
makes sense because the marquee element is not yet associated with any frame.
> 
> In other words, as long as the marquee element is in a template element, it
doesn't need to animate. Once the marquee element is associated with a frame, it
should start to animate. This corresponds to the latest implementation where
createdCallback() is invoked when the marquee element is associated with a
frame.

[haraken, 2014-10-21 12:00:38]

On 2014/10/20 17:51:59, adamk wrote:
> On 2014/10/20 at 06:03:00, haraken wrote:
> > Adamk: Updated the CL, PTAL.
> 
> Where did the layout test go?
> 
> > On 2014/10/17 16:24:44, adamk wrote:
> > > On 2014/10/17 at 00:52:14, haraken wrote:
> > > > On 2014/10/16 16:38:57, adamk wrote:
> > > > > Can you explain what effect delaying the installation of the private
> script
> > > has?
> > > > > Does this affect the behavior/availability of methods/attributes on
the
> > > instance
> > > > > while it's in a <template>?
> > > > 
> > > > There is no observable effect, because a <marquee> element doesn't do
> anything
> > > (it's not rendered) while it's in a <template> element. Once the document
> > > fragment of the <template> element is used somewhere, the <marquee>
element
> > > starts working. And at that point, the private script is lazily installed.
> > > 
> > > I mean observable from JavaScript. That is, if I have:
> > > 
> > > <template><marquee></marquee></template>
> > > <script>
> > > var marquee = document.querySelector('template').content.firstChild;
> > > if ('start' in marquee') {
> > >   ... // does this run?
> > > }
> > > marquee.loop = 5; // does this set the 'loop' IDL attribute?
> > > </script>
> > 
> > Sorry about the confusion -- it just works.
> > 
> > It just works because installPrivateScript is called when any method
> implemented in JS is going to be invoked at the first time. This means that we
> don't need to call installPrivateScript in HTMLMarqueeElement.cpp at all. What
> we need in HTMLMarqueeElement.cpp is just to delay calling createdCallback().
> The latest CL does this.
> 
> Ah, this sounds similar to what Polymer calls the "ready" callback (it's only
> called for elements that are actually part of a non-template document).
> 
> But I'm not sure your insertedInto code is correct. It can result in the
> createdCallback not being called before attachedCallback. The following test
> case would result in that:
> 
> <template><marquee></marquee></template>
> <script>
> var template = document.querySelector('template');
> var templateDoc = template.content.ownerDocument;
> templateDoc.appendChild(template.content.firstChild);
> </script>
> 
> in this case, <marquee> is in a document, but that document still does not
have
> a frame.

Thanks for the details. Hmm, I'm actually not sure what's a right thing to do
here.

The reason we cannot call createdCallback when the <marquee> element is actually
created is that we don't know the frame on which the private script runs. What
happens if we use the frame to which the <template> element belongs (instead of
delaying calling createdCallback)?

[adamk, 2014-10-21 20:07:08]

On 2014/10/21 at 12:00:38, haraken wrote:
> On 2014/10/20 17:51:59, adamk wrote:
> > On 2014/10/20 at 06:03:00, haraken wrote:
> > > Adamk: Updated the CL, PTAL.
> > 
> > Where did the layout test go?
> > 
> > > On 2014/10/17 16:24:44, adamk wrote:
> > > > On 2014/10/17 at 00:52:14, haraken wrote:
> > > > > On 2014/10/16 16:38:57, adamk wrote:
> > > > > > Can you explain what effect delaying the installation of the private
> > script
> > > > has?
> > > > > > Does this affect the behavior/availability of methods/attributes on
the
> > > > instance
> > > > > > while it's in a <template>?
> > > > > 
> > > > > There is no observable effect, because a <marquee> element doesn't do
> > anything
> > > > (it's not rendered) while it's in a <template> element. Once the
document
> > > > fragment of the <template> element is used somewhere, the <marquee>
element
> > > > starts working. And at that point, the private script is lazily
installed.
> > > > 
> > > > I mean observable from JavaScript. That is, if I have:
> > > > 
> > > > <template><marquee></marquee></template>
> > > > <script>
> > > > var marquee = document.querySelector('template').content.firstChild;
> > > > if ('start' in marquee') {
> > > >   ... // does this run?
> > > > }
> > > > marquee.loop = 5; // does this set the 'loop' IDL attribute?
> > > > </script>
> > > 
> > > Sorry about the confusion -- it just works.
> > > 
> > > It just works because installPrivateScript is called when any method
> > implemented in JS is going to be invoked at the first time. This means that
we
> > don't need to call installPrivateScript in HTMLMarqueeElement.cpp at all.
What
> > we need in HTMLMarqueeElement.cpp is just to delay calling
createdCallback().
> > The latest CL does this.
> > 
> > Ah, this sounds similar to what Polymer calls the "ready" callback (it's
only
> > called for elements that are actually part of a non-template document).
> > 
> > But I'm not sure your insertedInto code is correct. It can result in the
> > createdCallback not being called before attachedCallback. The following test
> > case would result in that:
> > 
> > <template><marquee></marquee></template>
> > <script>
> > var template = document.querySelector('template');
> > var templateDoc = template.content.ownerDocument;
> > templateDoc.appendChild(template.content.firstChild);
> > </script>
> > 
> > in this case, <marquee> is in a document, but that document still does not
have
> > a frame.
> 
> Thanks for the details. Hmm, I'm actually not sure what's a right thing to do
here.
> 
> The reason we cannot call createdCallback when the <marquee> element is
actually created is that we don't know the frame on which the private script
runs. What happens if we use the frame to which the <template> element belongs
(instead of delaying calling createdCallback)?

<template> isn't the only case where this can happen. Consider:

var otherDocument = document.implementation.createHTMLDocument('title');
otherDocument.body.innerHTML = '<marquee>';
var marquee = otherDocument.querySelector('marquee');

|otherDocument| has no frame associated with it. It seems like this case needs
to do something reasonable too.

There's a very high-level question here...why is private script for elements
tied to a Frame rather than an ExecutionContext?

[haraken, 2014-10-21 23:57:36]

On 2014/10/21 20:07:08, adamk wrote:
> On 2014/10/21 at 12:00:38, haraken wrote:
> > On 2014/10/20 17:51:59, adamk wrote:
> > > On 2014/10/20 at 06:03:00, haraken wrote:
> > > > Adamk: Updated the CL, PTAL.
> > > 
> > > Where did the layout test go?
> > > 
> > > > On 2014/10/17 16:24:44, adamk wrote:
> > > > > On 2014/10/17 at 00:52:14, haraken wrote:
> > > > > > On 2014/10/16 16:38:57, adamk wrote:
> > > > > > > Can you explain what effect delaying the installation of the
private
> > > script
> > > > > has?
> > > > > > > Does this affect the behavior/availability of methods/attributes
on
> the
> > > > > instance
> > > > > > > while it's in a <template>?
> > > > > > 
> > > > > > There is no observable effect, because a <marquee> element doesn't
do
> > > anything
> > > > > (it's not rendered) while it's in a <template> element. Once the
> document
> > > > > fragment of the <template> element is used somewhere, the <marquee>
> element
> > > > > starts working. And at that point, the private script is lazily
> installed.
> > > > > 
> > > > > I mean observable from JavaScript. That is, if I have:
> > > > > 
> > > > > <template><marquee></marquee></template>
> > > > > <script>
> > > > > var marquee = document.querySelector('template').content.firstChild;
> > > > > if ('start' in marquee') {
> > > > >   ... // does this run?
> > > > > }
> > > > > marquee.loop = 5; // does this set the 'loop' IDL attribute?
> > > > > </script>
> > > > 
> > > > Sorry about the confusion -- it just works.
> > > > 
> > > > It just works because installPrivateScript is called when any method
> > > implemented in JS is going to be invoked at the first time. This means
that
> we
> > > don't need to call installPrivateScript in HTMLMarqueeElement.cpp at all.
> What
> > > we need in HTMLMarqueeElement.cpp is just to delay calling
> createdCallback().
> > > The latest CL does this.
> > > 
> > > Ah, this sounds similar to what Polymer calls the "ready" callback (it's
> only
> > > called for elements that are actually part of a non-template document).
> > > 
> > > But I'm not sure your insertedInto code is correct. It can result in the
> > > createdCallback not being called before attachedCallback. The following
test
> > > case would result in that:
> > > 
> > > <template><marquee></marquee></template>
> > > <script>
> > > var template = document.querySelector('template');
> > > var templateDoc = template.content.ownerDocument;
> > > templateDoc.appendChild(template.content.firstChild);
> > > </script>
> > > 
> > > in this case, <marquee> is in a document, but that document still does not
> have
> > > a frame.
> > 
> > Thanks for the details. Hmm, I'm actually not sure what's a right thing to
do
> here.
> > 
> > The reason we cannot call createdCallback when the <marquee> element is
> actually created is that we don't know the frame on which the private script
> runs. What happens if we use the frame to which the <template> element belongs
> (instead of delaying calling createdCallback)?
> 
> <template> isn't the only case where this can happen. Consider:
> 
> var otherDocument = document.implementation.createHTMLDocument('title');
> otherDocument.body.innerHTML = '<marquee>';
> var marquee = otherDocument.querySelector('marquee');
> 
> |otherDocument| has no frame associated with it. It seems like this case needs
> to do something reasonable too.
> 
> There's a very high-level question here...why is private script for elements
> tied to a Frame rather than an ExecutionContext?

Thanks, Adam. That's because we need a frame to execute a JavaScript. It would
be possible to make JavaScript executable with ExecutionContext, but it would
require a surgery.

[haraken, 2014-10-22 12:14:27]

I'm sorry for taking a lot of cycles to fix this issue...

I think rhogan's CL (https://codereview.chromium.org/654583003/) is a right fix,
but what do you think?

[adamk, 2014-10-22 17:49:40]

On 2014/10/22 at 12:14:27, haraken wrote:
> I'm sorry for taking a lot of cycles to fix this issue...
> 
> I think rhogan's CL (https://codereview.chromium.org/654583003/) is a right
fix, but what do you think?

Responded over there. I think we're real close, just want to understand what the
right end point is for this work.