(Concept patch) Simplify WTF::HashTable::add() return value for size and performance

Source/core/animation/KeyframeEffectModel.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 228 matching lines @@
     if (m_keyframeGroups)
         return;
 
     m_keyframeGroups = adoptPtr(new KeyframeGroupMap);
     const KeyframeVector keyframes = normalizedKeyframes(getFrames());
     for (KeyframeVector::const_iterator keyframeIter = keyframes.begin(); keyframeIter != keyframes.end(); ++keyframeIter) {
         const Keyframe* keyframe = keyframeIter->get();
         PropertySet keyframeProperties = keyframe->properties();
         for (PropertySet::const_iterator propertyIter = keyframeProperties.begin(); propertyIter != keyframeProperties.end(); ++propertyIter) {
             CSSPropertyID property = *propertyIter;
             KeyframeGroupMap::iterator groupIter = m_keyframeGroups->find(property);

[Erik Corry, 2014/02/04 14:27:15]

This might be more elegant if there were a version of find that did not return
an iterator, but rather a pointer-or-null.  Then you can say if (!ptr) ptr =
...add(...);

             if (groupIter == m_keyframeGroups->end()) {
                 KeyframeGroupMap::AddResult result = m_keyframeGroups->add(property, adoptPtr(new PropertySpecificKeyframeGroup));
                 ASSERT(result.isNewEntry);
-                groupIter = result.iterator;
-            }
+                result.iterator->value->appendKeyframe(adoptPtr(
+                new PropertySpecificKeyframe(keyframe->offset(), keyframe->propertyValue(property), keyframe->composite())));

[Erik Corry, 2014/02/04 14:27:15]

Funny indentation, and there's a chunk of repeated code between here and the
else clause.

+            } else {
             groupIter->value->appendKeyframe(adoptPtr(
                 new PropertySpecificKeyframe(keyframe->offset(), keyframe->propertyValue(property), keyframe->composite())));
+            }
         }
     }
 
     // Add synthetic keyframes.
     for (KeyframeGroupMap::iterator iter = m_keyframeGroups->begin(); iter != m_keyframeGroups->end(); ++iter) {
         iter->value->addSyntheticKeyframeIfRequired();
         iter->value->removeRedundantKeyframes();
     }
 }
 
@@ skipping 107 matching lines @@
 
     if ((*before)->offset() == offset)
         return const_cast<CompositableValue*>((*before)->value());
     if ((*after)->offset() == offset)
         return const_cast<CompositableValue*>((*after)->value());
     return BlendedCompositableValue::create((*before)->value(), (*after)->value(),
         (offset - (*before)->offset()) / ((*after)->offset() - (*before)->offset()));
 }
 
 } // namespace