Synchronous XMLHttpRequest should get access to AppCache resources stored as flat...
authoryouenn.fablet@crf.canon.fr <youenn.fablet@crf.canon.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 29 Apr 2015 08:18:10 +0000 (08:18 +0000)
committeryouenn.fablet@crf.canon.fr <youenn.fablet@crf.canon.fr@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 29 Apr 2015 08:18:10 +0000 (08:18 +0000)
https://bugs.webkit.org/show_bug.cgi?id=143711

Reviewed by Darin Adler.

Source/WebCore:

This patch checks whether a substitute resource data is stored in memory or in file for synchronous loads.
If data is stored in file, it reads the data through SharedBuffer::createWithContentsOfFile.
This patch refactors some routines to replace Vector<char> by SharedBuffer to transmit response data.

Test: http/tests/appcache/simple-video-sync.html

* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::parseAttribute):
* loader/DocumentThreadableLoader.cpp:
(WebCore::DocumentThreadableLoader::loadRequest):
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::loadResourceSynchronously):
* loader/FrameLoader.h:
* loader/appcache/ApplicationCacheHost.cpp:
(WebCore::ApplicationCacheHost::maybeLoadResource):
(WebCore::ApplicationCacheHost::createFileURL):
(WebCore::ApplicationCacheHost::maybeLoadSynchronously):
(WebCore::ApplicationCacheHost::maybeLoadFallbackSynchronously):
* loader/appcache/ApplicationCacheHost.h:
* xml/XSLTProcessorLibxslt.cpp:
(WebCore::docLoaderFunc):
* xml/parser/XMLDocumentParserLibxml2.cpp:
(WebCore::openFunc):

LayoutTests:

* http/tests/appcache/resources/fake-video.mp4: Added.
* http/tests/appcache/resources/simple-video-sync.manifest: Added.
* http/tests/appcache/simple-video-sync-expected.txt: Added.
* http/tests/appcache/simple-video-sync.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@183537 268f45cc-cd09-0410-ab3c-d52691b4dbfc

14 files changed:
LayoutTests/ChangeLog
LayoutTests/http/tests/appcache/resources/fake-video.mp4 [new file with mode: 0644]
LayoutTests/http/tests/appcache/resources/simple-video-sync.manifest [new file with mode: 0644]
LayoutTests/http/tests/appcache/simple-video-sync-expected.txt [new file with mode: 0644]
LayoutTests/http/tests/appcache/simple-video-sync.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/html/HTMLMediaElement.cpp
Source/WebCore/loader/DocumentThreadableLoader.cpp
Source/WebCore/loader/FrameLoader.cpp
Source/WebCore/loader/FrameLoader.h
Source/WebCore/loader/appcache/ApplicationCacheHost.cpp
Source/WebCore/loader/appcache/ApplicationCacheHost.h
Source/WebCore/xml/XSLTProcessorLibxslt.cpp
Source/WebCore/xml/parser/XMLDocumentParserLibxml2.cpp

index 941c452..4ff50e5 100644 (file)
@@ -1,3 +1,15 @@
+2015-04-29  Youenn Fablet  <youenn.fablet@crf.canon.fr>
+
+        Synchronous XMLHttpRequest should get access to AppCache resources stored as flat files
+        https://bugs.webkit.org/show_bug.cgi?id=143711
+
+        Reviewed by Darin Adler.
+
+        * http/tests/appcache/resources/fake-video.mp4: Added.
+        * http/tests/appcache/resources/simple-video-sync.manifest: Added.
+        * http/tests/appcache/simple-video-sync-expected.txt: Added.
+        * http/tests/appcache/simple-video-sync.html: Added.
+
 2015-04-29  Joseph Pecoraro  <pecoraro@apple.com>
 
         REGRESSION(181868): Windows Live SkyDrive cannot open an excel file
