Commit 78bf2d4c authored by darin@apple.com's avatar darin@apple.com

Cut down on double hashing and code needlessly using hash table iterators

https://bugs.webkit.org/show_bug.cgi?id=120611

Reviewed by Andreas Kling.

Source/WebCore:

Some of these changes are primarily code cleanup, but others could provide
a small code size and speed improvement by avoiding extra hashing.

* Modules/geolocation/Geolocation.cpp:
(WebCore::Geolocation::Watchers::find): Use get instead of find.
(WebCore::Geolocation::Watchers::remove): Use take instead of find.
(WebCore::Geolocation::makeCachedPositionCallbacks): Use the return
value from remove to avoid hashing twice.

* Modules/webaudio/AudioContext.cpp:
(WebCore::AudioContext::addAutomaticPullNode): Use the return value from
add to avoid hashing twice.
(WebCore::AudioContext::removeAutomaticPullNode): Use the return value
from remove to avoid hashing twice.

* Modules/webaudio/AudioNodeInput.cpp:
(WebCore::AudioNodeInput::connect): Use the return value from add to avoid
hashing twice.
(WebCore::AudioNodeInput::disconnect): Use the return value from remove
to avoid hashing twice.

* Modules/webaudio/AudioParam.cpp:
(WebCore::AudioParam::connect): Use the return value from add to avoid
hashing twice.
(WebCore::AudioParam::disconnect): Use the return value from remove to
avoid hashing twice.

* bridge/NP_jsobject.cpp:
(ObjectMap::remove): Use remove instead of find/remove.

* dom/Node.cpp:
(WebCore::Node::~Node): Use the return value from remove instead of
find/remove.

* inspector/InspectorProfilerAgent.cpp:
(WebCore::InspectorProfilerAgent::removeProfile): Remove needless
calls to contains.

* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::removeSubresourceLoader): Use the return
value from remove instead of find/remove.

* loader/ResourceLoadScheduler.cpp:
(WebCore::ResourceLoadScheduler::HostInformation::remove): Use the
return value from remove to avoid hashing twice.

* loader/appcache/ApplicationCacheGroup.cpp:
(WebCore::ApplicationCacheGroup::disassociateDocumentLoader): Use
remove instead of find/remove.
(WebCore::ApplicationCacheGroup::cacheDestroyed): Removed a needless
call to contains to avoid hashing twice. It's fine to do the check
for an empty hash table unconditionally.

* page/DOMWindow.cpp:
(WebCore::addUnloadEventListener): Eliminated a local variable for clarity.
(WebCore::removeUnloadEventListener): Ditto. Also use remove instead
of find/remove.
(WebCore::removeAllUnloadEventListeners): Ditto. Also use removeAll instead
of find/removeAll.
(WebCore::addBeforeUnloadEventListener): Ditto.
(WebCore::removeBeforeUnloadEventListener): Ditto.
(WebCore::removeAllBeforeUnloadEventListeners): Ditto.

* page/FrameView.cpp:
(WebCore::FrameView::removeViewportConstrainedObject): Use the return
value from remove to avoid hashing twice.
(WebCore::FrameView::removeScrollableArea): Use the return value from
remove instead of find/remove.
(WebCore::FrameView::containsScrollableArea): Use && instead of an if
statement in a way that is idiomatic for this kind of function.

* page/Page.cpp:
(WebCore::Page::addRelevantRepaintedObject): Use the return value from
remove instead of find/remove.

* page/PageGroup.cpp:
(WebCore::PageGroup::removeUserScriptsFromWorld): Use remove instead
of find/remove.
(WebCore::PageGroup::removeUserStyleSheetsFromWorld): Use the return
value from remove instead of find/remove.

* page/PerformanceUserTiming.cpp:
(WebCore::clearPeformanceEntries): Removed a needless call to contains.

* platform/graphics/DisplayRefreshMonitor.cpp:
(WebCore::DisplayRefreshMonitor::removeClient): Use the return value
from remove instead of find/remove.
(WebCore::DisplayRefreshMonitorManager::displayDidRefresh): Use remove
instead of find/remove.

* platform/graphics/blackberry/LayerRenderer.cpp:
(WebCore::LayerRenderer::removeLayer): Use the return value from remove
instead of find/remove.

