Change the type of a SVG <use> element's shadow tree from "user agent" to "closed"

Change the type of a SVG <use> element's shadow tree from "user agent" to "closed"

The SVG 2's spec is: https://svgwg.org/svg2-draft/struct.html#UseElement

Because SVG spec has been updated to clarify that SVG's <use> element should
use a shadow tree as its implementation, we must use the *standard* shadow tree
which DOM Standard defines its behavior. That is either an open shadow root or
a closed shadow root. It should not be a user-agent shadow root because it is UA's
internal concept.

Though the SVG spec says nothing about which type of Shadow Root should be
used, this CL chose "closed" because it is more compatible to the behavior of
user-agent shadow root.  Eventually, the spec should clarify the type.


BUG=670572
Review-Url: https://codereview.chromium.org/2752763002
Cr-Commit-Position: refs/heads/master@{#457391}
Committed: https://chromium.googlesource.com/chromium/src/+/17fa68c396c45e6650df5dd2777a0112800ae61d

[hayato, 2017-03-15 10:39:32]

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

[commit-bot: I haz the power, 2017-03-15 10:39:41]

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

[hayato, 2017-03-15 10:49:51]

Description was changed from

==========
Chage the type of a shadow tree used by SVG <use> element from "user agent" to
"closed"

BUG=670572
==========

to

==========
Change the type of a SVG <use> element's shadow tree from "user agent" to
"closed"

The SVG2's spec is: https://svgwg.org/svg2-draft/struct.html#UseElement

Because SVG spec has been updated to clarify that SVG's <use> element should
use a shadow tree as its implementation, we must use the *standard* shadow tree
which DOM Standard defines its behavior. That is either an open shadow root or
a closed shadow root. It should not be a user-agent shadow root because it is
UA's
internal concept.

Though the SVG spec says nothing about which type of Shadow Root should be
used, this CL chose "closed" because it is more compatible to the behavior of
user-agent shadow root.  Eventually, the spec should clarify the type.


BUG=670572
==========

[hayato, 2017-03-15 10:50:04]

Description was changed from

==========
Change the type of a SVG <use> element's shadow tree from "user agent" to
"closed"

The SVG2's spec is: https://svgwg.org/svg2-draft/struct.html#UseElement

Because SVG spec has been updated to clarify that SVG's <use> element should
use a shadow tree as its implementation, we must use the *standard* shadow tree
which DOM Standard defines its behavior. That is either an open shadow root or
a closed shadow root. It should not be a user-agent shadow root because it is
UA's
internal concept.

Though the SVG spec says nothing about which type of Shadow Root should be
used, this CL chose "closed" because it is more compatible to the behavior of
user-agent shadow root.  Eventually, the spec should clarify the type.


BUG=670572
==========

to

==========
Change the type of a SVG <use> element's shadow tree from "user agent" to
"closed"

The SVG 2's spec is: https://svgwg.org/svg2-draft/struct.html#UseElement

Because SVG spec has been updated to clarify that SVG's <use> element should
use a shadow tree as its implementation, we must use the *standard* shadow tree
which DOM Standard defines its behavior. That is either an open shadow root or
a closed shadow root. It should not be a user-agent shadow root because it is
UA's
internal concept.

Though the SVG spec says nothing about which type of Shadow Root should be
used, this CL chose "closed" because it is more compatible to the behavior of
user-agent shadow root.  Eventually, the spec should clarify the type.


BUG=670572
==========

[hayato, 2017-03-15 10:52:40]

hayato@chromium.org changed reviewers:
+ fs@opera.com, pdr@chromium.org

[hayato, 2017-03-15 10:52:41]

PTAL.

I have separate this CL from https://codereview.chromium.org/2742293006

[hayato, 2017-03-15 11:13:00]

https://codereview.chromium.org/2752763002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/SVGUseElement.cpp (left):

https://codereview.chromium.org/2752763002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGUseElement.cpp:438: ShadowRoot*
shadowTreeRootElement = userAgentShadowRoot();
Since ShadowRoot is not an element, it would be better not to include "Element"
here.

[commit-bot: I haz the power, 2017-03-15 12:06:07]

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

[commit-bot: I haz the power, 2017-03-15 12:06:08]

Dry run: This issue passed the CQ dry run.

[fs, 2017-03-15 13:34:45]

LGTM w/ nits

https://codereview.chromium.org/2752763002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/HitTestResult.cpp (right):

https://codereview.chromium.org/2752763002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/HitTestResult.cpp:172: void
HitTestResult::setToShadowHostIfInUserAgentShadowRoot() {
Maybe the method name should be updated to reflect that it no longer only
operates on only UA shadow roots? (setToShadowHostIfInRestrictedShadowRoot or
something maybe?)

https://codereview.chromium.org/2752763002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/HitTestResult.cpp:180: // case. A toolip
does not work in svg's <use> element without this.
Nit: tooltip, also svg's -> SVG's (for consistency; or rewrite to drop the
repetition [preferred])

https://codereview.chromium.org/2752763002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/SVGElement.cpp (right):

https://codereview.chromium.org/2752763002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGElement.cpp:654: (root->type() ==
ShadowRootType::Closed))
Nit: Could you drop the redundant ()?

[hayato, 2017-03-16 03:56:02]

Nit; fixed

[hayato, 2017-03-16 03:56:04]

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