diff --git a/LayoutTests/http/tests/appcache/resources/fake-video.mp4 b/LayoutTests/http/tests/appcache/resources/fake-video.mp4
new file mode 100644 (file)
index 0000000..fa57e7f
--- /dev/null
@@ -0,0 +1 @@
+This is a fake video
diff --git a/LayoutTests/http/tests/appcache/resources/simple-video-sync.manifest b/LayoutTests/http/tests/appcache/resources/simple-video-sync.manifest
new file mode 100644 (file)
index 0000000..872ccd6
--- /dev/null
@@ -0,0 +1,2 @@
+CACHE MANIFEST
+fake-video.mp4
diff --git a/LayoutTests/http/tests/appcache/simple-video-sync-expected.txt b/LayoutTests/http/tests/appcache/simple-video-sync-expected.txt
new file mode 100644 (file)
index 0000000..e1570f4
--- /dev/null
@@ -0,0 +1,2 @@
+This tests that the application cache works for video retrieved by sync XMLHttpRequest
+SUCCESS
diff --git a/LayoutTests/http/tests/appcache/simple-video-sync.html b/LayoutTests/http/tests/appcache/simple-video-sync.html
new file mode 100644 (file)
index 0000000..e509413
--- /dev/null
@@ -0,0 +1,36 @@
+<html manifest="resources/simple-video-sync.manifest">
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText()
+    testRunner.waitUntilDone();
+}
+
+function finishTest(message) {
+    document.getElementById('result').innerHTML = message;
+    testRunner.notifyDone();
+}
+
+function cached()
+{
+    try {
+        var req = new XMLHttpRequest();
+        req.open("GET", "resources/fake-video.mp4", false);
+        req.send();
+        if (req.getResponseHeader("Content-Type") != "video/mp4")
+            finishTest("FAILURE: Did not get correct content type from cached resource");
+        if (req.responseText.trim() != "This is a fake video")
+            finishTest("FAILURE: Did not get correct data from cached resource");
+        finishTest("SUCCESS");
+    } catch (e) {
+        finishTest("FAILURE: Could not load video data from cache");
+    }
+}
+
+applicationCache.addEventListener('cached', cached, false);
+applicationCache.addEventListener('noupdate', cached, false);
+
+</script>
+<div>This tests that the application cache works for video retrieved by sync XMLHttpRequest</div>
+
+<div id="result">FAILURE</div>
+</html>
index 1638991..8feda23 100644 (file)
@@ -1,3 +1,34 @@
+2015-04-29  Youenn Fablet  <youenn.fablet@crf.canon.fr>
+
+        Synchronous XMLHttpRequest should get access to AppCache resources stored as flat files
+        https://bugs.webkit.org/show_bug.cgi?id=143711
+
+        Reviewed by Darin Adler.
+
+        This patch checks whether a substitute resource data is stored in memory or in file for synchronous loads.
+        If data is stored in file, it reads the data through SharedBuffer::createWithContentsOfFile.
+        This patch refactors some routines to replace Vector<char> by SharedBuffer to transmit response data.
+
+        Test: http/tests/appcache/simple-video-sync.html
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::parseAttribute):
+        * loader/DocumentThreadableLoader.cpp:
+        (WebCore::DocumentThreadableLoader::loadRequest):
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::loadResourceSynchronously):
+        * loader/FrameLoader.h:
+        * loader/appcache/ApplicationCacheHost.cpp:
+        (WebCore::ApplicationCacheHost::maybeLoadResource):
+        (WebCore::ApplicationCacheHost::createFileURL):
+        (WebCore::ApplicationCacheHost::maybeLoadSynchronously):
+        (WebCore::ApplicationCacheHost::maybeLoadFallbackSynchronously):
+        * loader/appcache/ApplicationCacheHost.h:
+        * xml/XSLTProcessorLibxslt.cpp:
+        (WebCore::docLoaderFunc):
+        * xml/parser/XMLDocumentParserLibxml2.cpp:
+        (WebCore::openFunc):
+
 2015-04-29  Gyuyoung Kim  <gyuyoung.kim@webkit.org>
 
         Purge PassRefPtr from createSVGPathSegFoo factory functions
index 45a1f93..3b40c27 100644 (file)
@@ -1079,25 +1079,6 @@ void HTMLMediaElement::loadNextSourceChild()
     loadResource(mediaURL, contentType, keySystem);
 }
 
