Implement callback reactions for Custom Elements V1

Implement callback reactions for Custom Elements V1

This patch implements callbacks for Custom Elements V1:
1. Add Hooks to call methods in CustomElement class.
   a. Element calls connected, disconnected, and attributeChanged.
   b. CustomElementDefinition calls on upgrade.
2. CustomElement looks up CustomElementDefinition. Enqueueing reactions
   were moved from CustomElement to CustomElementDefinition.

Implementing ScriptCustomElementDefinition to call JavaScript will be
in the following patch.

BUG=594918
Committed: https://crrev.com/35a2d25bafa5299fdb57cb857605c3d3655cf7be
Cr-Commit-Position: refs/heads/master@{#399496}

[kojii, 2016-06-13 02:24:26]

Description was changed from

==========
callback-ce

BUG=
==========

to

==========
Implement callback reactions for Custom Elements V1

This patch implements callbacks for Custom Elements V1:
1. Add Hooks to call appropriate methods in CustomElement class.
   a. Element calls on connected, disconnected, and attributeChanged.
   b. CustomElementDefinition calls connected and attributeChanged on
      upgrade.
2. CustomElement looks up CustomElementDefinition. Enqueueing reactions
   were moved from CustomElement to CustomElementDefinition.

Implementing ScriptCustomElementDefinition to call JavaScript will be
in the following patch.

BUG=594918
==========

[kojii, 2016-06-13 02:30:53]

Description was changed from

==========
Implement callback reactions for Custom Elements V1

This patch implements callbacks for Custom Elements V1:
1. Add Hooks to call appropriate methods in CustomElement class.
   a. Element calls on connected, disconnected, and attributeChanged.
   b. CustomElementDefinition calls connected and attributeChanged on
      upgrade.
2. CustomElement looks up CustomElementDefinition. Enqueueing reactions
   were moved from CustomElement to CustomElementDefinition.

Implementing ScriptCustomElementDefinition to call JavaScript will be
in the following patch.

BUG=594918
==========

to

==========
Implement callback reactions for Custom Elements V1

This patch implements callbacks for Custom Elements V1:
1. Add Hooks to call methods in CustomElement class.
   a. Element calls connected, disconnected, and attributeChanged.
   b. CustomElementDefinition calls on upgrade.
2. CustomElement looks up CustomElementDefinition. Enqueueing reactions
   were moved from CustomElement to CustomElementDefinition.

Implementing ScriptCustomElementDefinition to call JavaScript will be
in the following patch.

BUG=594918
==========

[kojii, 2016-06-13 02:31:09]

kojii@chromium.org changed reviewers:
+ dominicc@chromium.org

[kojii, 2016-06-13 02:31:10]

PTAL.

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/custom/CustomElement.h (left):

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/custom/CustomElement.h:44: static bool
descriptorMayMatch(const Element& element)
Should "getCustomElementState() == CustomElementState::Undefined" do this now? I
suspected this is a tentative code until we get getCustomElementState() stable,
but if I miss something, please let me know and I'll revert this change.

[dominicc (has gone to gerrit), 2016-06-13 07:59:01]

lgtm

This looks very nice. Nits inline.

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/core.gypi (right):

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/core.gypi:2668:
'dom/custom/CustomElementDisconnectedCallbackReaction.cpp',
Order

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/Element.cpp (right):

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/Element.cpp:1434: if
(isUpgradedV0CustomElement())
Maybe this case should come before if (getCustomElementState() ==
CustomElementState::Undefined) and make that the else branch:

if (getCustomElementState() == CustomElementState::Custom) {
  for upgraded v1 elements
else if (isUpgradedV0CustomElement())
  for upgraded v0 elements
else if (getCustomElementState() == CustomElementState::Undefined)
  v1 processing for un-upgraded elements (v0 does not do anything here)

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/custom/CustomElement.cpp (right):

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/custom/CustomElement.cpp:147:
DCHECK(definition);
We will null deref right after this, so this DCHECK does not add much. Also line
156. Just let it run (and crash.)

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/custom/CustomElement.h (left):

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/custom/CustomElement.h:44: static bool
descriptorMayMatch(const Element& element)
On 2016/06/13 at 02:31:10, kojii wrote:
> Should "getCustomElementState() == CustomElementState::Undefined" do this now?

Yep, sounds good! We don't need this any more. Yay.

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/custom/CustomElement.h (right):

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/custom/CustomElement.h:50: 
Nit: Delete?

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/dom/custom/CustomElementAttributeChangedCallbackReaction.h
(right):

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/custom/CustomElementAttributeChangedCallbackReaction.h:22:
const AtomicString& oldValue, const AtomicString& newValue);
Guess this should include AtomicString, even if QualifiedName coincidentally
does?

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/custom/CustomElementDefinition.cpp
(right):

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/custom/CustomElementDefinition.cpp:34: //
TODO(kojii): This should be reversed by exposing observedAttributes from
Could we put the observed attributes set on the core/dom side of the definition?

It is assuming we don't have a special attribute that wants to observe *every*
attribute, but I don't think anything like that exists in HTML today?

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/custom/CustomElementDefinition.h
(right):

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/custom/CustomElementDefinition.h:53: virtual
void runAttributeChangedCallback(Element*, const QualifiedName&, const
AtomicString& oldValue, const AtomicString& newValue) {}
I have been trying to keep these files to 80 cols.

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/custom/CustomElementsRegistryTest.cpp
(right):

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/custom/CustomElementsRegistryTest.cpp:271:
uint32_t m_invocationCount;
WDYT about making this vector<string> or something, so we can write assertions
that show the order of the callbacks and the data in them? The problem with
counts is that it is hard to assert the relative ordering of attribute changed
versus connected, etc.

[kojii, 2016-06-13 14:56:42]

Done, or commented ("done" comments omitted)

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/custom/CustomElement.h (right):

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/custom/CustomElement.h:50: 
On 2016/06/13 at 07:59:01, dominicc wrote:
> Nit: Delete?

This is to be deleted in another CL ;)

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/core/dom/custom/CustomElementAttributeChangedCallbackReaction.h
(right):

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/custom/CustomElementAttributeChangedCallbackReaction.h:22:
const AtomicString& oldValue, const AtomicString& newValue);
On 2016/06/13 at 07:59:01, dominicc wrote:
> Guess this should include AtomicString, even if QualifiedName coincidentally
does?

