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

Source/core/css/CSSSegmentedFontFace.cpp

 /*
  * Copyright (C) 2008 Apple 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:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 74 matching lines @@
 }
 
 void CSSSegmentedFontFace::addFontFace(PassRefPtr<FontFace> prpFontFace, bool cssConnected)
 {
     RefPtr<FontFace> fontFace = prpFontFace;
     pruneTable();
     fontFace->cssFontFace()->setSegmentedFontFace(this);
     if (cssConnected) {
         m_fontFaces.insertBefore(m_firstNonCssConnectedFace, fontFace);
     } else {
-        FontFaceList::AddResult result = m_fontFaces.add(fontFace);
+        m_fontFaces.add(fontFace);
         if (m_firstNonCssConnectedFace == m_fontFaces.end())
-            m_firstNonCssConnectedFace = result.iterator;
+            m_firstNonCssConnectedFace = m_fontFaces.find(fontFace);

[Inactive, 2014/02/04 14:21:33]

So for the cases where the caller of add() expects an iterator, we have to do an
additional call to find() and thus an extra lookup? This seems unfortunate.

[adamk, 2014/02/04 16:29:34]

This is my objection, too. Many uses of add() are intentionally trying to avoid
extra hash lookups, so while this change might save on binary size it's trading
runtime for that.

Additionally, given the above, it seems your claim in the CL description, that
the returned iterator "is never used as an iterator" is incorrect (in this case,
and in other cases where add() is used to avoid a double hash lookup, the fact
that it's an iterator is used).

     }
 }
 
 void CSSSegmentedFontFace::removeFontFace(PassRefPtr<FontFace> prpFontFace)
 {
     RefPtr<FontFace> fontFace = prpFontFace;
     FontFaceList::iterator it = m_fontFaces.find(fontFace);
     if (it == m_fontFaces.end())
         return;
 
@@ skipping 113 matching lines @@
 {
     Vector<RefPtr<FontFace> > fontFaces;
     for (FontFaceList::const_iterator it = m_fontFaces.begin(); it != m_fontFaces.end(); ++it) {
         if ((*it)->cssFontFace()->ranges().intersectsWith(text))
             fontFaces.append(*it);
     }
     return fontFaces;
 }
 
 }