Modified: trunk/Source/WebCore/ChangeLog (104755 => 104756)
--- trunk/Source/WebCore/ChangeLog 2012-01-11 23:51:15 UTC (rev 104755)
+++ trunk/Source/WebCore/ChangeLog 2012-01-11 23:54:28 UTC (rev 104756)
@@ -1,3 +1,29 @@
+2012-01-11 Nate Chapin <[email protected]>
+
+ SubresourceLoader cleanup post r100311.
+ 1. Simplify matching incrementRequestCount()/decrementRequestCount() calls.
+ 2. Remove CachedImage custom code from SubresourceLoader.
+ 3. Add a bunch of ASSERTs.
+ 4. Remove the multipart-only call to didReceiveData() from didReceiveResponse(),
+ since didReceiveData() would get called immediately after anyway.
+ https://bugs.webkit.org/show_bug.cgi?id=75887
+
+ Reviewed by Adam Barth.
+
+ No new tests, refactor only.
+
+ * loader/SubresourceLoader.cpp:
+ (WebCore::SubresourceLoader::didReceiveResponse): Remove multipart special case, handle it in didReceiveData().
+ (WebCore::SubresourceLoader::didReceiveData): Handle multipart state here, since we will receive only one
+ didReceiveData() call per multipart segment, but no didFinishLoading() call.
+ * loader/SubresourceLoader.h: Add a RequestCountTracker subclass to reduce complexity of ensuring we don't
+ decrement request count twice on CachedResourceLoader.
+ * loader/cache/CachedImage.cpp:
+ (WebCore::CachedImage::setResponse): Move CachedImage::clear() call out of SubresourceLoader, since it's
+ kind of a layering violation as is.
+ * loader/cache/CachedImage.h:
+ * loader/cache/CachedResource.cpp:
+
2012-01-11 Joshua Bell <[email protected]>
IndexedDB: Version change transaction should abort if constraints fail during createIndex
Modified: trunk/Source/WebCore/loader/SubresourceLoader.cpp (104755 => 104756)
--- trunk/Source/WebCore/loader/SubresourceLoader.cpp 2012-01-11 23:51:15 UTC (rev 104755)
+++ trunk/Source/WebCore/loader/SubresourceLoader.cpp 2012-01-11 23:54:28 UTC (rev 104756)
@@ -29,7 +29,6 @@
#include "config.h"
#include "SubresourceLoader.h"
-#include "CachedImage.h"
#include "CachedResourceLoader.h"
#include "Document.h"
#include "DocumentLoader.h"
@@ -47,12 +46,25 @@
DEFINE_DEBUG_ONLY_GLOBAL(WTF::RefCountedLeakCounter, subresourceLoaderCounter, ("SubresourceLoader"));
+SubresourceLoader::RequestCountTracker::RequestCountTracker(CachedResourceLoader* cachedResourceLoader, CachedResource* resource)
+ : m_cachedResourceLoader(cachedResourceLoader)
+ , m_resource(resource)
+{
+ m_cachedResourceLoader->incrementRequestCount(m_resource);
+}
+
+SubresourceLoader::RequestCountTracker::~RequestCountTracker()
+{
+ m_cachedResourceLoader->decrementRequestCount(m_resource);
+}
+
SubresourceLoader::SubresourceLoader(Frame* frame, CachedResource* resource, const ResourceLoaderOptions& options)
: ResourceLoader(frame, options)
, m_resource(resource)
, m_document(frame->document())
, m_loadingMultipartContent(false)
, m_state(Uninitialized)
+ , m_requestCountTracker(adoptPtr(new RequestCountTracker(frame->document()->cachedResourceLoader(), resource)))
{
#ifndef NDEBUG
subresourceLoaderCounter.increment();
@@ -61,6 +73,9 @@
SubresourceLoader::~SubresourceLoader()
{
+ ASSERT(m_state != Initialized);
+ ASSERT(!m_document);
+ ASSERT(reachedTerminalState());
#ifndef NDEBUG
subresourceLoaderCounter.decrement();
#endif
@@ -119,6 +134,7 @@
if (!ResourceLoader::init(request))
return false;
+ ASSERT(!reachedTerminalState());
m_state = Initialized;
m_documentLoader->addSubresourceLoader(this);
return true;
@@ -141,6 +157,7 @@
void SubresourceLoader::didSendData(unsigned long long bytesSent, unsigned long long totalBytesToBeSent)
{
+ ASSERT(m_state == Initialized);
RefPtr<SubresourceLoader> protect(this);
m_resource->didSendData(bytesSent, totalBytesToBeSent);
}
@@ -148,6 +165,7 @@
void SubresourceLoader::didReceiveResponse(const ResourceResponse& response)
{
ASSERT(!response.isNull());
+ ASSERT(m_state == Initialized);
// Reference the object in this method since the additional processing can do
// anything including removing the last reference to this object; one example of this is 3266216.
@@ -174,38 +192,24 @@
return;
ResourceLoader::didReceiveResponse(response);
- if (m_loadingMultipartContent) {
- ASSERT(m_resource->isImage());
- static_cast<CachedImage*>(m_resource)->clear();
- } else if (response.isMultipart()) {
+ if (response.isMultipart()) {
m_loadingMultipartContent = true;
// We don't count multiParts in a CachedResourceLoader's request count
- m_document->cachedResourceLoader()->decrementRequestCount(m_resource);
+ m_requestCountTracker.clear();
+ setShouldBufferData(DoNotBufferData);
if (!m_resource->isImage()) {
cancel();
return;
}
}
-
- RefPtr<SharedBuffer> buffer = resourceData();
- if (m_loadingMultipartContent && buffer && buffer->size()) {
- // Since a subresource loader does not load multipart sections progressively,
- // deliver the previously received data to the loader all at once now.
- // Then clear the data to make way for the next multipart section.
- sendDataToResource(buffer->data(), buffer->size());
- clearResourceData();
-
- // After the first multipart section is complete, signal to delegates that this load is "finished"
- m_documentLoader->subresourceLoaderFinishedLoadingOnePart(this);
- didFinishLoadingOnePart(0);
- }
}
void SubresourceLoader::didReceiveData(const char* data, int length, long long encodedDataLength, bool allAtOnce)
{
ASSERT(!m_resource->resourceToRevalidate());
ASSERT(!m_resource->errorOccurred());
+ ASSERT(m_state == Initialized);
// Reference the object in this method since the additional processing can do
// anything including removing the last reference to this object; one example of this is 3266216.
RefPtr<SubresourceLoader> protect(this);
@@ -214,8 +218,14 @@
if (errorLoadingResource())
return;
- if (!m_loadingMultipartContent)
- sendDataToResource(data, length);
+ sendDataToResource(data, length);
+
+ if (m_loadingMultipartContent) {
+ // Since a subresource loader does not load multipart sections progressively, data was delivered to the loader all at once.
+ // After the first multipart section is complete, signal to delegates that this load is "finished"
+ m_documentLoader->subresourceLoaderFinishedLoadingOnePart(this);
+ didFinishLoadingOnePart(0);
+ }
}
bool SubresourceLoader::errorLoadingResource()
@@ -231,21 +241,13 @@
void SubresourceLoader::sendDataToResource(const char* data, int length)
{
- // There are two cases where we might need to create our own SharedBuffer instead of copying the one in ResourceLoader.
- // (1) Multipart content: The loader delivers the data in a multipart section all at once, then sends eof.
- // The resource data will change as the next part is loaded, so we need to make a copy.
- // (2) Our client requested that the data not be buffered at the ResourceLoader level via ResourceLoaderOptions. In this case,
- // ResourceLoader::resourceData() will be null. However, unlike the multipart case, we don't want to tell the CachedResource
- // that all data has been received yet.
- if (m_loadingMultipartContent || !resourceData()) {
- RefPtr<SharedBuffer> copiedData = SharedBuffer::create(data, length);
- m_resource->data(copiedData.release(), m_loadingMultipartContent);
- } else
- m_resource->data(resourceData(), false);
+ RefPtr<SharedBuffer> buffer = resourceData() ? resourceData() : SharedBuffer::create(data, length);
+ m_resource->data(buffer.release(), m_loadingMultipartContent);
}
void SubresourceLoader::didReceiveCachedMetadata(const char* data, int length)
{
+ ASSERT(m_state == Initialized);
ASSERT(!m_resource->resourceToRevalidate());
m_resource->setSerializedCachedMetadata(data, length);
}
@@ -303,9 +305,7 @@
{
ASSERT(!reachedTerminalState());
if (m_state != Uninitialized) {
- if (!m_loadingMultipartContent && m_state != Releasing)
- m_document->cachedResourceLoader()->decrementRequestCount(m_resource);
- m_state = Releasing;
+ m_requestCountTracker.clear();
m_document->cachedResourceLoader()->loadDone();
if (reachedTerminalState())
return;
Modified: trunk/Source/WebCore/loader/SubresourceLoader.h (104755 => 104756)
--- trunk/Source/WebCore/loader/SubresourceLoader.h 2012-01-11 23:51:15 UTC (rev 104755)
+++ trunk/Source/WebCore/loader/SubresourceLoader.h 2012-01-11 23:54:28 UTC (rev 104756)
@@ -37,6 +37,7 @@
namespace WebCore {
class CachedResource;
+class CachedResourceLoader;
class Document;
class ResourceRequest;
@@ -78,14 +79,23 @@
Uninitialized,
Initialized,
Revalidating,
- Finishing,
- Releasing
+ Finishing
};
+ class RequestCountTracker {
+ public:
+ RequestCountTracker(CachedResourceLoader*, CachedResource*);
+ ~RequestCountTracker();
+ private:
+ CachedResourceLoader* m_cachedResourceLoader;
+ CachedResource* m_resource;
+ };
+
CachedResource* m_resource;
RefPtr<Document> m_document;
bool m_loadingMultipartContent;
SubresourceLoaderState m_state;
+ OwnPtr<RequestCountTracker> m_requestCountTracker;
};
}
Modified: trunk/Source/WebCore/loader/cache/CachedImage.h (104755 => 104756)
--- trunk/Source/WebCore/loader/cache/CachedImage.h 2012-01-11 23:51:15 UTC (rev 104755)
+++ trunk/Source/WebCore/loader/cache/CachedImage.h 2012-01-11 23:54:28 UTC (rev 104756)
@@ -75,14 +75,12 @@
virtual void data(PassRefPtr<SharedBuffer> data, bool allDataReceived);
virtual void error(CachedResource::Status);
+ virtual void setResponse(const ResourceResponse&);
// For compatibility, images keep loading even if there are HTTP errors.
virtual bool shouldIgnoreHTTPStatusCodeErrors() const { return true; }
virtual bool isImage() const { return true; }
-
- void clear();
-
bool stillNeedsLoad() const { return !errorOccurred() && status() == Unknown && !isLoading(); }
void load();
@@ -97,6 +95,8 @@
private:
Image* lookupOrCreateImageForRenderer(const RenderObject*);
+ void clear();
+
void createImage();
size_t maximumDecodedImageSize();
// If not null, changeRect is the changed part of the image.
Modified: trunk/Source/WebCore/loader/cache/CachedResource.cpp (104755 => 104756)
--- trunk/Source/WebCore/loader/cache/CachedResource.cpp 2012-01-11 23:51:15 UTC (rev 104755)
+++ trunk/Source/WebCore/loader/cache/CachedResource.cpp 2012-01-11 23:54:28 UTC (rev 104756)
@@ -214,7 +214,7 @@
m_resourceRequest.setPriority(loadPriority());
m_loader = resourceLoadScheduler()->scheduleSubresourceLoad(cachedResourceLoader->document()->frame(), this, m_resourceRequest, m_resourceRequest.priority(), options);
- if (!m_loader || m_loader->reachedTerminalState()) {
+ if (!m_loader) {
// FIXME: What if resources in other frames were waiting for this revalidation?
LOG(ResourceLoading, "Cannot start loading '%s'", url().string().latin1().data());
if (m_resourceToRevalidate)
@@ -224,7 +224,6 @@
}
m_status = Pending;
- cachedResourceLoader->incrementRequestCount(this);
}
void CachedResource::checkNotify()
