Skip to content

GitLab

T
tangled
Commit 0bc5ba70 authored by Xianzhu Wang's avatar Xianzhu Wang Committed by Commit Bot
Browse files
Options

Revert "Cache animation base computed style only on default StyleForElement()" 

This reverts commit a1fe850a.

Reason for revert: speculative revert for crbug.com/996037.

Original change's description:
> Cache animation base computed style only on default StyleForElement()
> 
> StyleForElement() can be used to get the default style (i.e. the
> style that will be applied on the element), or style derived from
> custom parent style for e.g. inherited first line style. We should
> cache animation base computed style in the first case only.
> 
> Bug: 989490
> Change-Id: Ia30b3e7bcd65cdb9456eb925e5c24f156353bff9
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1753030
> Reviewed-by: Rune Lillesveen <futhark@chromium.org>
> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#686915}

TBR=wangxianzhu@chromium.org,futhark@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 989490,996037
Change-Id: Ib12e4e55e70eaeb2c82ca173c145c225c1d79802
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1763343Reviewed-by: default avatarXianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#689165}
parent 66630f37
Hide whitespace changes
Inline Side-by-side
Showing with 13 additions and 66 deletions
third_party/blink/renderer/core/css/resolver/style_resolver.cc
View file @ 0bc5ba70
......@@ -673,7 +673,7 @@ void StyleResolver::LoadPendingResources(StyleResolverState& state) {
state.GetElementStyleResources().LoadPendingResources(state.Style());
}
static const ComputedStyle* CachedAnimationBaseComputedStyle(
static const ComputedStyle* CalculateBaseComputedStyle(
StyleResolverState& state) {
if (!state.GetAnimatingElement())
return nullptr;
......@@ -694,7 +694,7 @@ static const ComputedStyle* CachedAnimationBaseComputedStyle(
return element_animations->BaseComputedStyle();
}
static void UpdateAnimationBaseComputedStyle(StyleResolverState& state) {
static void UpdateBaseComputedStyle(StyleResolverState& state) {
if (!state.GetAnimatingElement())
return;
......@@ -726,19 +726,10 @@ scoped_refptr<ComputedStyle> StyleResolver::StyleForElement(
StyleResolverState state(GetDocument(), *element, default_parent,
default_layout_parent);
// This function can return different results with different last 3 params,
// but we can only cache one base computed style for animations, thus we cache
// only when this function is called with default last 3 params.
bool can_cache_animation_base_computed_style =
!default_parent && !default_layout_parent &&
matching_behavior == kMatchAllRules;
const ComputedStyle* animation_base_computed_style =
can_cache_animation_base_computed_style
? CachedAnimationBaseComputedStyle(state)
: nullptr;
if (animation_base_computed_style) {
state.SetStyle(ComputedStyle::Clone(*animation_base_computed_style));
const ComputedStyle* base_computed_style = CalculateBaseComputedStyle(state);
if (base_computed_style) {
state.SetStyle(ComputedStyle::Clone(*base_computed_style));
if (!state.ParentStyle()) {
state.SetParentStyle(InitialStyleForElement(GetDocument()));
state.SetLayoutParentStyle(state.ParentStyle());
......@@ -782,7 +773,7 @@ scoped_refptr<ComputedStyle> StyleResolver::StyleForElement(
state.Style()->SetInsideLink(link_state);
}
if (!animation_base_computed_style) {
if (!base_computed_style) {
GetDocument().GetStyleEngine().EnsureUAStyleForElement(*element);
ElementRuleCollector collector(state.ElementContext(), selector_filter_,
......@@ -833,8 +824,7 @@ scoped_refptr<ComputedStyle> StyleResolver::StyleForElement(
StyleAdjuster::AdjustComputedStyle(state, element);
if (can_cache_animation_base_computed_style)
UpdateAnimationBaseComputedStyle(state);
UpdateBaseComputedStyle(state);
} else {
INCREMENT_STYLE_STATS_COUNTER(GetDocument().GetStyleEngine(),
base_styles_used, 1);
......@@ -904,11 +894,10 @@ bool StyleResolver::PseudoStyleForElementInternal(
SelectorFilterParentScope::EnsureParentStackIsPushed();
const ComputedStyle* animation_base_computed_style =
CachedAnimationBaseComputedStyle(state);
const ComputedStyle* base_computed_style = CalculateBaseComputedStyle(state);
if (animation_base_computed_style) {
state.SetStyle(ComputedStyle::Clone(*animation_base_computed_style));
if (base_computed_style) {
state.SetStyle(ComputedStyle::Clone(*base_computed_style));
} else if (pseudo_style_request.AllowsInheritance(state.ParentStyle())) {
scoped_refptr<ComputedStyle> style = ComputedStyle::Create();
style->InheritFrom(*state.ParentStyle());
......@@ -923,7 +912,7 @@ bool StyleResolver::PseudoStyleForElementInternal(
// Since we don't use pseudo-elements in any of our quirk/print
// user agent rules, don't waste time walking those rules.
if (!animation_base_computed_style) {
if (!base_computed_style) {
// Check UA, user and author rules.
ElementRuleCollector collector(state.ElementContext(), selector_filter_,
state.Style());
......@@ -950,7 +939,7 @@ bool StyleResolver::PseudoStyleForElementInternal(
// in the StyleAdjuster::AdjustComputedStyle code.
StyleAdjuster::AdjustComputedStyle(state, nullptr);
UpdateAnimationBaseComputedStyle(state);
UpdateBaseComputedStyle(state);
}
// FIXME: The CSSWG wants to specify that the effects of animations are
......
third_party/blink/renderer/core/css/resolver/style_resolver_test.cc
View file @ 0bc5ba70
......@@ -5,7 +5,6 @@
#include "third_party/blink/renderer/core/css/resolver/style_resolver.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/renderer/core/animation/element_animations.h"
#include "third_party/blink/renderer/core/css/style_engine.h"
#include "third_party/blink/renderer/core/dom/document.h"
#include "third_party/blink/renderer/core/dom/node_computed_style.h"
......@@ -34,45 +33,4 @@ TEST_F(StyleResolverTest, StyleForTextInDisplayNone) {
To<Text>(GetDocument().body()->firstChild())));
}
TEST_F(StyleResolverTest, AnimationBaseComputedStyle) {
GetDocument().documentElement()->SetInnerHTMLFromString(R"HTML(
<style>
html { font-size: 10px; }
body { font-size: 20px; }
</style>
<div id="div">Test</div>
)HTML");
UpdateAllLifecyclePhasesForTest();
Element* div = GetDocument().getElementById("div");
StyleResolver* resolver = GetStyleEngine().Resolver();
ASSERT_TRUE(resolver);
ElementAnimations& animations = div->EnsureElementAnimations();
animations.SetAnimationStyleChange(true);
ASSERT_TRUE(resolver->StyleForElement(div));
EXPECT_EQ(20, resolver->StyleForElement(div)->FontSize());
#if DCHECK_IS_ON()
EXPECT_FALSE(animations.BaseComputedStyle());
#else
ASSERT_TRUE(animations.BaseComputedStyle());
EXPECT_EQ(20, animations.BaseComputedStyle()->FontSize());
#endif
// Getting style with customized parent style should not affect cached
// animation base computed style.
const ComputedStyle* parent_style =
GetDocument().documentElement()->GetComputedStyle();
EXPECT_EQ(
10,
resolver->StyleForElement(div, parent_style, parent_style)->FontSize());
#if DCHECK_IS_ON()
EXPECT_FALSE(animations.BaseComputedStyle());
#else
ASSERT_TRUE(animations.BaseComputedStyle());
EXPECT_EQ(20, animations.BaseComputedStyle()->FontSize());
#endif
EXPECT_EQ(20, resolver->StyleForElement(div)->FontSize());
}
} // 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