Changeset 143569 in webkit

Timestamp:

Feb 21, 2013, 12:27:10 AM (12 years ago)

Author:

ap@apple.com

Message:

Stop hardcoding knowledge about blob protocol in ResourceHandle
​https://bugs.webkit.org/show_bug.cgi?id=110382

Reviewed by Anders Carlsson.

• platform/network/BlobRegistry.h:
• platform/network/chromium/BlobRegistryProxy.h: Removed loadResourceSynchronously(). Chromium did not even implement it, and cross-platform code doesn't need it any more either.

• platform/network/BlobRegistryImpl.cpp: (WebCore::loadResourceSynchronously): A function to load a blob synchronously, suitable for being in a ResourceHandle map. (WebCore::registerBlobResourceHandleConstructor): Register loadResourceSynchronously too. (WebCore::BlobRegistryImpl::createResourceHandle): Don't make policy decisions here, it's just a registry. Moved code for checking request method down to BlobResourceHandle.

• platform/network/BlobRegistryImpl.h: Removed BlobRegistryImpl::loadResourceSynchronously(). A registry shouldn't load resources, and it already has enough accessors to implement this externally.

• platform/network/BlobResourceHandle.h:
• platform/network/BlobResourceHandle.cpp: (WebCore::BlobResourceHandle::createAsync): Renamed from create(), to make it clear that it's not OK to call from synchronous loader. Moved method check here. (WebCore::BlobResourceHandle::loadResourceSynchronously): Moved method check here. We used to fall through to NSURLConnection based loader when method was not GET, which of course resulted in an error eventually, but was just wrong.

• platform/network/ResourceHandle.cpp: (WebCore::builtinResourceHandleSynchronousLoaderMap): Added a map for synchronous case. (WebCore::ResourceHandle::registerBuiltinSynchronousLoader): Ditto. (WebCore::ResourceHandle::loadResourceSynchronously): We no longer need to hardcode knowledge about blobRegistry and blobs here.

• platform/network/ResourceHandle.h: Added registerBuiltinSynchronousLoader().

• platform/network/chromium/ResourceHandle.h: Removed loadResourceSynchronously(), which was not implemented for Chromium.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/network/BlobRegistry.h (modified) (1 diff)
• platform/network/BlobRegistryImpl.cpp (modified) (4 diffs)
• platform/network/BlobRegistryImpl.h (modified) (2 diffs)
• platform/network/BlobResourceHandle.cpp (modified) (2 diffs)
• platform/network/BlobResourceHandle.h (modified) (1 diff)
• platform/network/ResourceHandle.cpp (modified) (2 diffs)
• platform/network/ResourceHandle.h (modified) (1 diff)
• platform/network/chromium/BlobRegistryProxy.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog ¶

@@ +1 @@
+2013-02-20  Alexey Proskuryakov  <ap@apple.com>
+
+        Stop hardcoding knowledge about blob protocol in ResourceHandle
+        https://bugs.webkit.org/show_bug.cgi?id=110382
+
+        Reviewed by Anders Carlsson.
+
+        * platform/network/BlobRegistry.h:
+        * platform/network/chromium/BlobRegistryProxy.h:
+        Removed loadResourceSynchronously(). Chromium did not even implement it, and
+        cross-platform code doesn't need it any more either.
+
+        * platform/network/BlobRegistryImpl.cpp:
+        (WebCore::loadResourceSynchronously): A function to load a blob synchronously,
+        suitable for being in a ResourceHandle map.
+        (WebCore::registerBlobResourceHandleConstructor): Register loadResourceSynchronously too.
+        (WebCore::BlobRegistryImpl::createResourceHandle): Don't make policy decisions here,
+        it's just a registry. Moved code for checking request method down to BlobResourceHandle.
+
+        * platform/network/BlobRegistryImpl.h: Removed BlobRegistryImpl::loadResourceSynchronously().
+        A registry shouldn't load resources, and it already has enough accessors to implement
+        this externally.
+
+        * platform/network/BlobResourceHandle.h:
+        * platform/network/BlobResourceHandle.cpp:
+        (WebCore::BlobResourceHandle::createAsync): Renamed from create(), to make it clear
+        that it's not OK to call from synchronous loader. Moved method check here.
+        (WebCore::BlobResourceHandle::loadResourceSynchronously): Moved method check here.
+        We used to fall through to NSURLConnection based loader when method was not GET,
+        which of course resulted in an error eventually, but was just wrong.
+
+        * platform/network/ResourceHandle.cpp:
+        (WebCore::builtinResourceHandleSynchronousLoaderMap): Added a map for synchronous case.
+        (WebCore::ResourceHandle::registerBuiltinSynchronousLoader): Ditto.
+        (WebCore::ResourceHandle::loadResourceSynchronously): We no longer need to hardcode
+        knowledge about blobRegistry and blobs here.
+
+        * platform/network/ResourceHandle.h: Added registerBuiltinSynchronousLoader().
+
+        * platform/network/chromium/ResourceHandle.h: Removed loadResourceSynchronously(),
+        which was not implemented for Chromium.
+
 2013-02-20  Vsevolod Vlasov  <vsevik@chromium.org>
 

