- Revision
- 164947
- Author
- [email protected]
- Date
- 2014-03-02 12:34:51 -0800 (Sun, 02 Mar 2014)
Log Message
DocumentLoader should keep maps of ResourceLoaders instead of sets
https://bugs.webkit.org/show_bug.cgi?id=129388
Reviewed by Darin Adler.
For web replay, we need to be able to pull a ResourceLoader instance by
identifier from the DocumentLoader. This is easy to do if we convert
ResourceLoaderSet to ResourceLoaderMap, keyed by the loader's identifier.
Added assertions whenever adding or removing from the map to ensure
that we don't try to add duplicates or resources with zero identifiers.
No new tests required. No functionality was added.
* loader/DocumentLoader.cpp:
(WebCore::cancelAll):
(WebCore::setAllDefersLoading):
(WebCore::areAllLoadersPageCacheAcceptable):
(WebCore::DocumentLoader::addSubresourceLoader):
(WebCore::DocumentLoader::removeSubresourceLoader):
(WebCore::DocumentLoader::addPlugInStreamLoader):
(WebCore::DocumentLoader::removePlugInStreamLoader):
(WebCore::DocumentLoader::subresourceLoaderFinishedLoadingOnePart):
* loader/DocumentLoader.h:
* loader/NetscapePlugInStreamLoader.cpp:
(WebCore::NetscapePlugInStreamLoader::create): Only add the loader
to the document loader's map if it initialized successfully.
The old code was probably leaking resource loaders that failed to
initialize.
* loader/mac/DocumentLoaderMac.cpp:
(WebCore::scheduleAll):
(WebCore::unscheduleAll):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (164946 => 164947)
--- trunk/Source/WebCore/ChangeLog 2014-03-02 19:52:16 UTC (rev 164946)
+++ trunk/Source/WebCore/ChangeLog 2014-03-02 20:34:51 UTC (rev 164947)
@@ -1,3 +1,39 @@
+2014-03-02 Brian Burg <[email protected]>
+
+ DocumentLoader should keep maps of ResourceLoaders instead of sets
+ https://bugs.webkit.org/show_bug.cgi?id=129388
+
+ Reviewed by Darin Adler.
+
+ For web replay, we need to be able to pull a ResourceLoader instance by
+ identifier from the DocumentLoader. This is easy to do if we convert
+ ResourceLoaderSet to ResourceLoaderMap, keyed by the loader's identifier.
+
+ Added assertions whenever adding or removing from the map to ensure
+ that we don't try to add duplicates or resources with zero identifiers.
+
+ No new tests required. No functionality was added.
+
+ * loader/DocumentLoader.cpp:
+ (WebCore::cancelAll):
+ (WebCore::setAllDefersLoading):
+ (WebCore::areAllLoadersPageCacheAcceptable):
+ (WebCore::DocumentLoader::addSubresourceLoader):
+ (WebCore::DocumentLoader::removeSubresourceLoader):
+ (WebCore::DocumentLoader::addPlugInStreamLoader):
+ (WebCore::DocumentLoader::removePlugInStreamLoader):
+ (WebCore::DocumentLoader::subresourceLoaderFinishedLoadingOnePart):
+ * loader/DocumentLoader.h:
+ * loader/NetscapePlugInStreamLoader.cpp:
+ (WebCore::NetscapePlugInStreamLoader::create): Only add the loader
+ to the document loader's map if it initialized successfully.
+ The old code was probably leaking resource loaders that failed to
+ initialize.
+
+ * loader/mac/DocumentLoaderMac.cpp:
+ (WebCore::scheduleAll):
+ (WebCore::unscheduleAll):
+
2014-03-02 Dirkjan Ochtman <[email protected]>
Support ENABLE_ENCRYPTED_MEDIA in cmake builds
Modified: trunk/Source/WebCore/loader/DocumentLoader.cpp (164946 => 164947)
--- trunk/Source/WebCore/loader/DocumentLoader.cpp 2014-03-02 19:52:16 UTC (rev 164946)
+++ trunk/Source/WebCore/loader/DocumentLoader.cpp 2014-03-02 20:34:51 UTC (rev 164947)
@@ -78,28 +78,26 @@
namespace WebCore {
-static void cancelAll(const ResourceLoaderSet& loaders)
+static void cancelAll(const ResourceLoaderMap& loaders)
{
Vector<RefPtr<ResourceLoader>> loadersCopy;
- copyToVector(loaders, loadersCopy);
- size_t size = loadersCopy.size();
- for (size_t i = 0; i < size; ++i)
- loadersCopy[i]->cancel();
+ copyValuesToVector(loaders, loadersCopy);
+ for (auto& loader : loadersCopy)
+ loader->cancel();
}
-static void setAllDefersLoading(const ResourceLoaderSet& loaders, bool defers)
+static void setAllDefersLoading(const ResourceLoaderMap& loaders, bool defers)
{
Vector<RefPtr<ResourceLoader>> loadersCopy;
- copyToVector(loaders, loadersCopy);
- size_t size = loadersCopy.size();
- for (size_t i = 0; i < size; ++i)
- loadersCopy[i]->setDefersLoading(defers);
+ copyValuesToVector(loaders, loadersCopy);
+ for (auto& loader : loadersCopy)
+ loader->setDefersLoading(defers);
}
-static bool areAllLoadersPageCacheAcceptable(const ResourceLoaderSet& loaders)
+static bool areAllLoadersPageCacheAcceptable(const ResourceLoaderMap& loaders)
{
Vector<RefPtr<ResourceLoader>> loadersCopy;
- copyToVector(loaders, loadersCopy);
+ copyValuesToVector(loaders, loadersCopy);
for (auto& loader : loadersCopy) {
ResourceHandle* handle = loader->handle();
if (!handle)
@@ -1338,14 +1336,18 @@
// if we are just starting the main resource load.
if (!m_gotFirstByte)
return;
- ASSERT(!m_subresourceLoaders.contains(loader));
+ ASSERT(loader->identifier());
+ ASSERT(!m_subresourceLoaders.contains(loader->identifier()));
ASSERT(!mainResourceLoader() || mainResourceLoader() != loader);
- m_subresourceLoaders.add(loader);
+
+ m_subresourceLoaders.add(loader->identifier(), loader);
}
void DocumentLoader::removeSubresourceLoader(ResourceLoader* loader)
{
- if (!m_subresourceLoaders.remove(loader))
+ ASSERT(loader->identifier());
+
+ if (!m_subresourceLoaders.remove(loader->identifier()))
return;
checkLoadComplete();
if (Frame* frame = m_frame)
@@ -1354,12 +1356,18 @@
void DocumentLoader::addPlugInStreamLoader(ResourceLoader* loader)
{
- m_plugInStreamLoaders.add(loader);
+ ASSERT(loader->identifier());
+ ASSERT(!m_plugInStreamLoaders.contains(loader->identifier()));
+
+ m_plugInStreamLoaders.add(loader->identifier(), loader);
}
void DocumentLoader::removePlugInStreamLoader(ResourceLoader* loader)
{
- m_plugInStreamLoaders.remove(loader);
+ ASSERT(loader->identifier());
+ ASSERT(loader == m_plugInStreamLoaders.get(loader->identifier()));
+
+ m_plugInStreamLoaders.remove(loader->identifier());
checkLoadComplete();
}
@@ -1484,8 +1492,12 @@
void DocumentLoader::subresourceLoaderFinishedLoadingOnePart(ResourceLoader* loader)
{
- m_multipartSubresourceLoaders.add(loader);
- m_subresourceLoaders.remove(loader);
+ ASSERT(loader->identifier());
+ ASSERT(!m_multipartSubresourceLoaders.contains(loader->identifier()));
+ ASSERT(m_subresourceLoaders.contains(loader->identifier()));
+
+ m_multipartSubresourceLoaders.add(loader->identifier(), loader);
+ m_subresourceLoaders.remove(loader->identifier());
checkLoadComplete();
if (Frame* frame = m_frame)
frame->loader().checkLoadComplete();
Modified: trunk/Source/WebCore/loader/DocumentLoader.h (164946 => 164947)
--- trunk/Source/WebCore/loader/DocumentLoader.h 2014-03-02 19:52:16 UTC (rev 164946)
+++ trunk/Source/WebCore/loader/DocumentLoader.h 2014-03-02 20:34:51 UTC (rev 164947)
@@ -74,7 +74,7 @@
class SharedBuffer;
class SubstituteResource;
- typedef HashSet<RefPtr<ResourceLoader>> ResourceLoaderSet;
+ typedef HashMap<unsigned long, RefPtr<ResourceLoader>> ResourceLoaderMap;
typedef Vector<ResourceResponse> ResponseVector;
class DocumentLoader : public RefCounted<DocumentLoader>, private CachedRawResourceClient {
@@ -331,9 +331,9 @@
Ref<CachedResourceLoader> m_cachedResourceLoader;
CachedResourceHandle<CachedRawResource> m_mainResource;
- ResourceLoaderSet m_subresourceLoaders;
- ResourceLoaderSet m_multipartSubresourceLoaders;
- ResourceLoaderSet m_plugInStreamLoaders;
+ ResourceLoaderMap m_subresourceLoaders;
+ ResourceLoaderMap m_multipartSubresourceLoaders;
+ ResourceLoaderMap m_plugInStreamLoaders;
mutable DocumentWriter m_writer;
Modified: trunk/Source/WebCore/loader/NetscapePlugInStreamLoader.cpp (164946 => 164947)
--- trunk/Source/WebCore/loader/NetscapePlugInStreamLoader.cpp 2014-03-02 19:52:16 UTC (rev 164946)
+++ trunk/Source/WebCore/loader/NetscapePlugInStreamLoader.cpp 2014-03-02 20:34:51 UTC (rev 164947)
@@ -49,10 +49,10 @@
PassRefPtr<NetscapePlugInStreamLoader> NetscapePlugInStreamLoader::create(Frame* frame, NetscapePlugInStreamLoaderClient* client, const ResourceRequest& request)
{
RefPtr<NetscapePlugInStreamLoader> loader(adoptRef(new NetscapePlugInStreamLoader(frame, client)));
- loader->documentLoader()->addPlugInStreamLoader(loader.get());
if (!loader->init(request))
return 0;
+ loader->documentLoader()->addPlugInStreamLoader(loader.get());
return loader.release();
}
Modified: trunk/Source/WebCore/loader/mac/DocumentLoaderMac.cpp (164946 => 164947)
--- trunk/Source/WebCore/loader/mac/DocumentLoaderMac.cpp 2014-03-02 19:52:16 UTC (rev 164946)
+++ trunk/Source/WebCore/loader/mac/DocumentLoaderMac.cpp 2014-03-02 20:34:51 UTC (rev 164947)
@@ -35,22 +35,24 @@
namespace WebCore {
#if !PLATFORM(IOS)
-static void scheduleAll(const ResourceLoaderSet& loaders, SchedulePair* pair)
+static void scheduleAll(const ResourceLoaderMap& loaders, SchedulePair* pair)
{
- const ResourceLoaderSet copy = loaders;
- ResourceLoaderSet::const_iterator end = copy.end();
- for (ResourceLoaderSet::const_iterator it = copy.begin(); it != end; ++it)
- if (ResourceHandle* handle = (*it)->handle())
+ Vector<RefPtr<ResourceLoader>> loadersCopy;
+ copyValuesToVector(loaders, loadersCopy);
+ for (auto& loader : loadersCopy) {
+ if (ResourceHandle* handle = loader->handle())
handle->schedule(pair);
+ }
}
-static void unscheduleAll(const ResourceLoaderSet& loaders, SchedulePair* pair)
+static void unscheduleAll(const ResourceLoaderMap& loaders, SchedulePair* pair)
{
- const ResourceLoaderSet copy = loaders;
- ResourceLoaderSet::const_iterator end = copy.end();
- for (ResourceLoaderSet::const_iterator it = copy.begin(); it != end; ++it)
- if (ResourceHandle* handle = (*it)->handle())
+ Vector<RefPtr<ResourceLoader>> loadersCopy;
+ copyValuesToVector(loaders, loadersCopy);
+ for (auto& loader : loadersCopy) {
+ if (ResourceHandle* handle = loader->handle())
handle->unschedule(pair);
+ }
}
#endif
