Change custom element state in documents without browsing context

Change custom element state in documents without browsing context

The spec was changed to set the custom element state to "undefined"
even if the document does not have browsing context[1].

[1] https://github.com/whatwg/dom/pull/291

BUG=635402
Committed: https://crrev.com/2a60d7660f68e0bae04400346c28604b144f9016
Cr-Commit-Position: refs/heads/master@{#412975}

[kojii, 2016-08-17 11:59:44]

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

[commit-bot: I haz the power, 2016-08-17 11:59:57]

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

[kojii, 2016-08-17 12:04:30]

Description was changed from

==========
cestate

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

to

==========
Change custom element state in documents without browsing context

The spec was changed to set the custom element state to "undefined"
even if the document does not have browsing context[1].

This patch changes Blink behavior to match to the spec.

Tests were also updated.

[1] https://github.com/whatwg/dom/pull/291

BUG=635402
==========

[kojii, 2016-08-17 12:05:15]

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

[kojii, 2016-08-17 12:05:15]

PTAL.

[commit-bot: I haz the power, 2016-08-17 15:50:48]

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

[commit-bot: I haz the power, 2016-08-17 15:50:49]

Dry run: Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[kojii, 2016-08-17 16:02:58]

Description was changed from

==========
Change custom element state in documents without browsing context

The spec was changed to set the custom element state to "undefined"
even if the document does not have browsing context[1].

This patch changes Blink behavior to match to the spec.

Tests were also updated.

[1] https://github.com/whatwg/dom/pull/291

BUG=635402
==========

to

==========
Change custom element state in documents without browsing context

The spec was changed to set the custom element state to "undefined"
even if the document does not have browsing context[1].

This patch changes Blink behavior to match to the spec.

Tests were updated to reflect the spec change.

[1] https://github.com/whatwg/dom/pull/291

BUG=635402
==========

[dominicc (has gone to gerrit), 2016-08-18 04:58:38]

Comments inline.

Maybe these lines of the description are redundant, they are evident from the
diff stat:

"This patch changes Blink ..."

"Tests were updated ..."

https://codereview.chromium.org/2252003002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/custom-elements/spec/create-element-inside-template.html
(right):

https://codereview.chromium.org/2252003002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/custom-elements/spec/create-element-inside-template.html:18:
assert_false(element.matches(':defined'), '!matches(":defined")');
Could we write english prose in the description instead of pseudocode?

https://codereview.chromium.org/2252003002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/custom-elements/spec/selectors/pseudo-class-defined.html
(right):

https://codereview.chromium.org/2252003002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/custom-elements/spec/selectors/pseudo-class-defined.html:34:
// They used to behave differently, but the spec changed in
https://github.com/whatwg/dom/pull/291
Delete this line. Maybe just mentioning that PR in the change description is
enough.

https://codereview.chromium.org/2252003002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/custom-elements/spec/selectors/pseudo-class-defined.html:58:
if (!defined && doc.defaultView) {
I think these tests would be clearer without this function. What if
doc.defaultView is buggy? Then the tests will pass. Seems risky since the code
under test and the test code depended on having a browsing context.

Could you put a TODO to simplify this?

Can you add assertions for when the caller passes defined but there is no
defaultView?

[kojii, 2016-08-18 05:25:27]

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

[commit-bot: I haz the power, 2016-08-18 05:25:42]

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

[kojii, 2016-08-18 05:26:47]

All done, PTAL.

https://codereview.chromium.org/2252003002/diff/1/third_party/WebKit/LayoutTe...
File
third_party/WebKit/LayoutTests/custom-elements/spec/selectors/pseudo-class-defined.html
(right):

https://codereview.chromium.org/2252003002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/custom-elements/spec/selectors/pseudo-class-defined.html:58:
if (!defined && doc.defaultView) {
Made whether to test value change or not explicit. Does this work?

[kojii, 2016-08-18 07:11:24]

Description was changed from

==========
Change custom element state in documents without browsing context

The spec was changed to set the custom element state to "undefined"
even if the document does not have browsing context[1].

This patch changes Blink behavior to match to the spec.

Tests were updated to reflect the spec change.

[1] https://github.com/whatwg/dom/pull/291

BUG=635402
==========

to

==========
Change custom element state in documents without browsing context

The spec was changed to set the custom element state to "undefined"
even if the document does not have browsing context[1].

[1] https://github.com/whatwg/dom/pull/291

BUG=635402
==========

[commit-bot: I haz the power, 2016-08-18 07:42:55]

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

[commit-bot: I haz the power, 2016-08-18 07:42:56]

Dry run: This issue passed the CQ dry run.

[dominicc (has gone to gerrit), 2016-08-18 23:51:27]

The CQ bit was checked by dominicc@chromium.org

[dominicc (has gone to gerrit), 2016-08-18 23:51:28]

lgtm

Yeah, making it explicit is better. Thanks!

[commit-bot: I haz the power, 2016-08-18 23:52:17]

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-08-18 23:57:11]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-18 23:59:34]

Description was changed from

==========
Change custom element state in documents without browsing context

The spec was changed to set the custom element state to "undefined"
even if the document does not have browsing context[1].

[1] https://github.com/whatwg/dom/pull/291

BUG=635402
==========

to

==========
Change custom element state in documents without browsing context

The spec was changed to set the custom element state to "undefined"
even if the document does not have browsing context[1].

[1] https://github.com/whatwg/dom/pull/291

BUG=635402

Committed: https://crrev.com/2a60d7660f68e0bae04400346c28604b144f9016
Cr-Commit-Position: refs/heads/master@{#412975}
==========

[commit-bot: I haz the power, 2016-08-18 23:59:35]

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