Avoid unnecessary updateActiveStyle comparing shadow styles.
Found while checking performance for issue 603621.
Style sharing is done during style recalc at which point we know that
the active style is up-to-date. Instead of using the API for
document.styleSheets, compare active stylesheets in ScopedStyleResolver
directly.
BUG=603621
Review-Url: https://codereview.chromium.org/2610513003
Cr-Commit-Position: refs/heads/master@{#441357}
diff --git a/third_party/WebKit/Source/core/BUILD.gn b/third_party/WebKit/Source/core/BUILD.gn
index 1947213..8ca4a71 100644
--- a/third_party/WebKit/Source/core/BUILD.gn
+++ b/third_party/WebKit/Source/core/BUILD.gn
@@ -1063,6 +1063,7 @@
"css/parser/SizesCalcParserTest.cpp",
"css/resolver/FontBuilderTest.cpp",
"css/resolver/MatchResultTest.cpp",
+ "css/resolver/ScopedStyleResolverTest.cpp",
"css/resolver/SharedStyleFinderTest.cpp",
"dom/AttrTest.cpp",
"dom/CSSSelectorWatchTest.cpp",
diff --git a/third_party/WebKit/Source/core/css/resolver/ScopedStyleResolver.cpp b/third_party/WebKit/Source/core/css/resolver/ScopedStyleResolver.cpp
index b4e489d..373a63e 100644
--- a/third_party/WebKit/Source/core/css/resolver/ScopedStyleResolver.cpp
+++ b/third_party/WebKit/Source/core/css/resolver/ScopedStyleResolver.cpp
@@ -298,6 +298,28 @@
RuleSubSet::create(parentStyleSheet, sheetIndex, ruleSetForScope));
}
+bool ScopedStyleResolver::haveSameStyles(const ScopedStyleResolver* first,
+ const ScopedStyleResolver* second) {
+ // This method will return true if the two resolvers are either both empty, or
+ // if they contain the same active stylesheets by sharing the same
+ // StyleSheetContents. It is used to check if we can share ComputedStyle
+ // between two shadow hosts. This typically works when we have multiple
+ // instantiations of the same web component where the style elements are in
+ // the same order and contain the exact same source string in which case we
+ // will get a cache hit for sharing StyleSheetContents.
+
+ size_t firstCount = first ? first->m_authorStyleSheets.size() : 0;
+ size_t secondCount = second ? second->m_authorStyleSheets.size() : 0;
+ if (firstCount != secondCount)
+ return false;
+ while (firstCount--) {
+ if (first->m_authorStyleSheets[firstCount]->contents() !=
+ second->m_authorStyleSheets[firstCount]->contents())
+ return false;
+ }
+ return true;
+}
+
DEFINE_TRACE(ScopedStyleResolver::RuleSubSet) {
visitor->trace(m_parentStyleSheet);
visitor->trace(m_ruleSet);
diff --git a/third_party/WebKit/Source/core/css/resolver/ScopedStyleResolver.h b/third_party/WebKit/Source/core/css/resolver/ScopedStyleResolver.h
index 2ca625c..c636ea7a 100644
--- a/third_party/WebKit/Source/core/css/resolver/ScopedStyleResolver.h
+++ b/third_party/WebKit/Source/core/css/resolver/ScopedStyleResolver.h
@@ -76,6 +76,8 @@
void setNeedsAppendAllSheets() { m_needsAppendAllSheets = true; }
static void keyframesRulesAdded(const TreeScope&);
static ContainerNode& invalidationRootForTreeScope(const TreeScope&);
+ CORE_EXPORT static bool haveSameStyles(const ScopedStyleResolver*,
+ const ScopedStyleResolver*);
DECLARE_TRACE();
diff --git a/third_party/WebKit/Source/core/css/resolver/ScopedStyleResolverTest.cpp b/third_party/WebKit/Source/core/css/resolver/ScopedStyleResolverTest.cpp
new file mode 100644
index 0000000..689e4b4
--- /dev/null
+++ b/third_party/WebKit/Source/core/css/resolver/ScopedStyleResolverTest.cpp
@@ -0,0 +1,103 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "core/css/resolver/ScopedStyleResolver.h"
+
+#include "core/dom/shadow/ShadowRoot.h"
+#include "core/dom/shadow/ShadowRootInit.h"
+#include "core/frame/FrameView.h"
+#include "core/html/HTMLElement.h"
+#include "core/testing/DummyPageHolder.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace blink {
+
+class ScopedStyleResolverTest : public ::testing::Test {
+ protected:
+ void SetUp() override {
+ m_dummyPageHolder = DummyPageHolder::create(IntSize(800, 600));
+ }
+
+ Document& document() { return m_dummyPageHolder->document(); }
+ StyleEngine& styleEngine() { return document().styleEngine(); }
+ ShadowRoot& attachShadow(Element& host);
+
+ private:
+ std::unique_ptr<DummyPageHolder> m_dummyPageHolder;
+};
+
+ShadowRoot& ScopedStyleResolverTest::attachShadow(Element& host) {
+ ShadowRootInit init;
+ init.setMode("open");
+ ShadowRoot* shadowRoot = host.attachShadow(
+ ScriptState::forMainWorld(document().frame()), init, ASSERT_NO_EXCEPTION);
+ EXPECT_TRUE(shadowRoot);
+ return *shadowRoot;
+}
+
+TEST_F(ScopedStyleResolverTest, HasSameStylesNullNull) {
+ EXPECT_TRUE(ScopedStyleResolver::haveSameStyles(nullptr, nullptr));
+}
+
+TEST_F(ScopedStyleResolverTest, HasSameStylesNullEmpty) {
+ ScopedStyleResolver& resolver = document().ensureScopedStyleResolver();
+ EXPECT_TRUE(ScopedStyleResolver::haveSameStyles(nullptr, &resolver));
+ EXPECT_TRUE(ScopedStyleResolver::haveSameStyles(&resolver, nullptr));
+}
+
+TEST_F(ScopedStyleResolverTest, HasSameStylesEmptyEmpty) {
+ ScopedStyleResolver& resolver = document().ensureScopedStyleResolver();
+ EXPECT_TRUE(ScopedStyleResolver::haveSameStyles(&resolver, &resolver));
+}
+
+TEST_F(ScopedStyleResolverTest, HasSameStylesNonEmpty) {
+ document().body()->setInnerHTML("<div id=host1></div><div id=host2></div>");
+ Element* host1 = document().getElementById("host1");
+ Element* host2 = document().getElementById("host2");
+ ASSERT_TRUE(host1);
+ ASSERT_TRUE(host2);
+ ShadowRoot& root1 = attachShadow(*host1);
+ ShadowRoot& root2 = attachShadow(*host2);
+ root1.setInnerHTML("<style>::slotted(#dummy){color:pink}</style>");
+ root2.setInnerHTML("<style>::slotted(#dummy){color:pink}</style>");
+ document().view()->updateAllLifecyclePhases();
+ EXPECT_TRUE(ScopedStyleResolver::haveSameStyles(
+ &root1.ensureScopedStyleResolver(), &root2.ensureScopedStyleResolver()));
+}
+
+TEST_F(ScopedStyleResolverTest, HasSameStylesDifferentSheetCount) {
+ document().body()->setInnerHTML("<div id=host1></div><div id=host2></div>");
+ Element* host1 = document().getElementById("host1");
+ Element* host2 = document().getElementById("host2");
+ ASSERT_TRUE(host1);
+ ASSERT_TRUE(host2);
+ ShadowRoot& root1 = attachShadow(*host1);
+ ShadowRoot& root2 = attachShadow(*host2);
+ root1.setInnerHTML(
+ "<style>::slotted(#dummy){color:pink}</style><style>div{}</style>");
+ root2.setInnerHTML("<style>::slotted(#dummy){color:pink}</style>");
+ document().view()->updateAllLifecyclePhases();
+ EXPECT_FALSE(ScopedStyleResolver::haveSameStyles(
+ &root1.ensureScopedStyleResolver(), &root2.ensureScopedStyleResolver()));
+}
+
+TEST_F(ScopedStyleResolverTest, HasSameStylesCacheMiss) {
+ document().body()->setInnerHTML("<div id=host1></div><div id=host2></div>");
+ Element* host1 = document().getElementById("host1");
+ Element* host2 = document().getElementById("host2");
+ ASSERT_TRUE(host1);
+ ASSERT_TRUE(host2);
+ ShadowRoot& root1 = attachShadow(*host1);
+ ShadowRoot& root2 = attachShadow(*host2);
+ // Style equality is detected when StyleSheetContents is shared. That is only
+ // the case when the source text is the same. The comparison will fail when
+ // adding an extra space to one of the sheets.
+ root1.setInnerHTML("<style>::slotted(#dummy){color:pink}</style>");
+ root2.setInnerHTML("<style>::slotted(#dummy){ color:pink}</style>");
+ document().view()->updateAllLifecyclePhases();
+ EXPECT_FALSE(ScopedStyleResolver::haveSameStyles(
+ &root1.ensureScopedStyleResolver(), &root2.ensureScopedStyleResolver()));
+}
+
+} // namespace blink
diff --git a/third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp b/third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp
index 43b2518..e29035d 100644
--- a/third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp
+++ b/third_party/WebKit/Source/core/dom/shadow/ElementShadow.cpp
@@ -27,6 +27,7 @@
#include "core/dom/shadow/ElementShadow.h"
#include "core/css/StyleSheetList.h"
+#include "core/css/resolver/ScopedStyleResolver.h"
#include "core/dom/StyleChangeReason.h"
#include "core/dom/shadow/ElementShadowV0.h"
#include "core/frame/Deprecation.h"
@@ -139,17 +140,11 @@
if (!root || !otherRoot)
return false;
- StyleSheetList& list = root->styleSheets();
- StyleSheetList& otherList = otherRoot->styleSheets();
-
- if (list.length() != otherList.length())
+ if (!ScopedStyleResolver::haveSameStyles(
+ root->scopedStyleResolver(), otherRoot->scopedStyleResolver())) {
return false;
-
- for (size_t i = 0; i < list.length(); i++) {
- if (toCSSStyleSheet(list.item(i))->contents() !=
- toCSSStyleSheet(otherList.item(i))->contents())
- return false;
}
+
root = root->olderShadowRoot();
otherRoot = otherRoot->olderShadowRoot();
}
