Re-allow nullptr animating elements

Because of an unintended behavior change in [1], it's no longer possible
to have a nullptr animating element, since StyleResolverState::
GetAnimatingElement will fall back to GetElement() if pseudo_element is
nullptr. Previously (pre-[1]), if the PseudoElement* retrieved by
StyleResolver::PseudoStyleForElement was nullptr, the animating element
would just be nullptr, and not fall back to anything.

This CL restores that behavior by passing PseudoId to a dedicated
StyleResolverState constructor. That way we know whether we're doing a
style resolution for a pseudo element or a regular element, and we can
return the correct thing from GetAnimatingElement.

Also removed the StyleResolverState constructor which accepted a
ElementResolveContext, since it only had one call site which anyway
did the same thing as the other constructor.

[1] https://crrev.com/c/1724680

Bug: 989151,989402,989437
Change-Id: I0aecf0a7a3a857dbd0664d969abeea7ec5852260
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1729250Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#683124}

third_party/blink/renderer/core/animation/animation_test_helper.cc

@@ -78,8 +78,7 @@ void EnsureInterpolatedValueCached(const ActiveInterpolations& interpolations,
   // require our callers to propertly register every animation they pass in
   // here, which the current tests do not do.
   auto style = ComputedStyle::Create();
-  StyleResolverState state(document, *element, nullptr /* pseudo_element */,
-                           style.get(), style.get());
+  StyleResolverState state(document, *element, style.get(), style.get());
   state.SetStyle(style);
   CSSInterpolationTypesMap map(state.GetDocument().GetPropertyRegistry(),
                                state.GetDocument());

third_party/blink/renderer/core/css/resolver/css_variable_resolver_test.cc

@@ -283,7 +283,7 @@ TEST_F(CSSVariableResolverTest, NeedsResolutionClearedByResolver) {
 
   const ComputedStyle* initial = &ComputedStyle::InitialStyle();
   StyleResolverState state(GetDocument(), *GetDocument().documentElement(),
-                           nullptr /* pseudo_element */, initial, initial);
+                           initial, initial);
 
   scoped_refptr<ComputedStyle> style = ComputedStyle::Create();
   style->InheritFrom(*initial);
@@ -471,7 +471,7 @@ TEST_F(CSSVariableResolverTest, CSSWideKeywords) {
 
   const ComputedStyle* initial = &ComputedStyle::InitialStyle();
   StyleResolverState state(GetDocument(), *GetDocument().documentElement(),
-                           nullptr /* pseudo_element */, initial, initial);
+                           initial, initial);
 
   scoped_refptr<ComputedStyle> style = ComputedStyle::Create();
   style->InheritFrom(*initial);

third_party/blink/renderer/core/css/resolver/style_cascade_test.cc

@@ -53,7 +53,7 @@ class TestCascade {
 
  public:
   TestCascade(Document& document, Element* target = nullptr)
-      : state_(document, target ? *target : *document.body(), nullptr),
+      : state_(document, target ? *target : *document.body()),
         cascade_(InitState(state_)) {}
 
   scoped_refptr<ComputedStyle> TakeStyle() { return state_.TakeStyle(); }
@@ -448,7 +448,7 @@ TEST_F(StyleCascadeTest, ApplyCustomProperty) {
 }
 
 TEST_F(StyleCascadeTest, Copy) {
-  StyleResolverState state(GetDocument(), *GetDocument().body(), nullptr);
+  StyleResolverState state(GetDocument(), *GetDocument().body());
 
   TestCascade cascade(GetDocument());
   cascade.Add("--x", "10px");

third_party/blink/renderer/core/css/resolver/style_resolver.cc

@@ -722,10 +722,7 @@ scoped_refptr<ComputedStyle> StyleResolver::StyleForElement(
 
   SelectorFilterParentScope::EnsureParentStackIsPushed();
 
-  ElementResolveContext element_context(*element);
-
-  StyleResolverState state(GetDocument(), element_context,
-                           nullptr /* pseudo_element */, default_parent,
+  StyleResolverState state(GetDocument(), *element, default_parent,
                            default_layout_parent);
 
   const ComputedStyle* base_computed_style = CalculateBaseComputedStyle(state);
@@ -865,8 +862,7 @@ CompositorKeyframeValue* StyleResolver::CreateCompositorKeyframeValueSnapshot(
     const CSSValue* value) {
   // TODO(alancutter): Avoid creating a StyleResolverState just to apply a
   // single value on a ComputedStyle.
-  StyleResolverState state(element.GetDocument(), element,
-                           nullptr /* pseudo_element */, parent_style,
+  StyleResolverState state(element.GetDocument(), element, parent_style,
                            parent_style);
   state.SetStyle(ComputedStyle::Clone(base_style));
   if (value) {
@@ -971,10 +967,9 @@ scoped_refptr<ComputedStyle> StyleResolver::PseudoStyleForElement(
   if (!element)
     return nullptr;
 
-  StyleResolverState state(
-      GetDocument(), *element,
-      element->GetPseudoElement(pseudo_style_request.pseudo_id), parent_style,
-      parent_layout_object_style);
+  StyleResolverState state(GetDocument(), *element,
+                           pseudo_style_request.pseudo_id, parent_style,
+                           parent_layout_object_style);
   if (!PseudoStyleForElementInternal(*element, pseudo_style_request, state)) {
     if (pseudo_style_request.type == PseudoStyleRequest::kForRenderer)
       return nullptr;
@@ -996,8 +991,7 @@ scoped_refptr<const ComputedStyle> StyleResolver::StyleForPage(int page_index) {
     return initial_style;
 
   StyleResolverState state(GetDocument(), *GetDocument().documentElement(),
-                           nullptr /* pseudo_element */, initial_style.get(),
-                           initial_style.get());
+                           initial_style.get(), initial_style.get());
 
   scoped_refptr<ComputedStyle> style = ComputedStyle::Create();
   const ComputedStyle* root_element_style =
@@ -1095,8 +1089,7 @@ void StyleResolver::AddMatchedRulesToTracker(
 StyleRuleList* StyleResolver::StyleRulesForElement(Element* element,
                                                    unsigned rules_to_include) {
   DCHECK(element);
-  StyleResolverState state(GetDocument(), *element,
-                           nullptr /* pseudo_element */);
+  StyleResolverState state(GetDocument(), *element);
   ElementRuleCollector collector(state.ElementContext(), selector_filter_,
                                  state.Style());
   collector.SetMode(SelectorChecker::kCollectingStyleRules);
@@ -1110,8 +1103,7 @@ RuleIndexList* StyleResolver::PseudoCSSRulesForElement(
     PseudoId pseudo_id,
     unsigned rules_to_include) {
   DCHECK(element);
-  StyleResolverState state(GetDocument(), *element,
-                           nullptr /* pseudo_element */);
+  StyleResolverState state(GetDocument(), *element);
   ElementRuleCollector collector(state.ElementContext(), selector_filter_,
                                  state.Style());
   collector.SetMode(SelectorChecker::kCollectingCSSRules);
@@ -2202,8 +2194,7 @@ void StyleResolver::ComputeFont(Element& element,
   };
 
   // TODO(timloh): This is weird, the style is being used as its own parent
-  StyleResolverState state(GetDocument(), element, nullptr /* pseudo_element */,
-                           style, style);
+  StyleResolverState state(GetDocument(), element, style, style);
   state.SetStyle(style);
 
   for (const CSSProperty* property : properties) {

third_party/blink/renderer/core/css/resolver/style_resolver_state.cc

@@ -33,11 +33,12 @@ namespace blink {
 
 StyleResolverState::StyleResolverState(
     Document& document,
-    const ElementResolveContext& element_context,
+    Element& element,
     PseudoElement* pseudo_element,
+    AnimatingElementType animating_element_type,
     const ComputedStyle* parent_style,
     const ComputedStyle* layout_parent_style)
-    : element_context_(element_context),
+    : element_context_(element),
       document_(document),
       style_(nullptr),
       parent_style_(parent_style),
@@ -49,7 +50,8 @@ StyleResolverState::StyleResolverState(
       element_style_resources_(GetElement(),
                                document.DevicePixelRatio(),
                                pseudo_element),
-      pseudo_element_(pseudo_element) {
+      pseudo_element_(pseudo_element),
+      animating_element_type_(animating_element_type) {
   DCHECK(!!parent_style_ == !!layout_parent_style_);
 
   if (!parent_style_) {
@@ -67,12 +69,24 @@ StyleResolverState::StyleResolverState(
 
 StyleResolverState::StyleResolverState(Document& document,
                                        Element& element,
-                                       PseudoElement* pseudo_element,
                                        const ComputedStyle* parent_style,
                                        const ComputedStyle* layout_parent_style)
     : StyleResolverState(document,
-                         ElementResolveContext(element),
-                         pseudo_element,
+                         element,
+                         nullptr /* pseudo_element */,
+                         AnimatingElementType::kElement,
+                         parent_style,
+                         layout_parent_style) {}
+
+StyleResolverState::StyleResolverState(Document& document,
+                                       Element& element,
+                                       PseudoId pseudo_id,
+                                       const ComputedStyle* parent_style,
+                                       const ComputedStyle* layout_parent_style)
+    : StyleResolverState(document,
+                         element,
+                         element.GetPseudoElement(pseudo_id),
+                         AnimatingElementType::kPseudoElement,
                          parent_style,
                          layout_parent_style) {}
 
@@ -193,11 +207,10 @@ StyleResolverState::ParsedPropertiesForPendingSubstitutionCache(
 }
 
 const Element* StyleResolverState::GetAnimatingElement() const {
-  // The animating element may be this element, or the pseudo element we are
-  // resolving style for.
-  DCHECK(!pseudo_element_ ||
-         pseudo_element_->ParentOrShadowHostElement() == GetElement());
-  return pseudo_element_ ? pseudo_element_.Get() : &GetElement();
+  if (animating_element_type_ == AnimatingElementType::kElement)
+    return &GetElement();
+  DCHECK_EQ(AnimatingElementType::kPseudoElement, animating_element_type_);
+  return pseudo_element_;
 }
 
 }  // namespace blink

third_party/blink/renderer/core/css/resolver/style_resolver_state.h

@@ -51,16 +51,15 @@ class CORE_EXPORT StyleResolverState {
   STACK_ALLOCATED();
 
  public:
-  StyleResolverState(Document&,
-                     const ElementResolveContext&,
-                     PseudoElement* pseudo_element,
-                     const ComputedStyle* parent_style,
-                     const ComputedStyle* layout_parent_style);
   StyleResolverState(Document&,
                      Element&,
-                     PseudoElement* pseudo_element,
                      const ComputedStyle* parent_style = nullptr,
                      const ComputedStyle* layout_parent_style = nullptr);
+  StyleResolverState(Document&,
+                     Element&,
+                     PseudoId,
+                     const ComputedStyle* parent_style,
+                     const ComputedStyle* layout_parent_style);
   ~StyleResolverState();
 
   // In FontFaceSet and CanvasRenderingContext2D, we don't have an element to
@@ -180,6 +179,15 @@ class CORE_EXPORT StyleResolverState {
       const cssvalue::CSSPendingSubstitutionValue&) const;
 
  private:
+  enum class AnimatingElementType { kElement, kPseudoElement };
+
+  StyleResolverState(Document&,
+                     Element&,
+                     PseudoElement*,
+                     AnimatingElementType,
+                     const ComputedStyle* parent_style,
+                     const ComputedStyle* layout_parent_style);
+
   CSSToLengthConversionData UnzoomedLengthConversionData(
       const ComputedStyle* font_style) const;
 
@@ -210,7 +218,8 @@ class CORE_EXPORT StyleResolverState {
   std::unique_ptr<CachedUAStyle> cached_ua_style_;
 
   ElementStyleResources element_style_resources_;
-  Member<PseudoElement> pseudo_element_;
+  Member<Element> pseudo_element_;
+  AnimatingElementType animating_element_type_;
 
   mutable HeapHashMap<
       Member<const cssvalue::CSSPendingSubstitutionValue>,

third_party/blink/renderer/core/style/computed_style_test.cc

@@ -515,7 +515,7 @@ TEST(ComputedStyleTest, ApplyColorSchemeLightOnDark) {
       PreferredColorScheme::kDark);
   StyleResolverState state(dummy_page_holder_->GetDocument(),
                            *dummy_page_holder_->GetDocument().documentElement(),
-                           nullptr /* pseudo_element */, initial, initial);
+                           initial, initial);
 
   scoped_refptr<ComputedStyle> style = ComputedStyle::Create();
   state.SetStyle(style);

third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d_state.cc

@@ -336,9 +336,9 @@ sk_sp<PaintFilter> CanvasRenderingContext2DState::GetFilter(
     // Must set font in case the filter uses any font-relative units (em, ex)
     filter_style->SetFont(font_for_filter_);
 
-    StyleResolverState resolver_state(
-        style_resolution_host->GetDocument(), *style_resolution_host,
-        nullptr /* pseudo_element */, filter_style.get(), filter_style.get());
+    StyleResolverState resolver_state(style_resolution_host->GetDocument(),
+                                      *style_resolution_host,
+                                      filter_style.get(), filter_style.get());
     resolver_state.SetStyle(filter_style);
 
     StyleBuilder::ApplyProperty(GetCSSPropertyFilter(), resolver_state,