Attachment 225500 Details for Bug 129388 – the patch

[patch] the patch

bug-129388-20140228152548.patch (text/plain), 9.40 KB, created by Blaze Burg on 2014-02-28 15:25:39 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Blaze Burg

Created: 2014-02-28 15:25:39 PST

Size: 9.40 KB

patch

obsolete

>Subversion Revision: 164843
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index d5f4ac24aeae248b47e12af26776014fcc5391d4..9dd907937241d73e68e2c10581a369156610e07c 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,39 @@
>+2014-02-28  Brian Burg  <bburg@apple.com>
>+
>+        DocumentLoader should keep maps of ResourceLoaders instead of sets
>+        https://bugs.webkit.org/show_bug.cgi?id=129388
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        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-02-17  Myles C. Maxfield  <mmaxfield@apple.com>
> 
>         text-decoration-skip: ink does not skip over SVG fonts
>diff --git a/Source/WebCore/loader/DocumentLoader.cpp b/Source/WebCore/loader/DocumentLoader.cpp
>index 59089ac221a3fba7661da657d19021fbadcb1f74..5a3b146b16e7fa01dc92e555f8f7350cf5db97ad 100644
>--- a/Source/WebCore/loader/DocumentLoader.cpp
>+++ b/Source/WebCore/loader/DocumentLoader.cpp
>@@ -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 @@ void DocumentLoader::addSubresourceLoader(ResourceLoader* loader)
>     // 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.set(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::removeSubresourceLoader(ResourceLoader* loader)
> 
> void DocumentLoader::addPlugInStreamLoader(ResourceLoader* loader)
> {
>-    m_plugInStreamLoaders.add(loader);
>+    ASSERT(loader->identifier());
>+    ASSERT(!m_plugInStreamLoaders.contains(loader->identifier()));
>+
>+    m_plugInStreamLoaders.set(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::clearMainResource()
> 
> 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.set(loader->identifier(), loader);
>+    m_subresourceLoaders.remove(loader->identifier());
>     checkLoadComplete();
>     if (Frame* frame = m_frame)
>         frame->loader().checkLoadComplete();    
>diff --git a/Source/WebCore/loader/DocumentLoader.h b/Source/WebCore/loader/DocumentLoader.h
>index caeb36606ca162e14f0c1e177a3e850964068020..75415c1d15efb03edffe908d8b24e703f5274c8c 100644
>--- a/Source/WebCore/loader/DocumentLoader.h
>+++ b/Source/WebCore/loader/DocumentLoader.h
>@@ -74,7 +74,7 @@ namespace WebCore {
>     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 @@ namespace WebCore {
>         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;
> 
>diff --git a/Source/WebCore/loader/NetscapePlugInStreamLoader.cpp b/Source/WebCore/loader/NetscapePlugInStreamLoader.cpp
>index b8c4556fa96403d28a850d851a5b773be2d8e755..9163cd35ee97a5c96001aa396558f860f1d7d056 100644
>--- a/Source/WebCore/loader/NetscapePlugInStreamLoader.cpp
>+++ b/Source/WebCore/loader/NetscapePlugInStreamLoader.cpp
>@@ -49,10 +49,10 @@ NetscapePlugInStreamLoader::~NetscapePlugInStreamLoader()
> 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();
> }
> 
>diff --git a/Source/WebCore/loader/mac/DocumentLoaderMac.cpp b/Source/WebCore/loader/mac/DocumentLoaderMac.cpp
>index 95af7906742d87ddc60edfee9b31671d0813f96a..8dd161e54294ed013e3ac31a253b191d7ca87c61 100644
>--- a/Source/WebCore/loader/mac/DocumentLoaderMac.cpp
>+++ b/Source/WebCore/loader/mac/DocumentLoaderMac.cpp
>@@ -35,21 +35,21 @@
> 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

Flags:

darin: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 129388: 225500