Move Form Association Code to FormAssociated::associateWith

Moved form association code from constructors to associateWith() in HTMLImageElement, HTMLObjectElement, and HTMLFormControlElement.

HTMLInputElement had some issues with timing for the lazy parsing of input type.
    I moved the call to Element::parserDidSetAttribute outside Element::parserSetAttribute.
    parserDidSetAttribute is called before parserSetAttribute in XMLDocumentParser.

BUG=672359
Committed: https://crrev.com/cf1052fcc3e555fc9cbff68bd0f8d27ec86f3e35
Cr-Commit-Position: refs/heads/master@{#437159}

[yurak, 2016-12-06 12:28:00]

Description was changed from

==========
HTMLFormControlElement


HTMLImageElement and HTMLObjectElement

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

to

==========
Moved form association code from constructor to associateWith in
HTMLImageElement, HTMLObjectElement, and HTMLFormControlElement.

HTMLInputElement had some issues with timing for the lazy parsing of input type.
    I moved the call to Element::parserDidSetAttribute outside
Element::parserSetAttribute.
    parserDidSetAttribute is called before parserSetAttribute in
XMLDocumentParser.

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

[yurak, 2016-12-06 12:28:00]

yurak@google.com changed reviewers:
+ tkent@chromium.org

[yurak, 2016-12-06 12:31:05]

PTL.

I had to change Element::parserDidSetAttribute to public from protected so
XMLDocumentParser could reach it.
Alternatively I could change parserDidSetAttribute back to protected and check
if we're parsing a XML or HTML document in Element::parserSetAttribute. If it's
an XML document it can call parserDidSetAttribute.

Let me know what you think. Thank you!

[tkent, 2016-12-06 13:12:26]

The CQ bit was checked by tkent@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-06 13:12:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-06 14:36:51]

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

[commit-bot: I haz the power, 2016-12-06 14:36:52]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[tkent, 2016-12-06 22:50:33]

> HTMLInputElement had some issues with timing for the lazy parsing of input
type.

What issues did you see?

>    I moved the call to Element::parserDidSetAttribute outside
Element::parserSetAttribute.

We shouldn't do it.

[yurak, 2016-12-07 01:58:25]

On 2016/12/06 at 22:50:33, tkent wrote:
> > HTMLInputElement had some issues with timing for the lazy parsing of input
type.
> 
> What issues did you see?
> 
> >    I moved the call to Element::parserDidSetAttribute outside
Element::parserSetAttribute.
> 
> We shouldn't do it.

For the HTMLInputElement lazy parsing of input type:
m_inputType is set to null in the HTMLInputElement constructor, but we try to us
it in HTMLFormControlElement::associateByParser so there's a seg fault.
Before this change we ran associateByParser while m_inputType had its default
InputType value before the constructor set it to null.

I think we need to either set m_inputtype before we run associateWith or leave
m_inputType as the default value.
I thought leaving m_inputType as the default would interfere with lazy input
type.

[tkent, 2016-12-07 02:07:36]

On 2016/12/07 at 01:58:25, yurak wrote:
> On 2016/12/06 at 22:50:33, tkent wrote:
> > > HTMLInputElement had some issues with timing for the lazy parsing of input
type.
> > 
> > What issues did you see?
> > 
> > >    I moved the call to Element::parserDidSetAttribute outside
Element::parserSetAttribute.
> > 
> > We shouldn't do it.
> 
> For the HTMLInputElement lazy parsing of input type:
> m_inputType is set to null in the HTMLInputElement constructor, but we try to
us it in HTMLFormControlElement::associateByParser so there's a seg fault.
> Before this change we ran associateByParser while m_inputType had its default
InputType value before the constructor set it to null.

Does it crash in HTMLInputElement::radioButtonGroupScope()?

Can we move FormAssociated::associateWith() call after setAttributes() in
HTMLConstructionSite::createHTMLElement?

[yurak, 2016-12-07 07:47:48]

On 2016/12/07 at 02:07:36, tkent wrote:
> On 2016/12/07 at 01:58:25, yurak wrote:
> > On 2016/12/06 at 22:50:33, tkent wrote:
> > > > HTMLInputElement had some issues with timing for the lazy parsing of
input type.
> > > 
> > > What issues did you see?
> > > 
> > > >    I moved the call to Element::parserDidSetAttribute outside
Element::parserSetAttribute.
> > > 
> > > We shouldn't do it.
> > 
> > For the HTMLInputElement lazy parsing of input type:
> > m_inputType is set to null in the HTMLInputElement constructor, but we try
to us it in HTMLFormControlElement::associateByParser so there's a seg fault.
> > Before this change we ran associateByParser while m_inputType had its
default InputType value before the constructor set it to null.
> 
> Does it crash in HTMLInputElement::radioButtonGroupScope()?

Yes.

> Can we move FormAssociated::associateWith() call after setAttributes() in
HTMLConstructionSite::createHTMLElement?

No, there is a problem with form association.
The element's form attribute has to match the form element in its tree,
otherwise it's not associated with a form.

In this test case:
  <form name=alpha>
    <input type="radio" id="a"><br>
    <input type="radio" form=beta id="b"><br>
  </form>
  <form name=beta></form>
b should not be associated with a form, but it's associated with form alpha.

[tkent, 2016-12-07 08:59:02]