-static URL createFileURLForApplicationCacheResource(const String& path)
-{
-    // URL should have a function to create a url from a path, but it does not. This function
-    // is not suitable because URL::setPath uses encodeWithURLEscapeSequences, which it notes
-    // does not correctly escape '#' and '?'. This function works for our purposes because
-    // app cache media files are always created with encodeForFileName(createCanonicalUUIDString()).
-
-#if USE(CF) && PLATFORM(WIN)
-    RetainPtr<CFURLRef> cfURL = adoptCF(CFURLCreateWithFileSystemPath(0, path.createCFString().get(), kCFURLWindowsPathStyle, false));
-    URL url(cfURL.get());
-#else
-    URL url;
-
-    url.setProtocol(ASCIILiteral("file"));
-    url.setPath(path);
-#endif
-    return url;
-}
-
 void HTMLMediaElement::loadResource(const URL& initialURL, ContentType& contentType, const String& keySystem)
 {
     ASSERT(isSafeToLoadURL(initialURL, Complain));
@@ -1162,7 +1143,7 @@ void HTMLMediaElement::loadResource(const URL& initialURL, ContentType& contentT
     m_currentSrc = url;
 
     if (resource) {
-        url = createFileURLForApplicationCacheResource(resource->path());
+        url = ApplicationCacheHost::createFileURL(resource->path());
         LOG(Media, "HTMLMediaElement::loadResource(%p) - will load from app cache -> %s", this, urlForLoggingMedia(url).utf8().data());
     }
 
index 531e9a3..1e86fac 100644 (file)
@@ -375,7 +375,7 @@ void DocumentThreadableLoader::loadRequest(const ResourceRequest& request, Secur
     }
     
     // FIXME: ThreadableLoaderOptions.sniffContent is not supported for synchronous requests.
-    Vector<char> data;
+    RefPtr<SharedBuffer> data;
     ResourceError error;
     ResourceResponse response;
     unsigned long identifier = std::numeric_limits<unsigned long>::max();
@@ -404,10 +404,8 @@ void DocumentThreadableLoader::loadRequest(const ResourceRequest& request, Secur
 
     didReceiveResponse(identifier, response);
 
-    const char* bytes = static_cast<const char*>(data.data());
-    int len = static_cast<int>(data.size());
-    didReceiveData(identifier, bytes, len);
-
+    if (data)
+        didReceiveData(identifier, data->data(), data->size());
     didFinishLoading(identifier, 0.0);
 }
 
index 7c8839c..ad530c3 100644 (file)
@@ -2655,7 +2655,7 @@ void FrameLoader::loadPostRequest(const ResourceRequest& inRequest, const String
     }
 }
 
-unsigned long FrameLoader::loadResourceSynchronously(const ResourceRequest& request, StoredCredentials storedCredentials, ClientCredentialPolicy clientCredentialPolicy, ResourceError& error, ResourceResponse& response, Vector<char>& data)
+unsigned long FrameLoader::loadResourceSynchronously(const ResourceRequest& request, StoredCredentials storedCredentials, ClientCredentialPolicy clientCredentialPolicy, ResourceError& error, ResourceResponse& response, RefPtr<SharedBuffer>& data)
 {
     ASSERT(m_frame.document());
     String referrer = SecurityPolicy::generateReferrerHeader(m_frame.document()->referrerPolicy(), request.url(), outgoingReferrer());
@@ -2677,13 +2677,15 @@ unsigned long FrameLoader::loadResourceSynchronously(const ResourceRequest& requ
 
     if (error.isNull()) {
         ASSERT(!newRequest.isNull());
-        
+
         if (!documentLoader()->applicationCacheHost()->maybeLoadSynchronously(newRequest, error, response, data)) {
-            platformStrategies()->loaderStrategy()->loadResourceSynchronously(networkingContext(), identifier, newRequest, storedCredentials, clientCredentialPolicy, error, response, data);
+            Vector<char> buffer;
+            platformStrategies()->loaderStrategy()->loadResourceSynchronously(networkingContext(), identifier, newRequest, storedCredentials, clientCredentialPolicy, error, response, buffer);
+            data = SharedBuffer::adoptVector(buffer);
             documentLoader()->applicationCacheHost()->maybeLoadFallbackSynchronously(newRequest, error, response, data);
         }
     }
-    notifier().sendRemainingDelegateMessages(m_documentLoader.get(), identifier, request, response, data.data(), data.size(), -1, error);
+    notifier().sendRemainingDelegateMessages(m_documentLoader.get(), identifier, request, response, data ? data->data() : nullptr, data ? data->size() : 0, -1, error);
     return identifier;
 }
 
index c0d863c..885b2b7 100644 (file)
@@ -43,6 +43,7 @@
 #include "ResourceLoadNotifier.h"
 #include "ResourceRequestBase.h"
 #include "SecurityContext.h"
+#include "SharedBuffer.h"
 #include "Timer.h"
 #include <wtf/Forward.h>
 #include <wtf/HashSet.h>
@@ -115,7 +116,7 @@ public:
 #if ENABLE(WEB_ARCHIVE) || ENABLE(MHTML)
     WEBCORE_EXPORT void loadArchive(PassRefPtr<Archive>);
 #endif
-    unsigned long loadResourceSynchronously(const ResourceRequest&, StoredCredentials, ClientCredentialPolicy, ResourceError&, ResourceResponse&, Vector<char>& data);
+    unsigned long loadResourceSynchronously(const ResourceRequest&, StoredCredentials, ClientCredentialPolicy, ResourceError&, ResourceResponse&, RefPtr<SharedBuffer>& data);
 
     void changeLocation(const FrameLoadRequest&);
     WEBCORE_EXPORT void urlSelected(const URL&, const String& target, PassRefPtr<Event>, LockHistory, LockBackForwardList, ShouldSendReferrer);
index 1f36a95..101da23 100644 (file)
@@ -31,6 +31,7 @@
 #include "ApplicationCacheResource.h"
 #include "DocumentLoader.h"
 #include "DOMApplicationCache.h"
+#include "FileSystem.h"
 #include "Frame.h"
 #include "FrameLoader.h"
 #include "FrameLoaderClient.h"
@@ -168,10 +169,10 @@ bool ApplicationCacheHost::maybeLoadResource(ResourceLoader* loader, const Resou
     ApplicationCacheResource* resource;
     if (!shouldLoadResourceFromApplicationCache(request, resource))
         return false;
-    
+
     m_documentLoader.m_pendingSubstituteResources.set(loader, resource);
     m_documentLoader.deliverSubstituteResourcesAfterDelay();
-        
+
     return true;
 }
 
@@ -202,22 +203,48 @@ bool ApplicationCacheHost::maybeLoadFallbackForError(ResourceLoader* resourceLoa
     return false;
 }
 
-bool ApplicationCacheHost::maybeLoadSynchronously(ResourceRequest& request, ResourceError& error, ResourceResponse& response, Vector<char>& data)
+URL ApplicationCacheHost::createFileURL(const String& path)
+{
+    // FIXME: Can we just use fileURLWithFileSystemPath instead?
+
+    // fileURLWithFileSystemPath function is not suitable because URL::setPath uses encodeWithURLEscapeSequences, which it notes
+    // does not correctly escape '#' and '?'. This function works for our purposes because
+    // app cache media files are always created with encodeForFileName(createCanonicalUUIDString()).
+
+#if USE(CF) && PLATFORM(WIN)
+    RetainPtr<CFURLRef> cfURL = adoptCF(CFURLCreateWithFileSystemPath(0, path.createCFString().get(), kCFURLWindowsPathStyle, false));
+    URL url(cfURL.get());
+#else
+    URL url;
+
+    url.setProtocol(ASCIILiteral("file"));
+    url.setPath(path);
+#endif
+    return url;
+}
+
+bool ApplicationCacheHost::maybeLoadSynchronously(ResourceRequest& request, ResourceError& error, ResourceResponse& response, RefPtr<SharedBuffer>& data)
 {
     ApplicationCacheResource* resource;
     if (shouldLoadResourceFromApplicationCache(request, resource)) {
         if (resource) {
-            response = resource->response();
-            data.append(resource->data()->data(), resource->data()->size());
-        } else {
-            error = m_documentLoader.frameLoader()->client().cannotShowURLError(request);
+            // FIXME: Clients proably do not need a copy of the SharedBuffer.
+            // Remove the call to copy() once we ensure SharedBuffer will not be modified.
+            if (resource->path().isEmpty())
+                data = resource->data()->copy();
+            else
+                data = SharedBuffer::createWithContentsOfFile(resource->path());
         }
+        if (!data)
+            error = m_documentLoader.frameLoader()->client().cannotShowURLError(request);
+        else
+            response = resource->response();
         return true;
     }
     return false;
 }
 
-void ApplicationCacheHost::maybeLoadFallbackSynchronously(const ResourceRequest& request, ResourceError& error, ResourceResponse& response, Vector<char>& data)
+void ApplicationCacheHost::maybeLoadFallbackSynchronously(const ResourceRequest& request, ResourceError& error, ResourceResponse& response, RefPtr<SharedBuffer>& data)
 {
     // If normal loading results in a redirect to a resource with another origin (indicative of a captive portal), or a 4xx or 5xx status code or equivalent,
     // or if there were network errors (but not if the user canceled the download), then instead get, from the cache, the resource of the fallback entry
@@ -228,8 +255,9 @@ void ApplicationCacheHost::maybeLoadFallbackSynchronously(const ResourceRequest&
         ApplicationCacheResource* resource;
         if (getApplicationCacheFallbackResource(request, resource)) {
             response = resource->response();
-            data.clear();
-            data.append(resource->data()->data(), resource->data()->size());
+            // FIXME: Clients proably do not need a copy of the SharedBuffer.
+            // Remove the call to copy() once we ensure SharedBuffer will not be modified.
+            data = resource->data()->copy();
         }
     }
 }
index 45bf05b..6df2b89 100644 (file)
@@ -41,10 +41,11 @@ namespace WebCore {
     class DOMApplicationCache;
     class DocumentLoader;
     class Frame;
-    class ResourceLoader;
     class ResourceError;
+    class ResourceLoader;
     class ResourceRequest;
     class ResourceResponse;
+    class SharedBuffer;
     class SubstituteData;
     class ApplicationCache;
     class ApplicationCacheGroup;
@@ -110,6 +111,8 @@ namespace WebCore {
         explicit ApplicationCacheHost(DocumentLoader&);
         ~ApplicationCacheHost();
 
+        static URL createFileURL(const String&);
+
         void selectCacheWithoutManifest();
         void selectCacheWithManifest(const URL& manifestURL);
 
@@ -125,8 +128,8 @@ namespace WebCore {
         WEBCORE_EXPORT bool maybeLoadFallbackForResponse(ResourceLoader*, const ResourceResponse&);
         WEBCORE_EXPORT bool maybeLoadFallbackForError(ResourceLoader*, const ResourceError&);
 
-        bool maybeLoadSynchronously(ResourceRequest&, ResourceError&, ResourceResponse&, Vector<char>& data);
-        void maybeLoadFallbackSynchronously(const ResourceRequest&, ResourceError&, ResourceResponse&, Vector<char>& data);
+        bool maybeLoadSynchronously(ResourceRequest&, ResourceError&, ResourceResponse&, RefPtr<SharedBuffer>&);
+        void maybeLoadFallbackSynchronously(const ResourceRequest&, ResourceError&, ResourceResponse&, RefPtr<SharedBuffer>&);
 
         bool canCacheInPageCache();
 
@@ -177,7 +180,6 @@ namespace WebCore {
         ApplicationCache* mainResourceApplicationCache() const { return m_mainResourceApplicationCache.get(); }
         bool maybeLoadFallbackForMainError(const ResourceRequest&, const ResourceError&);
 
-
         // The application cache that the document loader is associated with (if any).
         RefPtr<ApplicationCache> m_applicationCache;
 
index 54b781a..add49f7 100644 (file)
@@ -124,18 +124,19 @@ static xmlDocPtr docLoaderFunc(const xmlChar* uri,
         ResourceError error;
         ResourceResponse response;
 
-        Vector<char> data;
+        RefPtr<SharedBuffer> data;
 
         bool requestAllowed = globalCachedResourceLoader->frame() && globalCachedResourceLoader->document()->securityOrigin()->canRequest(url);
         if (requestAllowed) {
             globalCachedResourceLoader->frame()->loader().loadResourceSynchronously(url, AllowStoredCredentials, DoNotAskClientForCrossOriginCredentials, error, response, data);
             if (error.isNull())
                 requestAllowed = globalCachedResourceLoader->document()->securityOrigin()->canRequest(response.url());
-            else
-                data.clear();
+            else if (data)
+                data = nullptr;
         }
         if (!requestAllowed) {
-            data.clear();
+            if (data)
+                data = nullptr;
             globalCachedResourceLoader->printAccessDeniedMessage(url);
         }
 
@@ -148,7 +149,7 @@ static xmlDocPtr docLoaderFunc(const xmlChar* uri,
 
         // We don't specify an encoding here. Neither Gecko nor WinIE respects
         // the encoding specified in the HTTP headers.
-        xmlDocPtr doc = xmlReadMemory(data.data(), data.size(), (const char*)uri, 0, options);
+        xmlDocPtr doc = xmlReadMemory(data ? data->data() : nullptr, data ? data->size() : 0, (const char*)uri, 0, options);
 
         xmlSetStructuredErrorFunc(0, 0);
         xmlSetGenericErrorFunc(0, 0);
index 60a0a5b..e04d825 100644 (file)
@@ -454,7 +454,7 @@ static void* openFunc(const char* uri)
 
     ResourceError error;
     ResourceResponse response;
-    Vector<char> data;
+    RefPtr<SharedBuffer> data;
 
 
     {
@@ -470,8 +470,10 @@ static void* openFunc(const char* uri)
     // See <https://bugs.webkit.org/show_bug.cgi?id=21963>.
     if (!shouldAllowExternalLoad(response.url()))
         return &globalDescriptor;
-
-    return new OffsetBuffer(WTF::move(data));
+    Vector<char> buffer;
+    if (data)
+        buffer.append(data->data(), data->size());
+    return new OffsetBuffer(WTF::move(buffer));
 }
 
 static int readFunc(void* context, char* buffer, int len)