[commit-bot: I haz the power, 2017-03-16 03:56:21]

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

[hayato, 2017-03-16 03:57:34]

Thank you for the review.

https://codereview.chromium.org/2752763002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/HitTestResult.cpp (right):

https://codereview.chromium.org/2752763002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/HitTestResult.cpp:172: void
HitTestResult::setToShadowHostIfInUserAgentShadowRoot() {
Done

https://codereview.chromium.org/2752763002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/HitTestResult.cpp:180: // case. A toolip
does not work in svg's <use> element without this.
Done. I have rewritten it to drop the repetition.

https://codereview.chromium.org/2752763002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/svg/SVGElement.cpp (right):

https://codereview.chromium.org/2752763002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/svg/SVGElement.cpp:654: (root->type() ==
ShadowRootType::Closed))
Good catch. It is redundant. Done.

[commit-bot: I haz the power, 2017-03-16 03:58:10]

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

[commit-bot: I haz the power, 2017-03-16 03:58:11]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[hayato, 2017-03-16 04:00:51]

hayato@chromium.org changed reviewers:
+ tkent@chromium.org

[hayato, 2017-03-16 04:00:52]

tkent@,
Could you take a look for Source/web/ files? That is just renaming; from
setToShadowHostIfInUserAgentShadowRoot to
setToShadowHostIfInRestrictedShadowRoot.

[tkent, 2017-03-16 04:04:41]

lgtm

[hayato, 2017-03-16 05:11:52]

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

[commit-bot: I haz the power, 2017-03-16 05:12:11]

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, 2017-03-16 05:14:11]

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

[commit-bot: I haz the power, 2017-03-16 05:14:12]

Dry run: Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[hayato, 2017-03-16 05:46:19]

Rebased and resolve a conflict in HitTestResult

[hayato, 2017-03-16 05:46:21]

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

[commit-bot: I haz the power, 2017-03-16 05:46:38]

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

[hayato, 2017-03-16 05:50:49]

The CQ bit was unchecked by hayato@chromium.org

[hayato, 2017-03-16 05:50:52]

The CQ bit was checked by hayato@chromium.org

[hayato, 2017-03-16 05:50:52]

The patchset sent to the CQ was uploaded after l-g-t-m from fs@opera.com,
tkent@chromium.org
Link to the patchset: https://codereview.chromium.org/2752763002/#ps40001
(title: "Rebased and resolve a conflict in HitTestResult")

[commit-bot: I haz the power, 2017-03-16 05:51:00]

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, 2017-03-16 06:13:29]

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

[commit-bot: I haz the power, 2017-03-16 06:13:31]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[hayato, 2017-03-16 06:23:59]

The CQ bit was checked by hayato@chromium.org

[commit-bot: I haz the power, 2017-03-16 06:24:12]

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, 2017-03-16 06:59:10]

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

[commit-bot: I haz the power, 2017-03-16 06:59:12]

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

[hayato, 2017-03-16 07:08:35]

The CQ bit was checked by hayato@chromium.org

[commit-bot: I haz the power, 2017-03-16 07:08:53]

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, 2017-03-16 07:14:27]

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

[commit-bot: I haz the power, 2017-03-16 07:14:29]

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

[hayato, 2017-03-16 08:24:55]

The CQ bit was checked by hayato@chromium.org

[commit-bot: I haz the power, 2017-03-16 08:25: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, 2017-03-16 10:11:25]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489652695882950,
"parent_rev": "a98e9b6fd8c5a7b05e519c512383d039188eac17", "commit_rev":
"17fa68c396c45e6650df5dd2777a0112800ae61d"}

[commit-bot: I haz the power, 2017-03-16 10:12:09]

Description was changed from

==========
Change the type of a SVG <use> element's shadow tree from "user agent" to
"closed"

The SVG 2's spec is: https://svgwg.org/svg2-draft/struct.html#UseElement

Because SVG spec has been updated to clarify that SVG's <use> element should
use a shadow tree as its implementation, we must use the *standard* shadow tree
which DOM Standard defines its behavior. That is either an open shadow root or
a closed shadow root. It should not be a user-agent shadow root because it is
UA's
internal concept.

Though the SVG spec says nothing about which type of Shadow Root should be
used, this CL chose "closed" because it is more compatible to the behavior of
user-agent shadow root.  Eventually, the spec should clarify the type.


BUG=670572
==========

to

==========
Change the type of a SVG <use> element's shadow tree from "user agent" to
"closed"

The SVG 2's spec is: https://svgwg.org/svg2-draft/struct.html#UseElement

Because SVG spec has been updated to clarify that SVG's <use> element should
use a shadow tree as its implementation, we must use the *standard* shadow tree
which DOM Standard defines its behavior. That is either an open shadow root or
a closed shadow root. It should not be a user-agent shadow root because it is
UA's
internal concept.

Though the SVG spec says nothing about which type of Shadow Root should be
used, this CL chose "closed" because it is more compatible to the behavior of
user-agent shadow root.  Eventually, the spec should clarify the type.


BUG=670572

Review-Url: https://codereview.chromium.org/2752763002
Cr-Commit-Position: refs/heads/master@{#457391}
Committed:
https://chromium.googlesource.com/chromium/src/+/17fa68c396c45e6650df5dd2777a...
==========

[commit-bot: I haz the power, 2017-03-16 10:12:11]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/17fa68c396c45e6650df5dd2777a...