Require that the correct MatchResult::Finish*-calls take place

If the calls to MatchResult::Finish are not correct, we can end up
rules that appear to exist in the wrong origin.

In CollectPseudoRulesForElement, there is an option _not_ match
UA/User rules, but it didn't call the Finish* functions for
skipped origins.

In PseudoStyleForElementInternal, there is a call to
FinishAddingAuthorRulesForTreeScope with _may_ take place without the
other finishers being called first. Since no further rules are added
anyway, we can simply remove this call.

Change-Id: I81e54b267eba6098d9037d2d403047bc3da25dfa
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2310338Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Cr-Commit-Position: refs/heads/master@{#790995}

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

@@ -63,11 +63,13 @@ void MatchResult::AddMatchedProperties(
 }
 
 void MatchResult::FinishAddingUARules() {
+  DCHECK_EQ(current_origin_, CascadeOrigin::kUserAgent);
   current_origin_ = CascadeOrigin::kUser;
   ua_range_end_ = matched_properties_.size();
 }
 
 void MatchResult::FinishAddingUserRules() {
+  DCHECK_EQ(current_origin_, CascadeOrigin::kUser);
   current_origin_ = CascadeOrigin::kAuthor;
   // Don't add empty ranges.
   if (user_range_ends_.IsEmpty() &&
@@ -81,6 +83,7 @@ void MatchResult::FinishAddingUserRules() {
 }
 
 void MatchResult::FinishAddingAuthorRulesForTreeScope() {
+  DCHECK_EQ(current_origin_, CascadeOrigin::kAuthor);
   // Don't add empty ranges.
   if (author_range_ends_.IsEmpty() && user_range_ends_.IsEmpty() &&
       ua_range_end_ == matched_properties_.size())

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

@@ -1065,7 +1065,6 @@ bool StyleResolver::PseudoStyleForElementInternal(
       MatchUserRules(collector);
       MatchAuthorRules(state.GetElement(), collector);
     }
-    collector.FinishAddingAuthorRulesForTreeScope();
 
     if (tracker_)
       AddMatchedRulesToTracker(collector);
@@ -1281,9 +1280,13 @@ void StyleResolver::CollectPseudoRulesForElement(
 
   if (rules_to_include & kUACSSRules)
     MatchUARules(element, collector);
+  else
+    collector.FinishAddingUARules();
 
   if (rules_to_include & kUserCSSRules)
     MatchUserRules(collector);
+  else
+    collector.FinishAddingUserRules();
 
   if (rules_to_include & kAuthorCSSRules) {
     collector.SetSameOriginOnly(!(rules_to_include & kCrossOriginCSSRules));

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

@@ -630,4 +630,28 @@ TEST_F(StyleResolverTest, ApplyInheritedOnlyCustomPropertyChange) {
   EXPECT_EQ("20px", ComputedValue("width", *StyleForId("child2")));
 }
 
+TEST_F(StyleResolverTest, CssRulesForElementIncludedRules) {
+  UpdateAllLifecyclePhasesForTest();
+
+  Element* body = GetDocument().body();
+  ASSERT_TRUE(body);
+
+  // Don't crash when only getting one type of rule.
+  auto& resolver = GetDocument().GetStyleResolver();
+  resolver.CssRulesForElement(body, StyleResolver::kUACSSRules);
+  resolver.CssRulesForElement(body, StyleResolver::kUserCSSRules);
+  resolver.CssRulesForElement(body, StyleResolver::kAuthorCSSRules);
+}
+
+TEST_F(StyleResolverTest, NestedPseudoElement) {
+  GetDocument().body()->setInnerHTML(R"HTML(
+    <style>
+      div::before { content: "Hello"; display: list-item; }
+      div::before::marker { color: green; }
+    </style>
+  )HTML");
+  UpdateAllLifecyclePhasesForTest();
+  // Don't crash when calculating style for nested pseudo elements.
+}
+
 }  // namespace blink