SSIA.
Created attachment 227732 [details] Patch
Comment on attachment 227732 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227732&action=review > Source/WebCore/platform/text/enchant/TextCheckerEnchant.cpp:57 > - for (Vector<EnchantDict*>::const_iterator iter = m_enchantDictionaries.begin(); iter != m_enchantDictionaries.end(); ++iter) > - enchant_dict_add_to_session(*iter, word.utf8().data(), -1); > + for (auto iter : m_enchantDictionaries) > + enchant_dict_add_to_session(iter, word.utf8().data(), -1); Use auto&, or even const auto& where possible in range-based for loops. This avoids unnecessary copies of the iterated objects. The benefit in this specific iteration is perhaps small since you'd be copying a pointer, but it's consistent with such loops elsewhere in the WebKit code base, and is generally a good practice. Also, with this type of iteration 'iter' can be replaced with a more appropriate, verbose name. In this case perhaps 'dictionay' would suit better. This applies to all the other loops that are being changed in this patch. > Source/WebCore/platform/text/enchant/TextCheckerEnchant.cpp:143 > - for (Vector<String>::const_iterator iter = languages.begin(); iter != languages.end(); ++iter) { > - CString currentLanguage = iter->utf8(); > + for (auto iter : languages) { > + CString currentLanguage = iter.utf8(); As a contrast, this copies every String object in the vector since you're not taking a reference of the objects.
Comment on attachment 227732 [details] Patch Attachment 227732 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/4920358457049088 New failing tests: media/W3C/audio/canPlayType/canPlayType_application_octet_stream.html
Created attachment 227739 [details] Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-10 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.5
(In reply to comment #2) > (From update of attachment 227732 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=227732&action=review > > > Source/WebCore/platform/text/enchant/TextCheckerEnchant.cpp:57 > > - for (Vector<EnchantDict*>::const_iterator iter = m_enchantDictionaries.begin(); iter != m_enchantDictionaries.end(); ++iter) > > - enchant_dict_add_to_session(*iter, word.utf8().data(), -1); > > + for (auto iter : m_enchantDictionaries) > > + enchant_dict_add_to_session(iter, word.utf8().data(), -1); > > Use auto&, or even const auto& where possible in range-based for loops. This avoids unnecessary copies of the iterated objects. The benefit in this specific iteration is perhaps small since you'd be copying a pointer, but it's consistent with such loops elsewhere in the WebKit code base, and is generally a good practice. > > Also, with this type of iteration 'iter' can be replaced with a more appropriate, verbose name. In this case perhaps 'dictionay' would suit better. > > This applies to all the other loops that are being changed in this patch. > > > Source/WebCore/platform/text/enchant/TextCheckerEnchant.cpp:143 > > - for (Vector<String>::const_iterator iter = languages.begin(); iter != languages.end(); ++iter) { > > - CString currentLanguage = iter->utf8(); > > + for (auto iter : languages) { > > + CString currentLanguage = iter.utf8(); > > As a contrast, this copies every String object in the vector since you're not taking a reference of the objects. Thanks for kind advice Zan! I'll upload a new patch.
Created attachment 228017 [details] Patch
Comment on attachment 228017 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=228017&action=review > Source/WebCore/platform/text/enchant/TextCheckerEnchant.cpp:56 > + for (const auto& dictionary : m_enchantDictionaries) I would have suggested auto& without the const for all of these loops. While I understand that we can say const, it adds very little.
Created attachment 228136 [details] Patch Applied Darin's comment.
Comment on attachment 228136 [details] Patch Attachment 228136 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/5089131008884736 New failing tests: compositing/columns/composited-rl-paginated-repaint.html
Created attachment 228141 [details] Archive of layout-test-results from webkit-ews-04 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-04 Port: mac-mountainlion Platform: Mac OS X 10.8.5
(In reply to comment #10) > Created an attachment (id=228141) [details] > Archive of layout-test-results from webkit-ews-04 for mac-mountainlion > > The attached test failures were seen while running run-webkit-tests on the mac-ews. > Bot: webkit-ews-04 Port: mac-mountainlion Platform: Mac OS X 10.8.5 Flaky test result.
Comment on attachment 228136 [details] Patch Clearing flags on attachment: 228136 Committed r166474: <http://trac.webkit.org/changeset/166474>
All reviewed patches have been landed. Closing bug.