> > Does it crash in HTMLInputElement::radioButtonGroupScope()?
> 
> Yes.
> 
> > Can we move FormAssociated::associateWith() call after setAttributes() in
HTMLConstructionSite::createHTMLElement?
> 
> No, there is a problem with form association.
> The element's form attribute has to match the form element in its tree,
otherwise it's not associated with a form.
> 
> In this test case:
>   <form name=alpha>
>     <input type="radio" id="a"><br>
>     <input type="radio" form=beta id="b"><br>
>   </form>
>   <form name=beta></form>
> b should not be associated with a form, but it's associated with form alpha.

ok, then we should check existence of m_inputType in
HTMLInputElement::willChangeForm and didChangeForm, like:

// willChangeForm can be called before m_inputType is set.
if (m_inputType)
  removedFromRadioButtonGroup();
TextControlElement::willChangeForm();

[yurak, 2016-12-07 09:56:18]

> ok, then we should check existence of m_inputType in
HTMLInputElement::willChangeForm and didChangeForm, like:
> 
> // willChangeForm can be called before m_inputType is set.
> if (m_inputType)
>   removedFromRadioButtonGroup();
> TextControlElement::willChangeForm();

Thank you, that's a much cleaner solution than mine.
I made the changes, PTL.

[tkent, 2016-12-07 23:11:11]

Description was changed from

==========
Moved form association code from constructor to associateWith in
HTMLImageElement, HTMLObjectElement, and HTMLFormControlElement.

HTMLInputElement had some issues with timing for the lazy parsing of input type.
    I moved the call to Element::parserDidSetAttribute outside
Element::parserSetAttribute.
    parserDidSetAttribute is called before parserSetAttribute in
XMLDocumentParser.

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

to

==========
Moved form association code from constructors to associateWith() in
HTMLImageElement, HTMLObjectElement, and HTMLFormControlElement.

HTMLInputElement had some issues with timing for the lazy parsing of input type.
    I moved the call to Element::parserDidSetAttribute outside
Element::parserSetAttribute.
    parserDidSetAttribute is called before parserSetAttribute in
XMLDocumentParser.
==========

[tkent, 2016-12-07 23:11:15]

The CQ bit was checked by tkent@chromium.org

[tkent, 2016-12-07 23:11:16]

lgtm

[commit-bot: I haz the power, 2016-12-07 23:11:44]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-08 00:31:32]

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

[commit-bot: I haz the power, 2016-12-08 00:31:33]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[tkent, 2016-12-08 00:46:53]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2016-12-08 00:47:25]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-08 01:53:53]

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

[commit-bot: I haz the power, 2016-12-08 01:53:54]

Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[tkent, 2016-12-08 02:09:31]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2016-12-08 02:10:18]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-08 02:56:30]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1481162971190230,
"parent_rev": "b30e3d97d3a0433bb3774baa53999b55ba2628e2", "commit_rev":
"3f3f05e10b7c481350161bb9702c35d6132d2b2a"}

[commit-bot: I haz the power, 2016-12-08 02:56:55]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-12-08 02:59:06]

Description was changed from

==========
Moved form association code from constructors to associateWith() in
HTMLImageElement, HTMLObjectElement, and HTMLFormControlElement.

HTMLInputElement had some issues with timing for the lazy parsing of input type.
    I moved the call to Element::parserDidSetAttribute outside
Element::parserSetAttribute.
    parserDidSetAttribute is called before parserSetAttribute in
XMLDocumentParser.
==========

to

==========
Moved form association code from constructors to associateWith() in
HTMLImageElement, HTMLObjectElement, and HTMLFormControlElement.

HTMLInputElement had some issues with timing for the lazy parsing of input type.
    I moved the call to Element::parserDidSetAttribute outside
Element::parserSetAttribute.
    parserDidSetAttribute is called before parserSetAttribute in
XMLDocumentParser.

Committed: https://crrev.com/cf1052fcc3e555fc9cbff68bd0f8d27ec86f3e35
Cr-Commit-Position: refs/heads/master@{#437159}
==========

[commit-bot: I haz the power, 2016-12-08 02:59:07]

Patchset 2 (id:??) landed as
https://crrev.com/cf1052fcc3e555fc9cbff68bd0f8d27ec86f3e35
Cr-Commit-Position: refs/heads/master@{#437159}

[yurak, 2016-12-08 03:55:26]

Description was changed from

==========
Moved form association code from constructors to associateWith() in
HTMLImageElement, HTMLObjectElement, and HTMLFormControlElement.

HTMLInputElement had some issues with timing for the lazy parsing of input type.
    I moved the call to Element::parserDidSetAttribute outside
Element::parserSetAttribute.
    parserDidSetAttribute is called before parserSetAttribute in
XMLDocumentParser.

Committed: https://crrev.com/cf1052fcc3e555fc9cbff68bd0f8d27ec86f3e35
Cr-Commit-Position: refs/heads/master@{#437159}
==========

to

==========
Moved form association code from constructors to associateWith() in
HTMLImageElement, HTMLObjectElement, and HTMLFormControlElement.

HTMLInputElement had some issues with timing for the lazy parsing of input type.
    I moved the call to Element::parserDidSetAttribute outside
Element::parserSetAttribute.
    parserDidSetAttribute is called before parserSetAttribute in
XMLDocumentParser.

BUG=672359

Committed: https://crrev.com/cf1052fcc3e555fc9cbff68bd0f8d27ec86f3e35
Cr-Commit-Position: refs/heads/master@{#437159}
==========