Avoid copy-prone idiom "for (auto item : collection)"
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Mar 2014 17:51:59 +0000 (17:51 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Mar 2014 17:51:59 +0000 (17:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=129990

Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

* heap/CodeBlockSet.h:
(JSC::CodeBlockSet::iterate): Use auto& to be sure we don't copy by accident.
* inspector/ScriptDebugServer.cpp:
(Inspector::ScriptDebugServer::dispatchBreakpointActionLog): Use auto* to
make explicit that we are iterating through pointers.
(Inspector::ScriptDebugServer::dispatchBreakpointActionSound): Ditto.
(Inspector::ScriptDebugServer::dispatchBreakpointActionProbe): Ditto.
* inspector/agents/InspectorDebuggerAgent.cpp:
(Inspector::InspectorDebuggerAgent::removeBreakpoint): Use auto&, and also
get rid of an unneeded local variable.

Source/WebCore:

Most of these changes have no effect. A few of them get rid of unwanted
copying of the items as we iterate them. Found these with the command
'git grep "for (auto .*:"' or the equivalent.

* Modules/indexeddb/IDBKeyData.cpp:
(WebCore::IDBKeyData::IDBKeyData): Use auto& to avoid copying the keys.
(WebCore::IDBKeyData::maybeCreateIDBKey): Ditto.
(WebCore::IDBKeyData::isolatedCopy): Ditto.

* dom/Node.cpp:
(WebCore::Document::invalidateNodeListAndCollectionCaches): Use auto*
to make explicit the fact that these are pointers. Stop using "it" for
a variable that is not an iterator. Get rid of unneeded local variables
for the collections themselves.

* dom/NodeRareData.h:
(WebCore::NodeListsNodeData::adoptDocument): Use auto& to make sure we
don't do any unnecessary copying. Stop using "it" for a variable that
is not an iterator.

* html/HTMLTableRowsCollection.cpp:
(WebCore::HTMLTableRowsCollection::lastRow): Use auto* to be explicit
that these are pointers.
* inspector/InspectorNodeFinder.cpp:
(WebCore::InspectorNodeFinder::searchUsingDOMTreeTraversal): Ditto.
* page/ios/FrameIOS.mm:
(WebCore::Frame::interpretationsForCurrentRoot): Ditto. Also got rid of
an unnecessary local variable.

* platform/FileChooser.cpp:
(WebCore::FileChooser::chooseFiles): Use auto&. Also fix a FIXME.
(WebCore::FileChooser::chooseMediaFiles): Ditto.

* platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:
(WebCore::SourceBufferPrivateAVFObjC::naturalSize): Use auto&.

* rendering/RenderIterator.h: Changed include from RenderObject.h to
RenderElement.h; iterators are based on RenderElement now.

* rendering/svg/RenderSVGResource.cpp:
(WebCore::removeFromCacheAndInvalidateDependencies): Use auto*.

* rendering/svg/RenderSVGResourceContainer.cpp:
(WebCore::RenderSVGResourceContainer::markAllClientsForInvalidation): Use auto*.
(WebCore::RenderSVGResourceContainer::markAllClientLayersForInvalidation): Ditto.
(WebCore::RenderSVGResourceContainer::registerResource): Ditto.

* rendering/svg/SVGRenderSupport.cpp:
(WebCore::SVGRenderSupport::layoutChildren): Updated to use a more specific
type, to use auto* instead of of auto, and to eliminate the slightly sloppily
capitalized and not-so-slightly ungrammatical notlayoutedObjects.
(WebCore::SVGRenderSupport::applyStrokeStyleToContext): Use auto&.
(WebCore::SVGRenderSupport::updateMaskedAncestorShouldIsolateBlending): Use auto*.

* rendering/svg/SVGResourcesCycleSolver.cpp:
(WebCore::SVGResourcesCycleSolver::resourceContainsCycles): Use auto*.
(WebCore::SVGResourcesCycleSolver::resolveCycles): Ditto. Also lineageOfType.

* svg/SVGAnimateMotionElement.cpp:
(WebCore::SVGAnimateMotionElement::applyResultsToTarget): Use auto*.
* svg/SVGPathElement.cpp:
(WebCore::SVGPathElement::invalidateMPathDependencies): Ditto.

Source/WebKit/mac:

* WebView/WebFrame.mm:
(-[WebFrame _documentFragmentWithNodesAsParagraphs:]):
Use auto*. Also removed uneeded ASSERT_NO_EXCEPTION, which is already
the default without specifying it explicitly.

Source/WebKit2:

* Shared/mac/RemoteLayerTreePropertyApplier.mm:
(WebKit::RemoteLayerTreePropertyApplier::applyProperties): Use auto&.

* Shared/mac/RemoteLayerTreeTransaction.mm:
(WebKit::RemoteLayerTreeTransaction::LayerProperties::decode): Use auto&
even though the type is a scalar. This does no harm and makes it easier
to spot uses that trigger unnecessary copying with grep.
(WebKit::RemoteLayerTreeTransaction::decode): Ditto.
(WebKit::dumpChangedLayers): Ditto.
* UIProcess/mac/RemoteLayerTreeHost.mm:
(WebKit::RemoteLayerTreeHost::updateLayerTree): Ditto.
* WebProcess/WebProcess.cpp:
(WebKit::WebProcess::visitedLinkStateChanged): Ditto.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@165546 268f45cc-cd09-0410-ab3c-d52691b4dbfc

27 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/heap/CodeBlockSet.h
Source/JavaScriptCore/inspector/ScriptDebugServer.cpp
Source/JavaScriptCore/inspector/agents/InspectorDebuggerAgent.cpp
Source/WebCore/ChangeLog
Source/WebCore/Modules/indexeddb/IDBKeyData.cpp
Source/WebCore/dom/Node.cpp
Source/WebCore/dom/NodeRareData.h
Source/WebCore/html/HTMLTableRowsCollection.cpp
Source/WebCore/inspector/InspectorNodeFinder.cpp
Source/WebCore/page/ios/FrameIOS.mm
Source/WebCore/platform/FileChooser.cpp
Source/WebCore/platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm
Source/WebCore/rendering/RenderIterator.h
Source/WebCore/rendering/svg/RenderSVGResource.cpp
Source/WebCore/rendering/svg/RenderSVGResourceContainer.cpp
Source/WebCore/rendering/svg/SVGRenderSupport.cpp
Source/WebCore/rendering/svg/SVGResourcesCycleSolver.cpp
Source/WebCore/svg/SVGAnimateMotionElement.cpp
Source/WebCore/svg/SVGPathElement.cpp
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/WebView/WebFrame.mm
Source/WebKit2/ChangeLog
Source/WebKit2/Shared/mac/RemoteLayerTreePropertyApplier.mm
Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.mm
Source/WebKit2/UIProcess/mac/RemoteLayerTreeHost.mm
Source/WebKit2/WebProcess/WebProcess.cpp

index 0fc7b9d..8579660 100644 (file)
@@ -1,3 +1,21 @@
+2014-03-10  Darin Adler  <darin@apple.com>
+
+        Avoid copy-prone idiom "for (auto item : collection)"
+        https://bugs.webkit.org/show_bug.cgi?id=129990
+
+        Reviewed by Geoffrey Garen.
+
+        * heap/CodeBlockSet.h:
+        (JSC::CodeBlockSet::iterate): Use auto& to be sure we don't copy by accident.
+        * inspector/ScriptDebugServer.cpp:
+        (Inspector::ScriptDebugServer::dispatchBreakpointActionLog): Use auto* to
+        make explicit that we are iterating through pointers.
+        (Inspector::ScriptDebugServer::dispatchBreakpointActionSound): Ditto.
+        (Inspector::ScriptDebugServer::dispatchBreakpointActionProbe): Ditto.
+        * inspector/agents/InspectorDebuggerAgent.cpp:
+        (Inspector::InspectorDebuggerAgent::removeBreakpoint): Use auto&, and also
+        get rid of an unneeded local variable.
+
 2014-03-13  Brian Burg  <bburg@apple.com>
 
         Web Inspector: Remove unused callId parameter from evaluateInWebInspector
index 104ffc4..7f50d5a 100644 (file)
@@ -80,7 +80,7 @@ public:
     // visited.
     template<typename Functor> void iterate(Functor& functor)
     {
-        for (auto &codeBlock : m_set) {
+        for (autocodeBlock : m_set) {
             bool done = functor(codeBlock);
             if (done)
                 break;
index 12dbaea..436be5f 100644 (file)
@@ -154,7 +154,7 @@ void ScriptDebugServer::dispatchBreakpointActionLog(ExecState* exec, const Strin
 
     Vector<ScriptDebugListener*> listenersCopy;
     copyToVector(*listeners, listenersCopy);
-    for (auto listener : listenersCopy)
+    for (auto* listener : listenersCopy)
         listener->breakpointActionLog(exec, message);
 }
 
@@ -172,7 +172,7 @@ void ScriptDebugServer::dispatchBreakpointActionSound(ExecState* exec, int break
 
     Vector<ScriptDebugListener*> listenersCopy;
     copyToVector(*listeners, listenersCopy);
-    for (auto listener : listenersCopy)
+    for (auto* listener : listenersCopy)
         listener->breakpointActionSound(breakpointActionIdentifier);
 }
 
@@ -190,7 +190,7 @@ void ScriptDebugServer::dispatchBreakpointActionProbe(ExecState* exec, const Scr
 
     Vector<ScriptDebugListener*> listenersCopy;
     copyToVector(*listeners, listenersCopy);
-    for (auto listener : listenersCopy)
+    for (auto* listener : listenersCopy)
         listener->breakpointActionProbe(exec, action, m_hitCount, sample);
 }
 
index a314e16..d826cc2 100644 (file)
@@ -341,8 +341,7 @@ void InspectorDebuggerAgent::removeBreakpoint(ErrorString*, const String& breakp
 {
     m_javaScriptBreakpoints.remove(breakpointIdentifier);
 
-    Vector<JSC::BreakpointID> breakpointIDs = m_breakpointIdentifierToDebugServerBreakpointIDs.take(breakpointIdentifier);
-    for (auto breakpointID : breakpointIDs) {
+    for (JSC::BreakpointID breakpointID : m_breakpointIdentifierToDebugServerBreakpointIDs.take(breakpointIdentifier)) {
         const Vector<ScriptBreakpointAction>& breakpointActions = scriptDebugServer().getActionsForBreakpoint(breakpointID);
         for (auto& action : breakpointActions)
             m_injectedScriptManager->releaseObjectGroup(objectGroupForBreakpointAction(action));
index c3392b3..730c51d 100644 (file)
@@ -1,3 +1,73 @@
+2014-03-11  Darin Adler  <darin@apple.com>
+
+        Avoid copy-prone idiom "for (auto item : collection)"
+        https://bugs.webkit.org/show_bug.cgi?id=129990
+
+        Reviewed by Geoffrey Garen.
+
+        Most of these changes have no effect. A few of them get rid of unwanted
+        copying of the items as we iterate them. Found these with the command
+        'git grep "for (auto .*:"' or the equivalent.
+
+        * Modules/indexeddb/IDBKeyData.cpp:
+        (WebCore::IDBKeyData::IDBKeyData): Use auto& to avoid copying the keys.
+        (WebCore::IDBKeyData::maybeCreateIDBKey): Ditto.
+        (WebCore::IDBKeyData::isolatedCopy): Ditto.
+
+        * dom/Node.cpp:
+        (WebCore::Document::invalidateNodeListAndCollectionCaches): Use auto*
+        to make explicit the fact that these are pointers. Stop using "it" for
+        a variable that is not an iterator. Get rid of unneeded local variables
+        for the collections themselves.
+
+        * dom/NodeRareData.h:
+        (WebCore::NodeListsNodeData::adoptDocument): Use auto& to make sure we
+        don't do any unnecessary copying. Stop using "it" for a variable that
+        is not an iterator.
+
+        * html/HTMLTableRowsCollection.cpp:
+        (WebCore::HTMLTableRowsCollection::lastRow): Use auto* to be explicit
+        that these are pointers.
+        * inspector/InspectorNodeFinder.cpp:
+        (WebCore::InspectorNodeFinder::searchUsingDOMTreeTraversal): Ditto.
+        * page/ios/FrameIOS.mm:
+        (WebCore::Frame::interpretationsForCurrentRoot): Ditto. Also got rid of
+        an unnecessary local variable.
+
+        * platform/FileChooser.cpp:
+        (WebCore::FileChooser::chooseFiles): Use auto&. Also fix a FIXME.
+        (WebCore::FileChooser::chooseMediaFiles): Ditto.
+
+        * platform/graphics/avfoundation/objc/SourceBufferPrivateAVFObjC.mm:
+        (WebCore::SourceBufferPrivateAVFObjC::naturalSize): Use auto&.
+
+        * rendering/RenderIterator.h: Changed include from RenderObject.h to
+        RenderElement.h; iterators are based on RenderElement now.
+
+        * rendering/svg/RenderSVGResource.cpp:
+        (WebCore::removeFromCacheAndInvalidateDependencies): Use auto*.
+
+        * rendering/svg/RenderSVGResourceContainer.cpp:
+        (WebCore::RenderSVGResourceContainer::markAllClientsForInvalidation): Use auto*.
+        (WebCore::RenderSVGResourceContainer::markAllClientLayersForInvalidation): Ditto.
+        (WebCore::RenderSVGResourceContainer::registerResource): Ditto.
+
+        * rendering/svg/SVGRenderSupport.cpp:
+        (WebCore::SVGRenderSupport::layoutChildren): Updated to use a more specific
+        type, to use auto* instead of of auto, and to eliminate the slightly sloppily
+        capitalized and not-so-slightly ungrammatical notlayoutedObjects.
+        (WebCore::SVGRenderSupport::applyStrokeStyleToContext): Use auto&.
+        (WebCore::SVGRenderSupport::updateMaskedAncestorShouldIsolateBlending): Use auto*.
+
+        * rendering/svg/SVGResourcesCycleSolver.cpp:
+        (WebCore::SVGResourcesCycleSolver::resourceContainsCycles): Use auto*.
+        (WebCore::SVGResourcesCycleSolver::resolveCycles): Ditto. Also lineageOfType.
+
+        * svg/SVGAnimateMotionElement.cpp:
+        (WebCore::SVGAnimateMotionElement::applyResultsToTarget): Use auto*.
+        * svg/SVGPathElement.cpp:
+        (WebCore::SVGPathElement::invalidateMPathDependencies): Ditto.
+
 2014-03-13  Brian Burg  <bburg@apple.com>
 
         Web Inspector: Remove unused callId parameter from evaluateInWebInspector
index 2a9fdc9..7f31c47 100644 (file)
@@ -49,7 +49,7 @@ IDBKeyData::IDBKeyData(const IDBKey* key)
     case IDBKey::InvalidType:
         break;
     case IDBKey::ArrayType:
-        for (auto key2 : key->array())
+        for (auto& key2 : key->array())
             arrayValue.append(IDBKeyData(key2.get()));
         break;
     case IDBKey::StringType:
@@ -78,7 +78,7 @@ PassRefPtr<IDBKey> IDBKeyData::maybeCreateIDBKey() const
     case IDBKey::ArrayType:
         {
             IDBKey::KeyArray array;
-            for (auto keyData : arrayValue) {
+            for (auto& keyData : arrayValue) {
                 array.append(keyData.maybeCreateIDBKey());
                 ASSERT(array.last());
             }
@@ -110,7 +110,7 @@ IDBKeyData IDBKeyData::isolatedCopy() const
     case IDBKey::InvalidType:
         return result;
     case IDBKey::ArrayType:
-        for (auto key : arrayValue)
+        for (auto& key : arrayValue)
             result.arrayValue.append(key.isolatedCopy());
         return result;
     case IDBKey::StringType:
index 43d3fcf..4f362a8 100644 (file)
@@ -724,13 +724,10 @@ void Document::invalidateNodeListAndCollectionCaches(const QualifiedName* attrNa
 #if !ASSERT_DISABLED
     m_inInvalidateNodeListAndCollectionCaches = true;
 #endif
-    HashSet<LiveNodeList*> liveNodeLists = std::move(m_listsInvalidatedAtDocument);
-    for (auto it : liveNodeLists)
-        it->invalidateCache(attrName);
-
-    HashSet<HTMLCollection*> collectionLists = std::move(m_collectionsInvalidatedAtDocument);
-    for (auto it : collectionLists)
-        it->invalidateCache(attrName);
+    for (auto* list : std::move(m_listsInvalidatedAtDocument))
+        list->invalidateCache(attrName);
+    for (auto* list : std::move(m_collectionsInvalidatedAtDocument))
+        list->invalidateCache(attrName);
 #if !ASSERT_DISABLED
     m_inInvalidateNodeListAndCollectionCaches = false;
 #endif
index ef736ba..0351f18 100644 (file)
 #include "Page.h"
 #include "QualifiedName.h"
 #include "TagNodeList.h"
-#if ENABLE(VIDEO_TRACK)
-#include "TextTrack.h"
-#endif
 #include <wtf/HashSet.h>
 #include <wtf/OwnPtr.h>
 #include <wtf/PassOwnPtr.h>
 #include <wtf/text/AtomicString.h>
 #include <wtf/text/StringHash.h>
 
+#if ENABLE(VIDEO_TRACK)
+#include "TextTrack.h"
+#endif
+
 namespace WebCore {
 
 class LabelsNodeList;
@@ -231,27 +232,27 @@ public:
             return;
         }
 
-        for (auto it : m_atomicNameCaches)
-            it.value->invalidateCache(*oldDocument);
+        for (auto& cache : m_atomicNameCaches.values())
+            cache->invalidateCache(*oldDocument);
 
-        for (auto it : m_nameCaches)
-            it.value->invalidateCache(*oldDocument);
+        for (auto& cache : m_nameCaches.values())
+            cache->invalidateCache(*oldDocument);
 
-        for (auto it : m_tagNodeListCacheNS) {
-            LiveNodeList& list = *it.value;
-            ASSERT(!list.isRootedAtDocument());
-            list.invalidateCache(*oldDocument);
+        for (auto& list : m_tagNodeListCacheNS.values()) {
+            ASSERT(!list->isRootedAtDocument());
+            list->invalidateCache(*oldDocument);
         }
 
-        for (auto it : m_cachedCollections)
-            it.value->invalidateCache(*oldDocument);
+        for (auto& collection : m_cachedCollections.values())
+            collection->invalidateCache(*oldDocument);
     }
 
 private:
     NodeListsNodeData()
         : m_childNodeList(nullptr)
         , m_emptyChildNodeList(nullptr)
-    { }
+    {
+    }
 
     std::pair<unsigned char, AtomicString> namedCollectionKey(CollectionType type, const AtomicString& name)
     {
index 67ae117..a50ed27 100644 (file)
@@ -122,25 +122,25 @@ HTMLTableRowElement* HTMLTableRowsCollection::rowAfter(HTMLTableElement* table,
 
 HTMLTableRowElement* HTMLTableRowsCollection::lastRow(HTMLTableElement* table)
 {
-    for (auto child = ElementTraversal::lastChild(table); child; child = ElementTraversal::previousSibling(child)) {
+    for (auto* child = ElementTraversal::lastChild(table); child; child = ElementTraversal::previousSibling(child)) {
         if (child->hasTagName(tfootTag)) {
-            if (auto row = childrenOfType<HTMLTableRowElement>(*child).last())
+            if (auto* row = childrenOfType<HTMLTableRowElement>(*child).last())
                 return row;
         }
     }
 
-    for (auto child = ElementTraversal::lastChild(table); child; child = ElementTraversal::previousSibling(child)) {
+    for (auto* child = ElementTraversal::lastChild(table); child; child = ElementTraversal::previousSibling(child)) {
         if (isHTMLTableRowElement(child))
             return toHTMLTableRowElement(child);
         if (child->hasTagName(tbodyTag)) {
-            if (auto row = childrenOfType<HTMLTableRowElement>(*child).last())
+            if (auto* row = childrenOfType<HTMLTableRowElement>(*child).last())
                 return row;
         }
     }
 
-    for (auto child = ElementTraversal::lastChild(table); child; child = ElementTraversal::previousSibling(child)) {
+    for (auto* child = ElementTraversal::lastChild(table); child; child = ElementTraversal::previousSibling(child)) {
         if (child->hasTagName(theadTag)) {
-            if (auto row = childrenOfType<HTMLTableRowElement>(*child).last())
+            if (auto* row = childrenOfType<HTMLTableRowElement>(*child).last())
                 return row;
         }
     }
index 89736f8..48e37ff 100644 (file)
@@ -75,7 +75,7 @@ void InspectorNodeFinder::performSearch(Node* parentNode)
 void InspectorNodeFinder::searchUsingDOMTreeTraversal(Node* parentNode)
 {
     // Manual plain text search.
-    for (auto node = parentNode; node; node = NodeTraversal::next(node, parentNode)) {
+    for (auto* node = parentNode; node; node = NodeTraversal::next(node, parentNode)) {
         switch (node->nodeType()) {
         case Node::TEXT_NODE:
         case Node::COMMENT_NODE:
index 4f48ea4..b3db2a3 100644 (file)
@@ -759,7 +759,7 @@ NSArray *Frame::interpretationsForCurrentRoot() const
     // The number of interpretations will be i1 * i2 * ... * iN, where iX is the number of interpretations for the Xth phrase with alternatives.
     size_t interpretationsCount = 1;
 
-    for (auto marker : markersInRoot)
+    for (auto* marker : markersInRoot)
         interpretationsCount *= marker->alternatives().size() + 1;
 
     Vector<Vector<UChar>> interpretations;
@@ -771,8 +771,7 @@ NSArray *Frame::interpretationsForCurrentRoot() const
 
     Node* pastLastNode = rangeOfRootContents->pastLastNode();
     for (Node* node = rangeOfRootContents->firstNode(); node != pastLastNode; node = NodeTraversal::next(node)) {
-        const Vector<DocumentMarker*>& markers = document()->markers().markersFor(node, DocumentMarker::MarkerTypes(DocumentMarker::DictationPhraseWithAlternatives));
-        for (auto marker : markers) {
+        for (auto* marker : document()->markers().markersFor(node, DocumentMarker::MarkerTypes(DocumentMarker::DictationPhraseWithAlternatives))) {
             // First, add text that precede the marker.
             if (precedingTextStartPosition != createLegacyEditingPosition(node, marker->startOffset())) {
                 RefPtr<Range> precedingTextRange = Range::create(*document(), precedingTextStartPosition, createLegacyEditingPosition(node, marker->startOffset()));
index 4a10f16..095f8de 100644 (file)
@@ -70,12 +70,13 @@ void FileChooser::chooseFiles(const Vector<String>& filenames)
         return;
 
     Vector<FileChooserFileInfo> files;
-    for (unsigned i = 0; i < filenames.size(); ++i)
-        files.append(FileChooserFileInfo(filenames[i]));
+    for (auto& filename : filenames)
+        files.append(FileChooserFileInfo(filename));
     m_client->filesChosen(files);
 }
 
 #if PLATFORM(IOS)
+
 // FIXME: This function is almost identical to FileChooser::chooseFiles(). We should merge this function
 // with FileChooser::chooseFiles() and hence remove the PLATFORM(IOS)-guard.
 void FileChooser::chooseMediaFiles(const Vector<String>& filenames, const String& displayString, Icon* icon)
@@ -88,19 +89,20 @@ void FileChooser::chooseMediaFiles(const Vector<String>& filenames, const String
         return;
 
     Vector<FileChooserFileInfo> files;
-    for (auto filename : filenames)
+    for (auto& filename : filenames)
         files.append(FileChooserFileInfo(filename));
     m_client->filesChosen(files, displayString, icon);
 }
+
 #endif
 
 void FileChooser::chooseFiles(const Vector<FileChooserFileInfo>& files)
 {
-    // FIXME: This is inelegant. We should not be looking at settings here.
     Vector<String> paths;
-    for (unsigned i = 0; i < files.size(); ++i)
-        paths.append(files[i].path);
+    for (auto& file : files)
+        paths.append(file.path);
 
+    // FIXME: This is inelegant. We should not be looking at settings here.
     if (m_settings.selectedFiles == paths)
         return;
 
index 4bf80cd..bf538b5 100644 (file)
@@ -695,7 +695,7 @@ void SourceBufferPrivateAVFObjC::seekToTime(MediaTime time)
 
 IntSize SourceBufferPrivateAVFObjC::naturalSize()
 {
-    for (auto videoTrack : m_videoTracks) {
+    for (auto& videoTrack : m_videoTracks) {
         if (videoTrack->selected())
             return videoTrack->naturalSize();
     }
index b50a344..5c51947 100644 (file)
@@ -26,7 +26,7 @@
 #ifndef RenderIterator_h
 #define RenderIterator_h
 
-#include "RenderObject.h"
+#include "RenderElement.h"
 
 namespace WebCore {
 
index 22616b1..9f599f0 100644 (file)
@@ -173,8 +173,8 @@ static inline void removeFromCacheAndInvalidateDependencies(RenderElement& rende
     HashSet<SVGElement*>* dependencies = renderer.document().accessSVGExtensions()->setOfElementsReferencingTarget(toSVGElement(renderer.element()));
     if (!dependencies)
         return;
-    for (auto element : *dependencies) {
-        if (auto renderer = element->renderer())
+    for (auto* element : *dependencies) {
+        if (auto* renderer = element->renderer())
             RenderSVGResource::markForLayoutAndParentResourceInvalidation(*renderer, needsLayout);
     }
 }
index 3defc46..3ee64c0 100644 (file)
@@ -96,7 +96,7 @@ void RenderSVGResourceContainer::markAllClientsForInvalidation(InvalidationMode
     bool needsLayout = mode == LayoutAndBoundariesInvalidation;
     bool markForInvalidation = mode != ParentOnlyInvalidation;
 
-    for (auto client : m_clients) {
+    for (auto* client : m_clients) {
         if (client->isSVGResourceContainer()) {
             toRenderSVGResourceContainer(*client).removeAllClientsFromCache(markForInvalidation);
             continue;
@@ -116,7 +116,7 @@ void RenderSVGResourceContainer::markAllClientsForInvalidation(InvalidationMode
 void RenderSVGResourceContainer::markAllClientLayersForInvalidation()
 {
 #if ENABLE(CSS_FILTERS)
-    for (auto clientLayer : m_clientLayers)
+    for (auto* clientLayer : m_clientLayers)
         clientLayer->filterNeedsRepaint();
 #endif
 }
@@ -176,11 +176,10 @@ void RenderSVGResourceContainer::registerResource()
     extensions.addResource(m_id, this);
 
     // Update cached resources of pending clients.
-    auto end = clients->end();
-    for (auto it = clients->begin(); it != end; ++it) {
-        ASSERT((*it)->hasPendingResources());
-        extensions.clearHasPendingResourcesIfPossible(*it);
-        auto renderer = (*it)->renderer();
+    for (auto* client : *clients) {
+        ASSERT(client->hasPendingResources());
+        extensions.clearHasPendingResourcesIfPossible(client);
+        auto* renderer = client->renderer();
         if (!renderer)
             continue;
         SVGResourcesCache::clientStyleChanged(*renderer, StyleDifferenceLayout, renderer->style());
index f324a58..3530505 100644 (file)
@@ -226,7 +226,7 @@ void SVGRenderSupport::layoutChildren(RenderElement& start, bool selfNeedsLayout
     bool transformChanged = transformToRootChanged(&start);
     bool hasSVGShadow = rendererHasSVGShadow(start);
     bool needsBoundariesUpdate = start.needsBoundariesUpdate();
-    HashSet<RenderObject*> notlayoutedObjects;
+    HashSet<RenderElement*> elementsThatDidNotReceiveLayout;
 
     for (RenderObject* child = start.firstChild(); child; child = child->nextSibling()) {
         bool needsLayout = selfNeedsLayout;
@@ -274,22 +274,20 @@ void SVGRenderSupport::layoutChildren(RenderElement& start, bool selfNeedsLayout
             // parent containers call repaint().  (RenderBlock::layout* has similar logic.)
             if (!childEverHadLayout)
                 child->repaint();
-        } else if (layoutSizeChanged)
-            notlayoutedObjects.add(child);
+        } else if (layoutSizeChanged && child->isRenderElement())
+            elementsThatDidNotReceiveLayout.add(toRenderElement(child));
 
         ASSERT(!child->needsLayout());
     }
 
     if (!layoutSizeChanged) {
-        ASSERT(notlayoutedObjects.isEmpty());
+        ASSERT(elementsThatDidNotReceiveLayout.isEmpty());
         return;
     }
 
     // If the layout size changed, invalidate all resources of all children that didn't go through the layout() code path.
-    for (auto child : notlayoutedObjects) {
-        if (child->isRenderElement())
-            invalidateResourcesOfChildren(toRenderElement(*child));
-    }
+    for (auto* element : elementsThatDidNotReceiveLayout)
+        invalidateResourcesOfChildren(*element);
 }
 
 bool SVGRenderSupport::isOverflowHidden(const RenderElement& renderer)
@@ -428,8 +426,8 @@ void SVGRenderSupport::applyStrokeStyleToContext(GraphicsContext* context, const
     else {
         DashArray dashArray;
         dashArray.reserveInitialCapacity(dashes.size());
-        for (unsigned i = 0, size = dashes.size(); i < size; ++i)
-            dashArray.uncheckedAppend(dashes[i].value(lengthContext));
+        for (auto& dash : dashes)
+            dashArray.uncheckedAppend(dash.value(lengthContext));
 
         context->setLineDash(dashArray, svgStyle.strokeDashOffset().value(lengthContext));
     }
@@ -465,7 +463,7 @@ void SVGRenderSupport::updateMaskedAncestorShouldIsolateBlending(const RenderEle
     ASSERT(renderer.element()->isSVGElement());
 
     bool maskedAncestorShouldIsolateBlending = renderer.style().hasBlendMode();
-    for (auto ancestor = renderer.element()->parentElement(); ancestor && ancestor->isSVGElement(); ancestor = ancestor->parentElement()) {
+    for (auto* ancestor = renderer.element()->parentElement(); ancestor && ancestor->isSVGElement(); ancestor = ancestor->parentElement()) {
         if (!toSVGElement(ancestor)->isSVGGraphicsElement() || !isolatesBlending(*ancestor->computedStyle()))
             continue;
 
index 649967c..e35620e 100644 (file)
@@ -23,8 +23,7 @@
 // Set to a value > 0, to debug the resource cache.
 #define DEBUG_CYCLE_DETECTION 0
 
-#include "RenderElement.h"
-#include "RenderIterator.h"
+#include "RenderAncestorIterator.h"
 #include "RenderSVGResourceClipper.h"
 #include "RenderSVGResourceFilter.h"
 #include "RenderSVGResourceMarker.h"
@@ -56,7 +55,7 @@ bool SVGResourcesCycleSolver::resourceContainsCycles(RenderElement& renderer) co
         resources->buildSetOfResources(resourceSet);
 
         // Walk all resources and check wheter they reference any resource contained in the resources set.
-        for (auto resource : resourceSet) {
+        for (auto* resource : resourceSet) {
             if (m_allResources.contains(resource))
                 return true;
         }
@@ -75,7 +74,7 @@ bool SVGResourcesCycleSolver::resourceContainsCycles(RenderElement& renderer) co
         childResources->buildSetOfResources(childResourceSet);
 
         // Walk all child resources and check wheter they reference any resource contained in the resources set.
-        for (auto& resource : childResourceSet) {
+        for (auto* resource : childResourceSet) {
             if (m_allResources.contains(resource))
                 return true;
         }
@@ -103,31 +102,27 @@ void SVGResourcesCycleSolver::resolveCycles()
     ASSERT(!localResources.isEmpty());
 
     // Add all parent resource containers to the HashSet.
-    HashSet<RenderSVGResourceContainer*> parentResources;
-    auto parent = m_renderer.parent();
-    while (parent) {
-        if (parent->isSVGResourceContainer())
-            parentResources.add(toRenderSVGResourceContainer(parent));
-        parent = parent->parent();
-    }
+    HashSet<RenderSVGResourceContainer*> ancestorResources;
+    for (auto& resource : ancestorsOfType<RenderSVGResourceContainer>(m_renderer))
+        ancestorResources.add(&resource);
 
 #if DEBUG_CYCLE_DETECTION > 0
     fprintf(stderr, "\nDetecting wheter any resources references any of following objects:\n");
     {
         fprintf(stderr, "Local resources:\n");
-        for (auto it = localResources.begin(), end = localResources.end(); it != end; ++it)
-            fprintf(stderr, "|> %s: object=%p (node=%p)\n", (*it)->renderName(), *it, (*it)->node());
+        for (auto* resource : localResources)
+            fprintf(stderr, "|> %s: object=%p (node=%p)\n", resource->renderName(), resource, resource->node());
 
         fprintf(stderr, "Parent resources:\n");
-        for (auto it = parentResources.begin(), end = parentResources.end(); it != end; ++it)
-            fprintf(stderr, "|> %s: object=%p (node=%p)\n", (*it)->renderName(), *it, (*it)->node());
+        for (auto* resource : ancestorResources)
+            fprintf(stderr, "|> %s: object=%p (node=%p)\n", resource->renderName(), resource, resource->node());
     }
 #endif
 
     // Build combined set of local and parent resources.
     m_allResources = localResources;
-    for (auto it = parentResources.begin(), end = parentResources.end(); it != end; ++it)
-        m_allResources.add(*it);
+    for (auto* resource : ancestorResources)
+        m_allResources.add(resource);
 
     // If we're a resource, add ourselves to the HashSet.
     if (m_renderer.isSVGResourceContainer())
@@ -137,10 +132,9 @@ void SVGResourcesCycleSolver::resolveCycles()
 
     // The job of this function is to determine wheter any of the 'resources' associated with the given 'renderer'
     // references us (or wheter any of its kids references us) -> that's a cycle, we need to find and break it.
-    for (auto it = localResources.begin(), end = localResources.end(); it != end; ++it) {
-        RenderSVGResourceContainer& resource = **it;
-        if (parentResources.contains(&resource) || resourceContainsCycles(resource))
-            breakCycle(resource);
+    for (auto* resource : localResources) {
+        if (ancestorResources.contains(resource) || resourceContainsCycles(*resource))
+            breakCycle(*resource);
     }
 
 #if DEBUG_CYCLE_DETECTION > 0
index 5c0fa22..30c72bf 100644 (file)
@@ -295,7 +295,7 @@ void SVGAnimateMotionElement::applyResultsToTarget()
         return;
 
     // ...except in case where we have additional instances in <use> trees.
-    for (auto instance : targetElement->instancesForElement()) {
+    for (auto* instance : targetElement->instancesForElement()) {
         SVGElement* shadowTreeElement = instance->shadowTreeElement();
         ASSERT(shadowTreeElement);
         AffineTransform* transform = shadowTreeElement->supplementalTransform();
index c08f413..9aaa4d2 100644 (file)
@@ -284,7 +284,7 @@ void SVGPathElement::invalidateMPathDependencies()
     // <mpath> can only reference <path> but this dependency is not handled in
     // markForLayoutAndParentResourceInvalidation so we update any mpath dependencies manually.
     if (HashSet<SVGElement*>* dependencies = document().accessSVGExtensions()->setOfElementsReferencingTarget(this)) {
-        for (auto element : *dependencies) {
+        for (auto* element : *dependencies) {
             if (element->hasTagName(SVGNames::mpathTag))
                 toSVGMPathElement(element)->targetPathChanged();
         }
index 5e2cfb2..4932ba4 100644 (file)
@@ -1,3 +1,15 @@
+2014-03-10  Darin Adler  <darin@apple.com>
+
+        Avoid copy-prone idiom "for (auto item : collection)"
+        https://bugs.webkit.org/show_bug.cgi?id=129990
+
+        Reviewed by Geoffrey Garen.
+
+        * WebView/WebFrame.mm:
+        (-[WebFrame _documentFragmentWithNodesAsParagraphs:]):
+        Use auto*. Also removed uneeded ASSERT_NO_EXCEPTION, which is already
+        the default without specifying it explicitly.
+
 2014-03-12  Brian Burg  <bburg@apple.com>
 
         Web Inspector: Remove unused callId parameter from evaluateInWebInspector
index cf8d723..a768ebd 100644 (file)
@@ -901,10 +901,10 @@ static inline WebDataSource *dataSource(DocumentLoader* loader)
 
     RefPtr<DocumentFragment> fragment = document->createDocumentFragment();
 
-    for (auto node : nodesVector) {
+    for (auto* node : nodesVector) {
         RefPtr<Element> element = createDefaultParagraphElement(*document);
-        element->appendChild(node, ASSERT_NO_EXCEPTION);
-        fragment->appendChild(element.release(), ASSERT_NO_EXCEPTION);
+        element->appendChild(node);
+        fragment->appendChild(element.release());
     }
 
     return kit(fragment.release().get());
index 179ec65..c83b837 100644 (file)
@@ -1,3 +1,24 @@
+2014-03-10  Darin Adler  <darin@apple.com>
+
+        Avoid copy-prone idiom "for (auto item : collection)"
+        https://bugs.webkit.org/show_bug.cgi?id=129990
+
+        Reviewed by Geoffrey Garen.
+
+        * Shared/mac/RemoteLayerTreePropertyApplier.mm:
+        (WebKit::RemoteLayerTreePropertyApplier::applyProperties): Use auto&.
+
+        * Shared/mac/RemoteLayerTreeTransaction.mm:
+        (WebKit::RemoteLayerTreeTransaction::LayerProperties::decode): Use auto&
+        even though the type is a scalar. This does no harm and makes it easier
+        to spot uses that trigger unnecessary copying with grep.
+        (WebKit::RemoteLayerTreeTransaction::decode): Ditto.
+        (WebKit::dumpChangedLayers): Ditto.
+        * UIProcess/mac/RemoteLayerTreeHost.mm:
+        (WebKit::RemoteLayerTreeHost::updateLayerTree): Ditto.
+        * WebProcess/WebProcess.cpp:
+        (WebKit::WebProcess::visitedLinkStateChanged): Ditto.
+
 2014-03-12  Brian Burg  <bburg@apple.com>
 
         Web Inspector: Remove unused callId parameter from evaluateInWebInspector
index 58a4ad5..b362c75 100644 (file)
@@ -240,7 +240,7 @@ void RemoteLayerTreePropertyApplier::applyProperties(UIView *view, const RemoteL
 
     if (properties.changedProperties & RemoteLayerTreeTransaction::ChildrenChanged) {
         RetainPtr<NSMutableArray> children = adoptNS([[NSMutableArray alloc] initWithCapacity:properties.children.size()]);
-        for (auto child : properties.children) {
+        for (auto& child : properties.children) {
             ASSERT(relatedLayers.get(child));
             [children addObject:relatedLayers.get(child)];
         }
index 4103935..e0cec3b 100644 (file)
@@ -240,7 +240,7 @@ bool RemoteLayerTreeTransaction::LayerProperties::decode(IPC::ArgumentDecoder& d
         if (!decoder.decode(result.children))
             return false;
 
-        for (auto layerID : result.children) {
+        for (auto& layerID : result.children) {
             if (!layerID)
                 return false;
         }
@@ -455,7 +455,7 @@ bool RemoteLayerTreeTransaction::decode(IPC::ArgumentDecoder& decoder, RemoteLay
     if (!decoder.decode(result.m_destroyedLayerIDs))
         return false;
 
-    for (auto layerID : result.m_destroyedLayerIDs) {
+    for (auto& layerID : result.m_destroyedLayerIDs) {
         if (!layerID)
             return false;
     }
@@ -699,7 +699,7 @@ static void dumpChangedLayers(RemoteLayerTreeTextStream& ts, const RemoteLayerTr
     copyKeysToVector(changedLayerProperties, layerIDs);
     std::sort(layerIDs.begin(), layerIDs.end());
 
-    for (auto layerID : layerIDs) {
+    for (auto& layerID : layerIDs) {
         const RemoteLayerTreeTransaction::LayerProperties& layerProperties = *changedLayerProperties.get(layerID);
 
         ts << "\n";
index d9f0a3c..ad003b3 100644 (file)
@@ -93,7 +93,7 @@ bool RemoteLayerTreeHost::updateLayerTree(const RemoteLayerTreeTransaction& tran
             RemoteLayerTreePropertyApplier::applyProperties(layer, properties, relatedLayers);
     }
 
-    for (auto destroyedLayer : transaction.destroyedLayers())
+    for (auto& destroyedLayer : transaction.destroyedLayers())
         m_layers.remove(destroyedLayer);
 
     return rootLayerChanged;
index 3be9fcf..b15e130 100644 (file)
@@ -514,7 +514,7 @@ void WebProcess::visitedLinkStateChanged(const Vector<WebCore::LinkHash>& linkHa
     // FIXME: We may want to track visited links per WebPageGroup rather than per WebContext.
     for (const auto& webPage : m_pageMap.values()) {
         if (Page* page = webPage->corePage()) {
-            for (auto linkHash : linkHashes)
+            for (auto& linkHash : linkHashes)
                 page->invalidateStylesForLink(linkHash);
         }
     }