* platform/win/WindowMessageBroadcaster.cpp:
(WebCore::WindowMessageBroadcaster::removeListener): Use remove instead
of find/remove. It's fine to do the check for an empty hash table unconditionally.

* plugins/PluginDatabase.cpp:
(WebCore::PluginDatabase::removeDisabledPluginFile): Use the return value
from remove instead of find/remove.

* rendering/style/StyleCustomFilterProgramCache.cpp:
(WebCore::StyleCustomFilterProgramCache::lookup): Use get instead of find.
(WebCore::StyleCustomFilterProgramCache::add): Use contains instead of find
in an assertion.
(WebCore::StyleCustomFilterProgramCache::remove): Use remove instead of
find/remove.

* svg/SVGCursorElement.cpp:
(WebCore::SVGCursorElement::removeClient): Use the return value from remove
instead of find/remove.

* svg/SVGDocumentExtensions.cpp:
(WebCore::SVGDocumentExtensions::removeResource): Removed an unneeded call
to contains.
(WebCore::SVGDocumentExtensions::removeAllTargetReferencesForElement): Use
remove instead of find/remove. It's fine to do the check for an empty hash
table unconditionally.
(WebCore::SVGDocumentExtensions::removeAllElementReferencesForTarget): Use
remove instead of find/remove. Also removed unhelpful assertions. One is
already done by HashMap, and the other is just checking a basic invariant
of every HashMap that doesn't need to be checked.

* svg/graphics/SVGImageCache.cpp:
(WebCore::SVGImageCache::removeClientFromCache): Removed an unneeded call
to contains.

* svg/properties/SVGAnimatedProperty.cpp:
(WebCore::SVGAnimatedProperty::~SVGAnimatedProperty): Use the version of
remove that takes an iterator rather than the one that takes a key, so
we don't need to redo the hashing.

Source/WebKit2:

* Platform/CoreIPC/Connection.cpp:
(CoreIPC::Connection::waitForMessage): Use take instead of find/remove.

* UIProcess/WebPreferences.cpp:
(WebKit::WebPreferences::removePageGroup): Use the return value from remove
instead of find/remove.

* WebProcess/Geolocation/GeolocationPermissionRequestManager.cpp:
(WebKit::GeolocationPermissionRequestManager::cancelRequestForGeolocation):
(WebKit::GeolocationPermissionRequestManager::didReceiveGeolocationPermissionDecision):
Use take instead of find/remove.

* WebProcess/Plugins/Netscape/NetscapePlugin.cpp:
(WebKit::NetscapePlugin::frameDidFinishLoading): Use take instead of find/remove.
(WebKit::NetscapePlugin::frameDidFail): Use take instead of find/remove.

* WebProcess/WebPage/WebBackForwardListProxy.cpp:
(WebKit::WebBackForwardListProxy::removeItem): Use take instead of find/remove.

* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::didFinishCheckingText): Use take instead of get/remove so we
hash only once.
(WebKit::WebPage::didCancelCheckingText): Ditto.
(WebKit::WebPage::stopExtendingIncrementalRenderingSuppression): Use the return
value from remove instead of contains/remove so we hash only once.

Source/WTF:

Double hashing is common in code that needs to combine a remove with some
action to only be done if the code is removed. The only way to avoid it is
to write code using find and a hash table iterator. To help with this, add
a boolean return value to remove functions to indicate if anything was removed.

Double hashing also happens in code that does a get followed by a remove.
The take function is helpful in this case. To help with this, add a takeFirst
funciton to ListHashSet.

* wtf/HashCountedSet.h:
(WTF::HashCountedSet::removeAll): Added a boolean return value, analogous to the one
that the HashCountedSet::remove function already has.

* wtf/HashMap.h:
(WTF::HashMap::remove): Added a boolean return value, true if something was removed.
* wtf/HashSet.h:
(WTF::HashSet::remove): Ditto.
* wtf/RefPtrHashMap.h:
(WTF::RefPtrHashMap::remove): Ditto.

* wtf/ListHashSet.h:
(WTF::ListHashSet::takeFirst): Added.
(WTF::ListHashSet::takeLast): Added.
(WTF::ListHashSet::remove): Added a boolean return value, true if something was removed.

