Fix for input/datalist failure when datalist is created outside document

Fix for input/datalist failure when datalist is created outside document

This patch fixes the failure, where datalist, when created in a shadowroot
outside the document, doesnot behave as expected. This patch changes
HTMLInputElement::resetListAttributeTargetObserver to  update the list
reference when it resets the observer.

TEST=fast/forms/datalist/datalist-inside-shadow-dom.html
BUG=391995
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=178632

[deepak.sa, 2014-07-08 17:10:38]

Please have a look.
Thanks!

[tkent, 2014-07-08 23:34:04]

+keishi

[keishi, 2014-07-09 02:04:37]

lgtm

https://codereview.chromium.org/371413004/diff/1/LayoutTests/fast/forms/datal...
File LayoutTests/fast/forms/datalist/datalist-inside-shadow-dom.html (right):

https://codereview.chromium.org/371413004/diff/1/LayoutTests/fast/forms/datal...
LayoutTests/fast/forms/datalist/datalist-inside-shadow-dom.html:10: 
Could we have a description what this test is checking. Something like,
"Checking if datalist picker element is shown."

https://codereview.chromium.org/371413004/diff/1/LayoutTests/fast/forms/datal...
LayoutTests/fast/forms/datalist/datalist-inside-shadow-dom.html:11:
<host></host>
This element doesn't seemed to be used in this file.

[esprehn, 2014-07-10 05:10:48]

Thanks for fixing this so fast!

[deepak.sa, 2014-07-15 05:39:20]

This patch is working in Release build but not in Debug build, as it is crashing
at ASSERT(!NoEventDispatchAssertion::isEventDispatchForbidden).
Even without this patch, http://jsbin.com/gijugudu/1/edit is crashing in Linux
Debug build.

I think we have to resolve that first, then we can proceed onto this.
Thoughts?
Thanks!

[keishi, 2014-07-15 06:12:13]

https://codereview.chromium.org/371413004/diff/1/LayoutTests/fast/forms/datal...
File LayoutTests/fast/forms/datalist/datalist-inside-shadow-dom-expected.html
(right):

https://codereview.chromium.org/371413004/diff/1/LayoutTests/fast/forms/datal...
LayoutTests/fast/forms/datalist/datalist-inside-shadow-dom-expected.html:5:
<datalist id="lang">
Extra tab.
You can move the datalist before the input and get around the issue for now.
The assertion failure is another issue.

[esprehn, 2014-07-15 06:52:21]

We need to roll this patch out, you've caused security bugs if you made
that assert trip.

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[deepak.sa, 2014-07-16 14:47:37]

Could you take another look?
In the previous approach, ASSERT was failing as
ContainerNode::notifyNodeInternal was initializing assertNoEventDispatch, so i
took listAttributeTargetChanged() call out of the assert and called it from the
outside.

https://codereview.chromium.org/371413004/diff/1/LayoutTests/fast/forms/datal...
File LayoutTests/fast/forms/datalist/datalist-inside-shadow-dom-expected.html
(right):

https://codereview.chromium.org/371413004/diff/1/LayoutTests/fast/forms/datal...
LayoutTests/fast/forms/datalist/datalist-inside-shadow-dom-expected.html:5:
<datalist id="lang">
On 2014/07/15 06:12:13, keishi wrote:
> Extra tab.
> You can move the datalist before the input and get around the issue for now.
> The assertion failure is another issue.

If input is put after datalist, it doesn't work properly even for this test file
also.
So i changed the test file in the next patch.

[esprehn, 2014-07-16 16:23:42]

ContainerNode shouldn't know anything about specific elements, this is not
correct.

[esprehn, 2014-07-16 19:43:50]

https://codereview.chromium.org/371413004/diff/20001/Source/core/dom/Containe...
File Source/core/dom/ContainerNode.cpp (right):

https://codereview.chromium.org/371413004/diff/20001/Source/core/dom/Containe...
Source/core/dom/ContainerNode.cpp:785:
toHTMLInputElement(root).listAttributeTargetChanged();
I think you might want to use didNotifySubtreeInsertionsToDocument()?

[deepak.sa, 2014-07-17 12:14:09]

Patchset 3 uses didNotifySubtreeInsertionsToDocument() to call
listAttributeTargetChanged.

PTAL?

Thanks!

[deepak.sa, 2014-07-21 05:23:49]

Ping!

[esprehn, 2014-07-22 06:10:17]

This lgtm. It might regress the performance on pages with lots of <input>
elements though so we should be careful there.

[deepak.sa, 2014-07-22 07:06:03]

The CQ bit was checked by deepak.sa@samsung.com

[commit-bot: I haz the power, 2014-07-22 07:06:31]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/deepak.sa@samsung.com/371413004/30001

[commit-bot: I haz the power, 2014-07-22 07:58:47]

Change committed as 178632