trunk/Source/WebCore/platform/network/BlobRegistry.h ¶

@@ -60 +60 @@
 
     virtual void unregisterBlobURL(const KURL&) = 0;
-    virtual bool loadResourceSynchronously(const ResourceRequest&, ResourceError&, ResourceResponse&, Vector<char>& data) = 0;
 
 protected:

trunk/Source/WebCore/platform/network/BlobRegistryImpl.cpp ¶

@@ -1 +1 @@
 /*
  * Copyright (C) 2010 Google Inc. All rights reserved.
+ * Copyright (C) 2013 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -59 +60 @@
 }
 
+static void loadResourceSynchronously(NetworkingContext*, const ResourceRequest& request, StoredCredentials, ResourceError& error, ResourceResponse& response, Vector<char>& data)
+{
+    RefPtr<BlobStorageData> blobData = static_cast<BlobRegistryImpl&>(blobRegistry()).getBlobDataFromURL(request.url());
+    BlobResourceHandle::loadResourceSynchronously(blobData, request, error, response, data);
+}
+
 static void registerBlobResourceHandleConstructor()
 {
@@ -64 +71 @@
     if (!didRegister) {
         ResourceHandle::registerBuiltinConstructor("blob", createResourceHandle);
+        ResourceHandle::registerBuiltinSynchronousLoader("blob", loadResourceSynchronously);
         didRegister = true;
     }
@@ -75 +83 @@
 #endif
 
-bool BlobRegistryImpl::shouldLoadResource(const ResourceRequest& request) const
-{
-    // If the resource is not fetched using the GET method, bail out.
-    if (!equalIgnoringCase(request.httpMethod(), "GET"))
-        return false;
-
-    return true;
-}
-
 PassRefPtr<ResourceHandle> BlobRegistryImpl::createResourceHandle(const ResourceRequest& request, ResourceHandleClient* client)
 {
-    if (!shouldLoadResource(request))
+    RefPtr<BlobResourceHandle> handle = BlobResourceHandle::createAsync(m_blobs.get(request.url().string()), request, client);
+    if (!handle)
         return 0;
 
-    RefPtr<BlobResourceHandle> handle = BlobResourceHandle::create(m_blobs.get(request.url().string()), request, client);
     handle->start();
     return handle.release();
-}
-
-bool BlobRegistryImpl::loadResourceSynchronously(const ResourceRequest& request, ResourceError& error, ResourceResponse& response, Vector<char>& data)
-{
-    if (!shouldLoadResource(request))
-        return false;
-
-    BlobResourceHandle::loadResourceSynchronously(m_blobs.get(request.url().string()), request, error, response, data);
-    return true;
 }
 

trunk/Source/WebCore/platform/network/BlobRegistryImpl.h ¶

@@ -58 +58 @@
     virtual void registerBlobURL(const KURL&, const KURL& srcURL);
     virtual void unregisterBlobURL(const KURL&);
-    virtual bool loadResourceSynchronously(const ResourceRequest&, ResourceError&, ResourceResponse&, Vector<char>& data);
 
     PassRefPtr<BlobStorageData> getBlobDataFromURL(const KURL&) const;
@@ -65 +64 @@
 
 private:
-    bool shouldLoadResource(const ResourceRequest& request) const;
     void appendStorageItems(BlobStorageData*, const BlobDataItemList&);
     void appendStorageItems(BlobStorageData*, const BlobDataItemList&, long long offset, long long length);

trunk/Source/WebCore/platform/network/BlobResourceHandle.cpp ¶

@@ -71 +71 @@
     rangeError = 3,
     notReadableError = 4,
+    methodNotAllowed = 5
 };
 
@@ -133 +134 @@
 // BlobResourceHandle
 
-// static
+PassRefPtr<BlobResourceHandle> BlobResourceHandle::createAsync(PassRefPtr<BlobStorageData> blobData, const ResourceRequest& request, ResourceHandleClient* client)
+{
+    // FIXME: Should probably call didFail() instead of blocking the load without explanation.
+    if (!equalIgnoringCase(request.httpMethod(), "GET"))
+        return 0;
+
+    return adoptRef(new BlobResourceHandle(blobData, request, client, true));
+}
+
 void BlobResourceHandle::loadResourceSynchronously(PassRefPtr<BlobStorageData> blobData, const ResourceRequest& request, ResourceError& error, ResourceResponse& response, Vector<char>& data)
 {
+    if (!equalIgnoringCase(request.httpMethod(), "GET")) {
+        error = ResourceError(webKitBlobResourceDomain, methodNotAllowed, response.url(), "Request method must be GET");
+        return;
+    }
+
     BlobResourceSynchronousLoader loader(error, response, data);
-    RefPtr<BlobResourceHandle> handle = BlobResourceHandle::create(blobData, request, &loader, false);
+    RefPtr<BlobResourceHandle> handle = adoptRef(new BlobResourceHandle(blobData, request, &loader, false));
     handle->start();
 }

trunk/Source/WebCore/platform/network/BlobResourceHandle.h ¶

@@ -51 +51 @@
 class BlobResourceHandle : public FileStreamClient, public ResourceHandle  {
 public:
-    static PassRefPtr<BlobResourceHandle> create(PassRefPtr<BlobStorageData> blobData, const ResourceRequest& request, ResourceHandleClient* client, bool async = true)
-    {
-        return adoptRef(new BlobResourceHandle(blobData, request, client, async));
-    }
+    static PassRefPtr<BlobResourceHandle> createAsync(PassRefPtr<BlobStorageData>, const ResourceRequest&, ResourceHandleClient*);
 
     static void loadResourceSynchronously(PassRefPtr<BlobStorageData> blobData, const ResourceRequest& request, ResourceError& error, ResourceResponse& response, Vector<char>& data);

trunk/Source/WebCore/platform/network/ResourceHandle.cpp ¶

@@ -54 +54 @@
 }
 
+typedef HashMap<AtomicString, ResourceHandle::BuiltinSynchronousLoader> BuiltinResourceHandleSynchronousLoaderMap;
+static BuiltinResourceHandleSynchronousLoaderMap& builtinResourceHandleSynchronousLoaderMap()
+{
+    ASSERT(isMainThread());
+    DEFINE_STATIC_LOCAL(BuiltinResourceHandleSynchronousLoaderMap, map, ());
+    return map;
+}
+
+void ResourceHandle::registerBuiltinSynchronousLoader(const AtomicString& protocol, ResourceHandle::BuiltinSynchronousLoader loader)
+{
+    builtinResourceHandleSynchronousLoaderMap().add(protocol, loader);
+}
+
 ResourceHandle::ResourceHandle(NetworkingContext* context, const ResourceRequest& request, ResourceHandleClient* client, bool defersLoading, bool shouldContentSniff)
     : d(adoptPtr(new ResourceHandleInternal(this, context, request, client, defersLoading, shouldContentSniff && shouldContentSniffURL(request.url()))))
@@ -116 +129 @@
 void ResourceHandle::loadResourceSynchronously(NetworkingContext* context, const ResourceRequest& request, StoredCredentials storedCredentials, ResourceError& error, ResourceResponse& response, Vector<char>& data)
 {
-#if ENABLE(BLOB)
-    // FIXME: This should use a more generic mechanism, like builtinResourceHandleConstructorMap we have for async requests.
-    if (request.url().protocolIs("blob"))
-        if (blobRegistry().loadResourceSynchronously(request, error, response, data))
-            return;
-#endif
-
-    ASSERT(builtinResourceHandleConstructorMap().find(request.url().protocol()) == builtinResourceHandleConstructorMap().end());
+    BuiltinResourceHandleSynchronousLoaderMap::iterator protocolMapItem = builtinResourceHandleSynchronousLoaderMap().find(request.url().protocol());
+
+    if (protocolMapItem != builtinResourceHandleSynchronousLoaderMap().end()) {
+        protocolMapItem->value(context, request, storedCredentials, error, response, data);
+        return;
+    }
 
     platformLoadResourceSynchronously(context, request, storedCredentials, error, response, data);

trunk/Source/WebCore/platform/network/ResourceHandle.h ¶

@@ -216 +216 @@
     static void registerBuiltinConstructor(const AtomicString& protocol, BuiltinConstructor);
 
+    typedef void (*BuiltinSynchronousLoader)(NetworkingContext*, const ResourceRequest&, StoredCredentials, ResourceError&, ResourceResponse&, Vector<char>& data);
+    static void registerBuiltinSynchronousLoader(const AtomicString& protocol, BuiltinSynchronousLoader);
+
 protected:
     ResourceHandle(NetworkingContext*, const ResourceRequest&, ResourceHandleClient*, bool defersLoading, bool shouldContentSniff);

trunk/Source/WebCore/platform/network/chromium/BlobRegistryProxy.h ¶

@@ -48 +48 @@
     virtual void unregisterBlobURL(const KURL&);
 
-    virtual bool loadResourceSynchronously(const ResourceRequest&, ResourceError&, ResourceResponse&, Vector<char>& data) { return false; }
-
 private:
     virtual ~BlobRegistryProxy() { }