[css-properties-values-api] Remove root-inherited variables properly.

Currently, StyleInheritedVariables::RemoveVariable removes variable data
properly (i.e. it sets the value to nullptr locally), but only nullifies
the registered value if we already have an entry for that variable
locally. This means that if we inherit a variable via the root bucket,
calling ::RemoveVariable will not actually remove the variable.

Fix by setting registered value to nullptr if a non-nullptr value would
have been returned from the root.

R=futhark@chromium.org

Bug: 641877
Change-Id: I78f0442bd0a8eba26e74a8a152b5bee048fbc23c
Reviewed-on: https://chromium-review.googlesource.com/1235724
Commit-Queue: Anders Ruud <andruud@chromium.org>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593135}

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

@@ -72,6 +72,10 @@ class CSSVariableResolverTest : public PageTestBase {
     return CSSVariableParser::ParseDeclarationValue(
         name, tokens, false, *CSSParserContext::Create(GetDocument()));
   }
+
+  const CSSValue* CreatePxValue(double px) {
+    return CSSPrimitiveValue::Create(px, CSSPrimitiveValue::UnitType::kPixels);
+  }
 };
 
 TEST_F(CSSVariableResolverTest, ParseEnvVariable_Missing_NestedVar) {
@@ -277,4 +281,85 @@ TEST_F(CSSVariableResolverTest, NeedsResolutionClearedByResolver) {
   EXPECT_FALSE(state.Style()->NonInheritedVariables()->NeedsResolution());
 }
 
+TEST_F(CSSVariableResolverTest, RemoveInheritedVariableAtRoot) {
+  scoped_refptr<StyleInheritedVariables> inherited_variables_root =
+      StyleInheritedVariables::Create();
+
+  AtomicString name("--prop");
+  const auto* prop = CreateCustomProperty("test");
+  const CSSValue* value = CreatePxValue(10.0);
+  inherited_variables_root->SetVariable(name, prop->Value());
+  inherited_variables_root->SetRegisteredVariable(name, value);
+
+  EXPECT_TRUE(inherited_variables_root->GetVariable(name));
+  EXPECT_TRUE(inherited_variables_root->RegisteredVariable(name));
+
+  inherited_variables_root->RemoveVariable(name);
+
+  EXPECT_FALSE(inherited_variables_root->GetVariable(name));
+  EXPECT_FALSE(inherited_variables_root->RegisteredVariable(name));
+}
+
+TEST_F(CSSVariableResolverTest, RemoveInheritedVariableAtNonRoot) {
+  scoped_refptr<StyleInheritedVariables> inherited_variables_root =
+      StyleInheritedVariables::Create();
+  scoped_refptr<StyleInheritedVariables> inherited_variables =
+      inherited_variables_root->Copy();
+
+  AtomicString name("--prop");
+  const auto* prop = CreateCustomProperty("test");
+  const CSSValue* value = CreatePxValue(10.0);
+  inherited_variables->SetVariable(name, prop->Value());
+  inherited_variables->SetRegisteredVariable(name, value);
+
+  EXPECT_TRUE(inherited_variables->GetVariable(name));
+  EXPECT_TRUE(inherited_variables->RegisteredVariable(name));
+
+  inherited_variables->RemoveVariable(name);
+
+  EXPECT_FALSE(inherited_variables->GetVariable(name));
+  EXPECT_FALSE(inherited_variables->RegisteredVariable(name));
+}
+
+TEST_F(CSSVariableResolverTest, RemoveVariableInheritedViaRoot) {
+  scoped_refptr<StyleInheritedVariables> inherited_variables_root =
+      StyleInheritedVariables::Create();
+
+  AtomicString name("--prop");
+  const auto* prop = CreateCustomProperty("test");
+  const CSSValue* value = CreatePxValue(10.0);
+  inherited_variables_root->SetVariable(name, prop->Value());
+  inherited_variables_root->SetRegisteredVariable(name, value);
+
+  scoped_refptr<StyleInheritedVariables> inherited_variables =
+      inherited_variables_root->Copy();
+
+  EXPECT_TRUE(inherited_variables->GetVariable(name));
+  EXPECT_TRUE(inherited_variables->RegisteredVariable(name));
+
+  inherited_variables->RemoveVariable(name);
+
+  EXPECT_FALSE(inherited_variables->GetVariable(name));
+  EXPECT_FALSE(inherited_variables->RegisteredVariable(name));
+}
+
+TEST_F(CSSVariableResolverTest, RemoveNonInheritedVariable) {
+  std::unique_ptr<StyleNonInheritedVariables> non_inherited_variables =
+      StyleNonInheritedVariables::Create();
+
+  AtomicString name("--prop");
+  const auto* prop = CreateCustomProperty("test");
+  const CSSValue* value = CreatePxValue(10.0);
+  non_inherited_variables->SetVariable(name, prop->Value());
+  non_inherited_variables->SetRegisteredVariable(name, value);
+
+  EXPECT_TRUE(non_inherited_variables->GetVariable(name));
+  EXPECT_TRUE(non_inherited_variables->RegisteredVariable(name));
+
+  non_inherited_variables->RemoveVariable(name);
+
+  EXPECT_FALSE(non_inherited_variables->GetVariable(name));
+  EXPECT_FALSE(non_inherited_variables->RegisteredVariable(name));
+}
+
 }  // namespace blink

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

@@ -71,8 +71,11 @@ const CSSValue* StyleInheritedVariables::RegisteredVariable(
 void StyleInheritedVariables::RemoveVariable(const AtomicString& name) {
   data_.Set(name, nullptr);
   auto iterator = registered_data_.find(name);
-  if (iterator != registered_data_.end())
+  if (iterator != registered_data_.end()) {
     iterator->value = nullptr;
+  } else if (root_ && root_->RegisteredVariable(name)) {
+    SetRegisteredVariable(name, nullptr);
+  }
 }
 
 HashSet<AtomicString> StyleInheritedVariables::GetCustomPropertyNames() const {