Commit 7ca88b5b authored by Hayato Ito's avatar Hayato Ito Committed by Commit Bot

Fix TreeScopeAdopter in case of nested shadow trees moving across documents

https://crrev.com/517033 still misses a case where shadow trees are nested, which was the cause of
DCHECK [1] failure of the test [2]. This CL fixes that.

[1] https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/dom/node.cc?q=MayContainLe&sq=package:chromium&l=773
[2] virtual/incremental-shadow-dom/media/controls/overlay-play-button-document-move.html.

Bug: 776656,783055
Change-Id: I22005d1518768c1750be33a818d25230efa26b76
Reviewed-on: https://chromium-review.googlesource.com/1056927
Commit-Queue: Hayato Ito <hayato@chromium.org>
Reviewed-by: default avatarTakayoshi Kochi <kochi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558243}
parent 54658fe5
......@@ -1724,7 +1724,6 @@ crbug.com/591099 virtual/layout_ng_experimental/external/wpt/css/css-flexbox/ord
crbug.com/776656 virtual/incremental-shadow-dom/external/wpt/shadow-dom/untriaged/styles/test-003.html [ Failure ]
crbug.com/776656 virtual/incremental-shadow-dom/media/controls/controls-cast-do-not-fade-out.html [ Pass Timeout ]
crbug.com/776656 virtual/incremental-shadow-dom/media/controls/overlay-play-button-document-move.html [ Failure Crash ]
# These tests are also failing without the flag
crbug.com/832447 virtual/incremental-shadow-dom/media/controls/controls-page-zoom-in.html [ Failure Pass ]
......
......@@ -82,20 +82,25 @@ void TreeScopeAdopter::MoveTreeToNewScope(Node& root) const {
if (ShadowRoot* shadow = element.GetShadowRoot()) {
shadow->SetParentTreeScope(NewScope());
if (will_move_to_new_document) {
if (shadow->GetType() == ShadowRootType::V0) {
new_document.SetShadowCascadeOrder(
ShadowCascadeOrder::kShadowCascadeV0);
} else if (shadow->IsV1() && !shadow->IsUserAgent()) {
new_document.SetShadowCascadeOrder(
ShadowCascadeOrder::kShadowCascadeV1);
}
MoveTreeToNewDocument(*shadow, old_document, new_document);
}
if (will_move_to_new_document)
MoveShadowTreeToNewDocument(*shadow, old_document, new_document);
}
}
}
void TreeScopeAdopter::MoveShadowTreeToNewDocument(
ShadowRoot& shadow_root,
Document& old_document,
Document& new_document) const {
DCHECK_NE(old_document, new_document);
if (shadow_root.GetType() == ShadowRootType::V0) {
new_document.SetShadowCascadeOrder(ShadowCascadeOrder::kShadowCascadeV0);
} else if (shadow_root.IsV1() && !shadow_root.IsUserAgent()) {
new_document.SetShadowCascadeOrder(ShadowCascadeOrder::kShadowCascadeV1);
}
MoveTreeToNewDocument(shadow_root, old_document, new_document);
}
void TreeScopeAdopter::MoveTreeToNewDocument(Node& root,
Document& old_document,
Document& new_document) const {
......@@ -113,8 +118,8 @@ void TreeScopeAdopter::MoveTreeToNewDocument(Node& root,
MoveTreeToNewDocument(*attr, old_document, new_document);
}
if (ShadowRoot* shadow = element.GetShadowRoot())
MoveTreeToNewDocument(*shadow, old_document, new_document);
if (ShadowRoot* shadow_root = element.GetShadowRoot())
MoveShadowTreeToNewDocument(*shadow_root, old_document, new_document);
}
}
......
......@@ -54,6 +54,9 @@ class CORE_EXPORT TreeScopeAdopter {
void MoveTreeToNewDocument(Node&,
Document& old_document,
Document& new_document) const;
void MoveShadowTreeToNewDocument(ShadowRoot&,
Document& old_document,
Document& new_document) const;
void MoveNodeToNewDocument(Node&,
Document& old_document,
Document& new_document) const;
......
......@@ -11,6 +11,8 @@
namespace blink {
// TODO(hayato): It's hard to see what's happening in these tests.
// It would be better to refactor these tests.
TEST(TreeScopeAdopterTest, SimpleMove) {
Document* doc1 = Document::CreateForTest();
Document* doc2 = Document::CreateForTest();
......@@ -111,4 +113,35 @@ TEST(TreeScopeAdopterTest, AdoptV0ShadowRootToV1Document) {
EXPECT_TRUE(doc2->MayContainV0Shadow());
}
TEST(TreeScopeAdopterTest, AdoptV0InV1ToNewDocument) {
Document* old_doc = Document::CreateForTest();
Element* html = old_doc->CreateRawElement(HTMLNames::htmlTag);
old_doc->AppendChild(html);
Element* host1 = old_doc->CreateRawElement(HTMLNames::divTag);
html->AppendChild(host1);
ShadowRoot& shadow_root_v1 =
host1->AttachShadowRootInternal(ShadowRootType::kOpen);
Element* host2 = old_doc->CreateRawElement(HTMLNames::divTag);
shadow_root_v1.AppendChild(host2);
host2->CreateShadowRootInternal();
// old_doc
// └── html
// └── host1
// └──/shadow-root-v1
// └── host2
// └──/shadow-root-v0
EXPECT_TRUE(old_doc->MayContainV0Shadow());
Document* new_doc = Document::CreateForTest();
EXPECT_FALSE(new_doc->MayContainV0Shadow());
TreeScopeAdopter adopter(*host1, *new_doc);
ASSERT_TRUE(adopter.NeedsScopeChange());
adopter.Execute();
EXPECT_TRUE(old_doc->MayContainV0Shadow());
EXPECT_TRUE(new_doc->MayContainV0Shadow());
}
} // namespace blink
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment