Bug 1187784 (part 8) - Replace nsBaseHashtable::EnumerateRead() calls in layout/ with iterators. r=heycam.
authorNicholas Nethercote <nnethercote@mozilla.com>
Thu, 22 Oct 2015 22:48:40 -0700
changeset 269446 588e410c0ddb76ce261c9c0bd344b1ad634fcb32
parent 269445 c24a4d0cd9d1c4e23785feb276870ee6f1f248dc
child 269447 2938e64f6798d3f20f1869428cc7145f5f52507a
push idunknown
push userunknown
push dateunknown
reviewersheycam
bugs1187784
milestone44.0a1
Bug 1187784 (part 8) - Replace nsBaseHashtable::EnumerateRead() calls in layout/ with iterators. r=heycam. This fixes a type bug in CSSVariableDeclarations::MapRuleInfoInto(). The existing code passes aRuleData->mVariables.get(), which has type |CSSVariableDeclarations*|, into the |void*| parameter to EnumerateRead(). It then extracts that in EnumerateVariableForMapRuleInfoInto() via a cast to a different type, |nsDataHashtable<nsStringHashKey, nsString>*|. It's missing an intermediate access of CSSVariableDeclarations::mVariables. It's likely that this hasn't (seemingly) caused problems prior to now because mVariables is the only field in CSSVariableDeclarations, so mVariables->mVariables is at the same address as mVariables.
layout/style/CSSVariableDeclarations.cpp
layout/style/CSSVariableDeclarations.h
--- a/layout/style/CSSVariableDeclarations.cpp
+++ b/layout/style/CSSVariableDeclarations.cpp
@@ -1,9 +1,10 @@
-/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 /* CSS Custom Property assignments for a Declaration at a given priority */
 
 #include "CSSVariableDeclarations.h"
 
@@ -45,30 +46,22 @@ CSSVariableDeclarations::operator=(const
     return *this;
   }
 
   mVariables.Clear();
   CopyVariablesFrom(aOther);
   return *this;
 }
 
-/* static */ PLDHashOperator
-CSSVariableDeclarations::EnumerateVariableForCopy(const nsAString& aName,
-                                                  nsString aValue,
-                                                  void* aData)
-{
-  CSSVariableDeclarations* variables = static_cast<CSSVariableDeclarations*>(aData);
-  variables->mVariables.Put(aName, aValue);
-  return PL_DHASH_NEXT;
-}
-
 void
 CSSVariableDeclarations::CopyVariablesFrom(const CSSVariableDeclarations& aOther)
 {
-  aOther.mVariables.EnumerateRead(EnumerateVariableForCopy, this);
+  for (auto iter = aOther.mVariables.ConstIter(); !iter.Done(); iter.Next()) {
+    mVariables.Put(iter.Key(), iter.UserData());
+  }
 }
 
 bool
 CSSVariableDeclarations::Has(const nsAString& aName) const
 {
   nsString value;
   return mVariables.Get(aName, &value);
 }
@@ -127,86 +120,68 @@ CSSVariableDeclarations::PutUnset(const 
 }
 
 void
 CSSVariableDeclarations::Remove(const nsAString& aName)
 {
   mVariables.Remove(aName);
 }
 
-/* static */ PLDHashOperator
-CSSVariableDeclarations::EnumerateVariableForMapRuleInfoInto(
-                                                         const nsAString& aName,
-                                                         nsString aValue,
-                                                         void* aData)
-{
-  nsDataHashtable<nsStringHashKey, nsString>* variables =
-    static_cast<nsDataHashtable<nsStringHashKey, nsString>*>(aData);
-  if (!variables->Contains(aName)) {
-    variables->Put(aName, aValue);
-  }
-  return PL_DHASH_NEXT;
-}
-
 void
 CSSVariableDeclarations::MapRuleInfoInto(nsRuleData* aRuleData)
 {
   if (!(aRuleData->mSIDs & NS_STYLE_INHERIT_BIT(Variables))) {
     return;
   }
 
   if (!aRuleData->mVariables) {
     aRuleData->mVariables = new CSSVariableDeclarations(*this);
   } else {
-    mVariables.EnumerateRead(EnumerateVariableForMapRuleInfoInto,
-                             aRuleData->mVariables.get());
+    for (auto iter = mVariables.Iter(); !iter.Done(); iter.Next()) {
+      nsDataHashtable<nsStringHashKey, nsString>& variables =
+        aRuleData->mVariables->mVariables;
+      const nsAString& aName = iter.Key();
+      if (!variables.Contains(aName)) {
+        variables.Put(aName, iter.UserData());
+      }
+    }
   }
 }
 
-/* static */ PLDHashOperator
-CSSVariableDeclarations::EnumerateVariableForAddVariablesToResolver(
-                                                         const nsAString& aName,
-                                                         nsString aValue,
-                                                         void* aData)
-{
-  CSSVariableResolver* resolver = static_cast<CSSVariableResolver*>(aData);
-  if (aValue.EqualsLiteral(INITIAL_VALUE)) {
-    // Values of 'initial' are treated the same as an invalid value in the
-    // variable resolver.
-    resolver->Put(aName, EmptyString(),
-                  eCSSTokenSerialization_Nothing,
-                  eCSSTokenSerialization_Nothing,
-                  false);
-  } else if (aValue.EqualsLiteral(INHERIT_VALUE) ||
-             aValue.EqualsLiteral(UNSET_VALUE)) {
-    // Values of 'inherit' and 'unset' don't need any handling, since it means
-    // we just need to keep whatever value is currently in the resolver.
-    // Values of 'inherit' and 'unset' don't need any handling, since it means
-    // we just need to keep whatever value is currently in the resolver.  This
-    // is because the specified variable declarations already have only the
-    // winning declaration for the variable and no longer have any of the
-    // others.
-  } else {
-    // At this point, we don't know what token types are at the start and end
-    // of the specified variable value.  These will be determined later during
-    // the resolving process.
-    resolver->Put(aName, aValue,
-                  eCSSTokenSerialization_Nothing,
-                  eCSSTokenSerialization_Nothing,
-                  false);
-  }
-  return PL_DHASH_NEXT;
-}
-
 void
 CSSVariableDeclarations::AddVariablesToResolver(
                                            CSSVariableResolver* aResolver) const
 {
-  mVariables.EnumerateRead(EnumerateVariableForAddVariablesToResolver,
-                           aResolver);
+  for (auto iter = mVariables.ConstIter(); !iter.Done(); iter.Next()) {
+    const nsAString& name = iter.Key();
+    nsString value = iter.UserData();
+    if (value.EqualsLiteral(INITIAL_VALUE)) {
+      // Values of 'initial' are treated the same as an invalid value in the
+      // variable resolver.
+      aResolver->Put(name, EmptyString(),
+                     eCSSTokenSerialization_Nothing,
+                     eCSSTokenSerialization_Nothing,
+                     false);
+    } else if (value.EqualsLiteral(INHERIT_VALUE) ||
+               value.EqualsLiteral(UNSET_VALUE)) {
+      // Values of 'inherit' and 'unset' don't need any handling, since it means
+      // we just need to keep whatever value is currently in the resolver.  This
+      // is because the specified variable declarations already have only the
+      // winning declaration for the variable and no longer have any of the
+      // others.
+    } else {
+      // At this point, we don't know what token types are at the start and end
+      // of the specified variable value.  These will be determined later during
+      // the resolving process.
+      aResolver->Put(name, value,
+                     eCSSTokenSerialization_Nothing,
+                     eCSSTokenSerialization_Nothing,
+                     false);
+    }
+  }
 }
 
 size_t
 CSSVariableDeclarations::SizeOfIncludingThis(
                                       mozilla::MallocSizeOf aMallocSizeOf) const
 {
   size_t n = aMallocSizeOf(this);
   n += mVariables.ShallowSizeOfExcludingThis(aMallocSizeOf);
--- a/layout/style/CSSVariableDeclarations.h
+++ b/layout/style/CSSVariableDeclarations.h
@@ -125,26 +125,15 @@ public:
 
   size_t SizeOfIncludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;
 
 private:
   /**
    * Adds all the variable declarations from aOther into this object.
    */
   void CopyVariablesFrom(const CSSVariableDeclarations& aOther);
-  static PLDHashOperator EnumerateVariableForCopy(const nsAString& aName,
-                                                  nsString aValue,
-                                                  void* aData);
-  static PLDHashOperator
-    EnumerateVariableForMapRuleInfoInto(const nsAString& aName,
-                                        nsString aValue,
-                                        void* aData);
-  static PLDHashOperator
-    EnumerateVariableForAddVariablesToResolver(const nsAString& aName,
-                                               nsString aValue,
-                                               void* aData);
 
   nsDataHashtable<nsStringHashKey, nsString> mVariables;
 };
 
 } // namespace mozilla
 
 #endif