[webkit-reviews] review granted: [Bug 129388] DocumentLoader should keep maps of ResourceLoaders instead of sets : [Attachment 225500] the patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Brian Burg <bburg at apple.com>'s
request for review:
Bug 129388: DocumentLoader should keep maps of ResourceLoaders instead of sets
https://bugs.webkit.org/show_bug.cgi?id=129388

Attachment 225500: the patch
https://bugs.webkit.org/attachment.cgi?id=225500&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=225500&action=review


Would be nice to find an even better idiom for the "copy values and iterate"
that doesn't require two extra lines of code and explicitly specifying the
vector type.

> Source/WebCore/loader/DocumentLoader.cpp:1343
> +    m_subresourceLoaders.set(loader->identifier(), loader);

Should use add instead of set. The only difference is that set will replace an
existing element if there is one, and set is implemented by doing an add and
then an additional write after the fact.

> Source/WebCore/loader/DocumentLoader.cpp:1362
> +    m_plugInStreamLoaders.set(loader->identifier(), loader);

Should use add instead of set. Same reasons as above.

> Source/WebCore/loader/DocumentLoader.cpp:1499
> +    m_multipartSubresourceLoaders.set(loader->identifier(), loader);

Should use add instead of set. Same reasons as above.

> Source/WebCore/loader/mac/DocumentLoaderMac.cpp:44
> +    for (auto& loader : loadersCopy)
> +	   if (ResourceHandle* handle = loader->handle())
>	       handle->schedule(pair);

Multi-line for loop body gets braces in WebKit coding style.

> Source/WebCore/loader/mac/DocumentLoaderMac.cpp:53
> +    for (auto& loader : loadersCopy)
> +	   if (ResourceHandle* handle = loader->handle())
>	       handle->unschedule(pair);

Multi-line for loop body gets braces in WebKit coding style.