* wtf/WTFThreadData.h:
(JSC::IdentifierTable::remove): Use the new remove return value to get rid of most of
the code in this function.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@154967 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 583efd1e
2013-09-02 Darin Adler <darin@apple.com>
Cut down on double hashing and code needlessly using hash table iterators
https://bugs.webkit.org/show_bug.cgi?id=120611
Reviewed by Andreas Kling.
Double hashing is common in code that needs to combine a remove with some
action to only be done if the code is removed. The only way to avoid it is
to write code using find and a hash table iterator. To help with this, add
a boolean return value to remove functions to indicate if anything was removed.
Double hashing also happens in code that does a get followed by a remove.
The take function is helpful in this case. To help with this, add a takeFirst
funciton to ListHashSet.
* wtf/HashCountedSet.h:
(WTF::HashCountedSet::removeAll): Added a boolean return value, analogous to the one
that the HashCountedSet::remove function already has.
* wtf/HashMap.h:
(WTF::HashMap::remove): Added a boolean return value, true if something was removed.
* wtf/HashSet.h:
(WTF::HashSet::remove): Ditto.
* wtf/RefPtrHashMap.h:
(WTF::RefPtrHashMap::remove): Ditto.
* wtf/ListHashSet.h:
(WTF::ListHashSet::takeFirst): Added.
(WTF::ListHashSet::takeLast): Added.
(WTF::ListHashSet::remove): Added a boolean return value, true if something was removed.
* wtf/WTFThreadData.h:
(JSC::IdentifierTable::remove): Use the new remove return value to get rid of most of
the code in this function.
2013-09-02 David Kilzer <ddkilzer@apple.com>
Remove duplicate entries found by Xcode in WTF project
......
/*
* Copyright (C) 2005, 2006, 2008 Apple Inc. All rights reserved.
* Copyright (C) 2005, 2006, 2008, 2013 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
......@@ -68,8 +68,8 @@ namespace WTF {
bool remove(iterator);
// Removes the value, regardless of its count.
void removeAll(iterator);
void removeAll(const ValueType&);
bool removeAll(iterator);
bool removeAll(const ValueType&);
// Clears the whole set.
void clear();
......@@ -183,18 +183,19 @@ namespace WTF {
}
template<typename Value, typename HashFunctions, typename Traits>
inline void HashCountedSet<Value, HashFunctions, Traits>::removeAll(const ValueType& value)
inline bool HashCountedSet<Value, HashFunctions, Traits>::removeAll(const ValueType& value)
{
removeAll(find(value));
return removeAll(find(value));
}
template<typename Value, typename HashFunctions, typename Traits>
inline void HashCountedSet<Value, HashFunctions, Traits>::removeAll(iterator it)
inline bool HashCountedSet<Value, HashFunctions, Traits>::removeAll(iterator it)
{
if (it == end())
return;
return false;
m_impl.remove(it);
return true;
}
template<typename Value, typename HashFunctions, typename Traits>
......
/*
* Copyright (C) 2005, 2006, 2007, 2008, 2011 Apple Inc. All rights reserved.
* Copyright (C) 2005, 2006, 2007, 2008, 2011, 2013 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
......@@ -106,8 +106,8 @@ namespace WTF {
// and a boolean that's true if a new value was actually added
AddResult add(const KeyType&, MappedPassInType);
void remove(const KeyType&);
void remove(iterator);
bool remove(const KeyType&);
bool remove(iterator);
void clear();
MappedPassOutType take(const KeyType&); // efficient combination of get with remove
......@@ -380,18 +380,19 @@ namespace WTF {
}
template<typename T, typename U, typename V, typename W, typename X>
inline void HashMap<T, U, V, W, X>::remove(iterator it)
inline bool HashMap<T, U, V, W, X>::remove(iterator it)
{
if (it.m_impl == m_impl.end())
return;
return false;
m_impl.internalCheckTableConsistency();
m_impl.removeWithoutEntryConsistencyCheck(it.m_impl);
return true;
}
template<typename T, typename U, typename V, typename W, typename X>
inline void HashMap<T, U, V, W, X>::remove(const KeyType& key)
inline bool HashMap<T, U, V, W, X>::remove(const KeyType& key)
{
remove(find(key));
return remove(find(key));
}
template<typename T, typename U, typename V, typename W, typename X>
......
/*
* Copyright (C) 2005, 2006, 2007, 2008, 2011 Apple Inc. All rights reserved.
* Copyright (C) 2005, 2006, 2007, 2008, 2011, 2013 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
......@@ -88,8 +88,8 @@ namespace WTF {
template<typename IteratorType>
bool add(IteratorType begin, IteratorType end);
void remove(const ValueType&);
void remove(iterator);
bool remove(const ValueType&);
bool remove(iterator);
void clear();
static bool isValidValue(const ValueType&);
......@@ -204,18 +204,19 @@ namespace WTF {
}
template<typename T, typename U, typename V>
inline void HashSet<T, U, V>::remove(iterator it)
inline bool HashSet<T, U, V>::remove(iterator it)
{
if (it.m_impl == m_impl.end())
return;
return false;
m_impl.internalCheckTableConsistency();
m_impl.removeWithoutEntryConsistencyCheck(it.m_impl);
return true;
}
template<typename T, typename U, typename V>
inline void HashSet<T, U, V>::remove(const ValueType& value)
inline bool HashSet<T, U, V>::remove(const ValueType& value)
{
remove(find(value));
return remove(find(value));
}
template<typename T, typename U, typename V>
......
/*
* Copyright (C) 2005, 2006, 2007, 2008, 2011, 2012 Apple Inc. All rights reserved.
* Copyright (C) 2005, 2006, 2007, 2008, 2011, 2012, 2013 Apple Inc. All rights reserved.
* Copyright (C) 2011, Benjamin Poulain <ikipou@gmail.com>
*
* This library is free software; you can redistribute it and/or
......@@ -107,10 +107,12 @@ namespace WTF {
ValueType& first();
const ValueType& first() const;
void removeFirst();
ValueType takeFirst();
ValueType& last();
const ValueType& last() const;
void removeLast();
ValueType takeLast();
iterator find(const ValueType&);
const_iterator find(const ValueType&) const;
......@@ -140,8 +142,8 @@ namespace WTF {
AddResult insertBefore(const ValueType& beforeValue, const ValueType& newValue);
AddResult insertBefore(iterator, const ValueType&);
void remove(const ValueType&);
void remove(iterator);
bool remove(const ValueType&);
bool remove(iterator);
void clear();
private:
......@@ -628,6 +630,14 @@ namespace WTF {
unlinkAndDelete(m_head);
}
template<typename T, size_t inlineCapacity, typename U>
inline T ListHashSet<T, inlineCapacity, U>::takeFirst()
{
T result = first();
removeFirst();
return result;
}
template<typename T, size_t inlineCapacity, typename U>
inline const T& ListHashSet<T, inlineCapacity, U>::first() const
{
......@@ -657,6 +667,14 @@ namespace WTF {
unlinkAndDelete(m_tail);
}
template<typename T, size_t inlineCapacity, typename U>
inline T ListHashSet<T, inlineCapacity, U>::takeLast()
{
T result = last();
removeLast();
return result;
}
template<typename T, size_t inlineCapacity, typename U>
inline typename ListHashSet<T, inlineCapacity, U>::iterator ListHashSet<T, inlineCapacity, U>::find(const ValueType& value)
{
......@@ -761,18 +779,19 @@ namespace WTF {
}
template<typename T, size_t inlineCapacity, typename U>
inline void ListHashSet<T, inlineCapacity, U>::remove(iterator it)
inline bool ListHashSet<T, inlineCapacity, U>::remove(iterator it)
{
if (it == end())
return;
return false;
m_impl.remove(it.node());
unlinkAndDelete(it.node());
return true;
}
template<typename T, size_t inlineCapacity, typename U>
inline void ListHashSet<T, inlineCapacity, U>::remove(const ValueType& value)
inline bool ListHashSet<T, inlineCapacity, U>::remove(const ValueType& value)
{
remove(find(value));
return remove(find(value));
}
template<typename T, size_t inlineCapacity, typename U>
......
/*
* Copyright (C) 2005, 2006, 2007, 2008, 2011 Apple Inc. All rights reserved.
* Copyright (C) 2005, 2006, 2007, 2008, 2011, 2013 Apple Inc. All rights reserved.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
......@@ -96,9 +96,9 @@ namespace WTF {
AddResult add(const KeyType&, MappedPassInType);
AddResult add(RawKeyType, MappedPassInType);
void remove(const KeyType&);
void remove(RawKeyType);
void remove(iterator);
bool remove(const KeyType&);
bool remove(RawKeyType);
bool remove(iterator);
void clear();
MappedPassOutType take(const KeyType&); // efficient combination of get with remove
......@@ -275,24 +275,25 @@ namespace WTF {
}
template<typename T, typename U, typename V, typename W, typename X>
inline void HashMap<RefPtr<T>, U, V, W, X>::remove(iterator it)
inline bool HashMap<RefPtr<T>, U, V, W, X>::remove(iterator it)
{
if (it.m_impl == m_impl.end())
return;
return false;
m_impl.internalCheckTableConsistency();
m_impl.removeWithoutEntryConsistencyCheck(it.m_impl);
return true;
}
template<typename T, typename U, typename V, typename W, typename X>
inline void HashMap<RefPtr<T>, U, V, W, X>::remove(const KeyType& key)
inline bool HashMap<RefPtr<T>, U, V, W, X>::remove(const KeyType& key)
{
remove(find(key));
return remove(find(key));
}
template<typename T, typename U, typename V, typename W, typename X>
inline void HashMap<RefPtr<T>, U, V, W, X>::remove(RawKeyType key)
inline bool HashMap<RefPtr<T>, U, V, W, X>::remove(RawKeyType key)
{
remove(find(key));
return remove(find(key));
}
template<typename T, typename U, typename V, typename W, typename X>
......
......@@ -36,7 +36,7 @@
#include <wtf/ThreadSpecific.h>
#include <wtf/Threading.h>
// FIXME: This is a temporary layering violation while we move more string code to WTF.
// FIXME: This is a temporary layering violation until we move more of the string code from JavaScriptCore to WTF.
namespace JSC {
class IdentifierTable {
......@@ -44,18 +44,10 @@ class IdentifierTable {
public:
WTF_EXPORT_PRIVATE ~IdentifierTable();
WTF_EXPORT_PRIVATE HashSet<StringImpl*>::AddResult add(StringImpl* value);
template<typename U, typename V>
HashSet<StringImpl*>::AddResult add(U value);
WTF_EXPORT_PRIVATE HashSet<StringImpl*>::AddResult add(StringImpl*);
template<typename U, typename V> HashSet<StringImpl*>::AddResult add(U);
bool remove(StringImpl* r)
{
HashSet<StringImpl*>::iterator iter = m_table.find(r);
if (iter == m_table.end())
return false;
m_table.remove(iter);
return true;
}
bool remove(StringImpl* identifier) { return m_table.remove(identifier); }
private:
HashSet<StringImpl*> m_table;
......
2013-09-02 Darin Adler <darin@apple.com>
Cut down on double hashing and code needlessly using hash table iterators
https://bugs.webkit.org/show_bug.cgi?id=120611
Reviewed by Andreas Kling.
Some of these changes are primarily code cleanup, but others could provide
a small code size and speed improvement by avoiding extra hashing.
* Modules/geolocation/Geolocation.cpp:
(WebCore::Geolocation::Watchers::find): Use get instead of find.
(WebCore::Geolocation::Watchers::remove): Use take instead of find.
(WebCore::Geolocation::makeCachedPositionCallbacks): Use the return
value from remove to avoid hashing twice.
* Modules/webaudio/AudioContext.cpp:
(WebCore::AudioContext::addAutomaticPullNode): Use the return value from
add to avoid hashing twice.
(WebCore::AudioContext::removeAutomaticPullNode): Use the return value
from remove to avoid hashing twice.
* Modules/webaudio/AudioNodeInput.cpp:
(WebCore::AudioNodeInput::connect): Use the return value from add to avoid
hashing twice.
(WebCore::AudioNodeInput::disconnect): Use the return value from remove
to avoid hashing twice.
* Modules/webaudio/AudioParam.cpp:
(WebCore::AudioParam::connect): Use the return value from add to avoid
hashing twice.
(WebCore::AudioParam::disconnect): Use the return value from remove to
avoid hashing twice.
* bridge/NP_jsobject.cpp:
(ObjectMap::remove): Use remove instead of find/remove.
* dom/Node.cpp:
(WebCore::Node::~Node): Use the return value from remove instead of
find/remove.
* inspector/InspectorProfilerAgent.cpp:
(WebCore::InspectorProfilerAgent::removeProfile): Remove needless
calls to contains.
* loader/DocumentLoader.cpp:
(WebCore::DocumentLoader::removeSubresourceLoader): Use the return
value from remove instead of find/remove.
* loader/ResourceLoadScheduler.cpp:
(WebCore::ResourceLoadScheduler::HostInformation::remove): Use the
return value from remove to avoid hashing twice.
* loader/appcache/ApplicationCacheGroup.cpp:
(WebCore::ApplicationCacheGroup::disassociateDocumentLoader): Use
remove instead of find/remove.
(WebCore::ApplicationCacheGroup::cacheDestroyed): Removed a needless
call to contains to avoid hashing twice. It's fine to do the check
for an empty hash table unconditionally.
* page/DOMWindow.cpp:
(WebCore::addUnloadEventListener): Eliminated a local variable for clarity.
(WebCore::removeUnloadEventListener): Ditto. Also use remove instead
of find/remove.
(WebCore::removeAllUnloadEventListeners): Ditto. Also use removeAll instead
of find/removeAll.
(WebCore::addBeforeUnloadEventListener): Ditto.
(WebCore::removeBeforeUnloadEventListener): Ditto.
(WebCore::removeAllBeforeUnloadEventListeners): Ditto.
* page/FrameView.cpp:
(WebCore::FrameView::removeViewportConstrainedObject): Use the return
value from remove to avoid hashing twice.
(WebCore::FrameView::removeScrollableArea): Use the return value from
remove instead of find/remove.
(WebCore::FrameView::containsScrollableArea): Use && instead of an if
statement in a way that is idiomatic for this kind of function.
* page/Page.cpp:
(WebCore::Page::addRelevantRepaintedObject): Use the return value from
remove instead of find/remove.
* page/PageGroup.cpp:
(WebCore::PageGroup::removeUserScriptsFromWorld): Use remove instead
of find/remove.
(WebCore::PageGroup::removeUserStyleSheetsFromWorld): Use the return
value from remove instead of find/remove.
* page/PerformanceUserTiming.cpp:
(WebCore::clearPeformanceEntries): Removed a needless call to contains.
* platform/graphics/DisplayRefreshMonitor.cpp:
(WebCore::DisplayRefreshMonitor::removeClient): Use the return value
from remove instead of find/remove.
(WebCore::DisplayRefreshMonitorManager::displayDidRefresh): Use remove
instead of find/remove.
* platform/graphics/blackberry/LayerRenderer.cpp:
(WebCore::LayerRenderer::removeLayer): Use the return value from remove
instead of find/remove.
* platform/win/WindowMessageBroadcaster.cpp:
(WebCore::WindowMessageBroadcaster::removeListener): Use remove instead
of find/remove. It's fine to do the check for an empty hash table unconditionally.
* plugins/PluginDatabase.cpp:
(WebCore::PluginDatabase::removeDisabledPluginFile): Use the return value
from remove instead of find/remove.
* rendering/style/StyleCustomFilterProgramCache.cpp:
(WebCore::StyleCustomFilterProgramCache::lookup): Use get instead of find.
(WebCore::StyleCustomFilterProgramCache::add): Use contains instead of find
in an assertion.
(WebCore::StyleCustomFilterProgramCache::remove): Use remove instead of
find/remove.
* svg/SVGCursorElement.cpp:
(WebCore::SVGCursorElement::removeClient): Use the return value from remove
instead of find/remove.
* svg/SVGDocumentExtensions.cpp:
(WebCore::SVGDocumentExtensions::removeResource): Removed an unneeded call
to contains.
(WebCore::SVGDocumentExtensions::removeAllTargetReferencesForElement): Use
remove instead of find/remove. It's fine to do the check for an empty hash
table unconditionally.
(WebCore::SVGDocumentExtensions::removeAllElementReferencesForTarget): Use
remove instead of find/remove. Also removed unhelpful assertions. One is
already done by HashMap, and the other is just checking a basic invariant
of every HashMap that doesn't need to be checked.
* svg/graphics/SVGImageCache.cpp:
(WebCore::SVGImageCache::removeClientFromCache): Removed an unneeded call
to contains.
* svg/properties/SVGAnimatedProperty.cpp:
(WebCore::SVGAnimatedProperty::~SVGAnimatedProperty): Use the version of
remove that takes an iterator rather than the one that takes a key, so
we don't need to redo the hashing.
2013-09-02 Andreas Kling <akling@apple.com>
Generate isFooElement() functions from tagname data.
......@@ -188,29 +188,20 @@ bool Geolocation::Watchers::add(int id, PassRefPtr<GeoNotifier> prpNotifier)
Geolocation::GeoNotifier* Geolocation::Watchers::find(int id)
{
ASSERT(id > 0);
IdToNotifierMap::iterator iter = m_idToNotifierMap.find(id);
if (iter == m_idToNotifierMap.end())
return 0;
return iter->value.get();
return m_idToNotifierMap.get(id);
}
void Geolocation::Watchers::remove(int id)
{
ASSERT(id > 0);
IdToNotifierMap::iterator iter = m_idToNotifierMap.find(id);
if (iter == m_idToNotifierMap.end())
return;
m_notifierToIdMap.remove(iter->value);
m_idToNotifierMap.remove(iter);
if (auto notifier = m_idToNotifierMap.take(id))
m_notifierToIdMap.remove(notifier);
}
void Geolocation::Watchers::remove(GeoNotifier* notifier)
{
NotifierToIdMap::iterator iter = m_notifierToIdMap.find(notifier);
if (iter == m_notifierToIdMap.end())
return;
m_idToNotifierMap.remove(iter->value);
m_notifierToIdMap.remove(iter);
if (auto identifier = m_notifierToIdMap.take(notifier))
m_idToNotifierMap.remove(identifier);
}
bool Geolocation::Watchers::contains(GeoNotifier* notifier) const
......@@ -380,9 +371,7 @@ void Geolocation::makeCachedPositionCallbacks()
// If this is a one-shot request, stop it. Otherwise, if the watch still
// exists, start the service to get updates.
if (m_oneShots.contains(notifier))
m_oneShots.remove(notifier);
else if (m_watchers.contains(notifier)) {
if (!m_oneShots.remove(notifier) && m_watchers.contains(notifier)) {
if (notifier->hasZeroTimeout() || startUpdating(notifier))
notifier->startTimerIfNeeded();
else
......
......@@ -912,20 +912,16 @@ void AudioContext::addAutomaticPullNode(AudioNode* node)
{
ASSERT(isGraphOwner());
if (!m_automaticPullNodes.contains(node)) {
m_automaticPullNodes.add(node);
if (m_automaticPullNodes.add(node).isNewEntry)
m_automaticPullNodesNeedUpdating = true;
}
}
void AudioContext::removeAutomaticPullNode(AudioNode* node)
{
ASSERT(isGraphOwner());
if (m_automaticPullNodes.contains(node)) {
m_automaticPullNodes.remove(node);
if (m_automaticPullNodes.remove(node))
m_automaticPullNodesNeedUpdating = true;
}
}
void AudioContext::updateAutomaticPullNodes()
......
......@@ -54,11 +54,10 @@ void AudioNodeInput::connect(AudioNodeOutput* output)
return;
// Check if we're already connected to this output.
if (m_outputs.contains(output))
if (!m_outputs.add(output).isNewEntry)
return;
output->addInput(this);
m_outputs.add(output);
changedOutputs();
// Sombody has just connected to us, so count it as a reference.
......@@ -74,8 +73,7 @@ void AudioNodeInput::disconnect(AudioNodeOutput* output)
return;
// First try to disconnect from "active" connections.
if (m_outputs.contains(output)) {
m_outputs.remove(output);
if (m_outputs.remove(output)) {
changedOutputs();
output->removeInput(this);
node()->deref(AudioNode::RefTypeConnection); // Note: it's important to return immediately after all deref() calls since the node may be deleted.
......@@ -83,8 +81,7 @@ void AudioNodeInput::disconnect(AudioNodeOutput* output)
}
// Otherwise, try to disconnect from disabled connections.
if (m_disabledOutputs.contains(output)) {
m_disabledOutputs.remove(output);
if (m_disabledOutputs.remove(output)) {
output->removeInput(this);
node()->deref(AudioNode::RefTypeConnection); // Note: it's important to return immediately after all deref() calls since the node may be deleted.
return;
......
......@@ -172,11 +172,10 @@ void AudioParam::connect(AudioNodeOutput* output)
if (!output)
return;
if (m_outputs.contains(output))
if (!m_outputs.add(output).isNewEntry)
return;
output->addParam(this);
m_outputs.add(output);
changedOutputs();
}
......@@ -188,8 +187,7 @@ void AudioParam::disconnect(AudioNodeOutput* output)
if (!output)
return;
if (m_outputs.contains(output)) {
m_outputs.remove(output);
if (m_outputs.remove(output)) {
changedOutputs();
output->removeParam(this);
}
......
......@@ -70,9 +70,8 @@ public:
void remove(RootObject* rootObject)
{
HashMap<RootObject*, JSToNPObjectMap>::iterator iter = m_map.find(rootObject);
ASSERT(iter != m_map.end());
m_map.remove(iter);
ASSERT(m_map.contains(rootObject));
m_map.remove(rootObject);
}
void remove(RootObject* rootObject, JSObject* jsObject)
......
......@@ -336,10 +336,7 @@ void Node::trackForDebugging()
Node::~Node()
{
#ifndef NDEBUG
HashSet<Node*>::iterator it = ignoreSet.find(this);
if (it != ignoreSet.end())
ignoreSet.remove(it);
else
if (ignoreSet.remove(this))
nodeCounter.decrement();
#endif
......
......@@ -312,11 +312,9 @@ void InspectorProfilerAgent::removeProfile(ErrorString*, const String& type, int
{
unsigned uid = static_cast<unsigned>(rawUid);
if (type == CPUProfileType) {
if (m_profiles.contains(uid))
m_profiles.remove(uid);
m_profiles.remove(uid);
} else if (type == HeapProfileType) {
if (m_snapshots.contains(uid))
m_snapshots.remove(uid);
m_snapshots.remove(uid);
}
}
......
......@@ -1311,10 +1311,8 @@ void DocumentLoader::addSubresourceLoader(ResourceLoader* loader)
void DocumentLoader::removeSubresourceLoader(ResourceLoader* loader)
{
ResourceLoaderSet::iterator it = m_subresourceLoaders.find(loader);
if (it == m_subresourceLoaders.end())
if (!m_subresourceLoaders.remove(loader))
return;
m_subresourceLoaders.remove(it);
checkLoadComplete();
if (Frame* frame = m_frame)
frame->loader().checkLoadComplete();
......
......@@ -278,10 +278,8 @@ void ResourceLoadScheduler::HostInformation::addLoadInProgress(ResourceLoader* r
void ResourceLoadScheduler::HostInformation::remove(ResourceLoader* resourceLoader)
{
if (m_requestsLoading.contains(resourceLoader)) {
m_requestsLoading.remove(resourceLoader);
if (m_requestsLoading.remove(resourceLoader))
return;
}
for (int priority = ResourceLoadPriorityHighest; priority >= ResourceLoadPriorityLowest; --priority) {
RequestQueue::iterator end = m_requestsPending[priority].end();
......
......@@ -362,10 +362,7 @@ void ApplicationCacheGroup::stopLoading()
void ApplicationCacheGroup::disassociateDocumentLoader(DocumentLoader* loader)
{
HashSet<DocumentLoader*>::iterator it = m_associatedDocumentLoaders.find(loader);
if (it != m_associatedDocumentLoaders.end())
m_associatedDocumentLoaders.remove(it);
m_associatedDocumentLoaders.remove(loader);
m_pendingMasterResourceLoaders.remove(loader);
loader->applicationCacheHost()->setApplicationCache(0); // Will set candidate to 0, too.
......@@ -390,11 +387,7 @@ void ApplicationCacheGroup::disassociateDocumentLoader(DocumentLoader* loader)
void ApplicationCacheGroup::cacheDestroyed(ApplicationCache* cache)
{
if (!m_caches.contains(cache))
return;
m_caches.remove(cache);
if (m_caches.isEmpty()) {
ASSERT(m_associatedDocumentLoaders.isEmpty());
ASSERT(m_pendingMasterResourceLoaders.isEmpty());
......
......@@ -175,52 +175,38 @@ static DOMWindowSet& windowsWithBeforeUnloadEventListeners()
static void addUnloadEventListener(DOMWindow* domWindow)
{
DOMWindowSet& set = windowsWithUnloadEventListeners();