Other arguments are localName and namespaceURI, so the QualifiedName should
cover both...or did I miss something?

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/dom/custom/CustomElementDefinition.cpp
(right):

https://codereview.chromium.org/2058823002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/dom/custom/CustomElementDefinition.cpp:34: //
TODO(kojii): This should be reversed by exposing observedAttributes from
On 2016/06/13 at 07:59:01, dominicc wrote:
> Could we put the observed attributes set on the core/dom side of the
definition?
> 
> It is assuming we don't have a special attribute that wants to observe *every*
attribute, but I don't think anything like that exists in HTML today?

Yes, that's the plan, but it touches bindings/*, so I'll do in the next
script-side patch.

[kojii, 2016-06-13 18:46:49]

The CQ bit was checked by kojii@chromium.org

[kojii, 2016-06-13 18:46:49]

The patchset sent to the CQ was uploaded after l-g-t-m from
dominicc@chromium.org
Link to the patchset: https://codereview.chromium.org/2058823002/#ps80001
(title: "dominicc review")

[commit-bot: I haz the power, 2016-06-13 18:48:05]

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

[commit-bot: I haz the power, 2016-06-13 18:56:34]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-06-13 19:02:24]

Description was changed from

==========
Implement callback reactions for Custom Elements V1

This patch implements callbacks for Custom Elements V1:
1. Add Hooks to call methods in CustomElement class.
   a. Element calls connected, disconnected, and attributeChanged.
   b. CustomElementDefinition calls on upgrade.
2. CustomElement looks up CustomElementDefinition. Enqueueing reactions
   were moved from CustomElement to CustomElementDefinition.

Implementing ScriptCustomElementDefinition to call JavaScript will be
in the following patch.

BUG=594918
==========

to

==========
Implement callback reactions for Custom Elements V1

This patch implements callbacks for Custom Elements V1:
1. Add Hooks to call methods in CustomElement class.
   a. Element calls connected, disconnected, and attributeChanged.
   b. CustomElementDefinition calls on upgrade.
2. CustomElement looks up CustomElementDefinition. Enqueueing reactions
   were moved from CustomElement to CustomElementDefinition.

Implementing ScriptCustomElementDefinition to call JavaScript will be
in the following patch.

BUG=594918

Committed: https://crrev.com/35a2d25bafa5299fdb57cb857605c3d3655cf7be
Cr-Commit-Position: refs/heads/master@{#399496}
==========

[commit-bot: I haz the power, 2016-06-13 19:02:25]

Patchset 5 (id:??) landed as
https://crrev.com/35a2d25bafa5299fdb57cb857605c3d3655cf7be
Cr-Commit-Position: refs/heads/master@{#399496}