Title: [117432] trunk
Revision
117432
Author
kin...@chromium.org
Date
2012-05-17 05:07:55 -0700 (Thu, 17 May 2012)

Log Message

Allow FileSystem API implementation to pass snapshot metadata at File creation time
https://bugs.webkit.org/show_bug.cgi?id=78879

Reviewed by Jian Li.

Source/WebCore:

We query File metadata (e.g. size and modifiedTime) when File.size,
lastModifiedTime or webkitSlice() is accessed / called, but in some
platform-specific filesystems it may not be feasible since synchronous
metadata query could take very long time.

This patch adds new File constructor which takes metadata argument
to allow each FileSystem API implementation to pass snapshot metadata
so that File object could cache the given metadata not to make
synchronous query.

We only call this constructor if the filesystem type is neither
Temporary nor Persistent, therefore this patch should not affect
existing code behavior.

Test: fast/filesystem/file-read-after-write.html

* Modules/filesystem/DOMFileSystem.cpp:
(WebCore::DOMFileSystem::createFile):
* Modules/filesystem/DOMFileSystemSync.cpp:
* fileapi/Blob.cpp:
(WebCore::Blob::webkitSlice): Updated implementation.
* fileapi/Blob.h:
* fileapi/File.cpp:
(WebCore::File::File): Added new constructor.
(WebCore::File::lastModifiedDate): Updated implementation.
(WebCore::File::size): Updated implementation.
(WebCore::File::captureSnapshot): Updated implementation.
* fileapi/File.h:
(WebCore::File::createForFileSystemFile): Added.
* platform/AsyncFileSystem.h:
(AsyncFileSystem): Updated comment.

LayoutTests:

Added tests for making sure metadata is not cached in the regular
temporary filesystem.

* fast/filesystem/file-metadata-after-write-expected.txt: Added.
* fast/filesystem/file-metadata-after-write.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (117431 => 117432)


--- trunk/LayoutTests/ChangeLog	2012-05-17 11:55:42 UTC (rev 117431)
+++ trunk/LayoutTests/ChangeLog	2012-05-17 12:07:55 UTC (rev 117432)
@@ -1,3 +1,16 @@
+2012-05-11  Kinuko Yasuda  <kin...@chromium.org>
+
+        Allow FileSystem API implementation to pass snapshot metadata at File creation time
+        https://bugs.webkit.org/show_bug.cgi?id=78879
+
+        Reviewed by Jian Li.
+
+        Added tests for making sure metadata is not cached in the regular
+        temporary filesystem.
+
+        * fast/filesystem/file-metadata-after-write-expected.txt: Added.
+        * fast/filesystem/file-metadata-after-write.html: Added.
+
 2012-05-17  Kinuko Yasuda  <kin...@chromium.org>
 
         Unreviewed, updating chrome test expectations.

Added: trunk/LayoutTests/fast/filesystem/file-metadata-after-write-expected.txt (0 => 117432)


--- trunk/LayoutTests/fast/filesystem/file-metadata-after-write-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/filesystem/file-metadata-after-write-expected.txt	2012-05-17 12:07:55 UTC (rev 117432)
@@ -0,0 +1,10 @@
+This verifies File.size (for a file from FileSystem API) always returns the fresh size even after the file is modified.
+Writing 1234567890 to the file...
+Created a writer.
+Write succeeded.
+PASS testFile.size is testText1.length
+Writing abcdefghijklmnopqrstuvwxyz to the file...
+Created a writer.
+Write succeeded.
+PASS testFile.size is testText2.length
+
Property changes on: trunk/LayoutTests/fast/filesystem/file-metadata-after-write-expected.txt
___________________________________________________________________

Added: svn:eol-style

Added: trunk/LayoutTests/fast/filesystem/file-metadata-after-write.html (0 => 117432)


--- trunk/LayoutTests/fast/filesystem/file-metadata-after-write.html	                        (rev 0)
+++ trunk/LayoutTests/fast/filesystem/file-metadata-after-write.html	2012-05-17 12:07:55 UTC (rev 117432)
@@ -0,0 +1,90 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+</head>
+<body>
+<div>This verifies File.size (for a file from FileSystem API) always returns the fresh size even after the file is modified.</div>
+<div id='console'></div>
+<script>
+
+if (window.layoutTestController) {
+  layoutTestController.dumpAsText();
+  layoutTestController.waitUntilDone();
+}
+
+var fileEntryForCleanup;
+var testFile;
+var testText1 = '1234567890';
+var testText2 = 'abcdefghijklmnopqrstuvwxyz';
+
+webkitRequestFileSystem(
+    window.TEMPORARY, 100, function(fs) {
+        fs.root.getFile('test', {create:true}, function(entry) {
+            fileEntryForCleanup = entry;
+            writeTextToFile(entry, testText1, function() {
+                log('Write succeeded.');
+                entry.file(onWrittenFile.bind(this, entry), onError);
+            });
+        }, onError);
+    }, onError);
+
+function onWrittenFile(entry, file) {
+    testFile = file;
+    shouldBe('testFile.size', 'testText1.length');
+    // Write into this file again.
+    writeTextToFile(entry, testText2, function() {
+        log('Write succeeded.');
+        entry.file(function() {
+            // The file size should be updated.
+            shouldBe('testFile.size', 'testText2.length');
+            cleanup();
+        }, onError);
+    });
+}
+
+function writeTextToFile(entry, contents, successCallback)
+{
+    log('Writing ' + contents + ' to the file...');
+    entry.createWriter(function(writer) {
+        log('Created a writer.');
+        var builder = new WebKitBlobBuilder();
+        builder.append(contents);
+        writer.write(builder.getBlob('plain/text'));
+        writer._onwriteend_ = successCallback;
+        writer._onerror_ = onError;
+    });
+}
+
+function log(text)
+{
+    var console = document.getElementById('console');
+    console.appendChild(document.createTextNode(text));
+    console.appendChild(document.createElement('br'));
+}
+
+function onError(e)
+{
+    log('ERROR: ' + e.code);
+    console.log(e);
+    cleanup();
+}
+
+function cleanup()
+{
+    if (fileEntryForCleanup) {
+        fileEntryForCleanup.remove(endTest, endTest);
+        return;
+    }
+    endTest();
+}
+
+function endTest()
+{
+    if (layoutTestController)
+        layoutTestController.notifyDone();
+}
+
+</script>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/filesystem/file-metadata-after-write.html
___________________________________________________________________

Added: svn:eol-style

Modified: trunk/Source/WebCore/ChangeLog (117431 => 117432)


--- trunk/Source/WebCore/ChangeLog	2012-05-17 11:55:42 UTC (rev 117431)
+++ trunk/Source/WebCore/ChangeLog	2012-05-17 12:07:55 UTC (rev 117432)
@@ -1,3 +1,42 @@
+2012-05-11  Kinuko Yasuda  <kin...@chromium.org>
+
+        Allow FileSystem API implementation to pass snapshot metadata at File creation time
+        https://bugs.webkit.org/show_bug.cgi?id=78879
+
+        Reviewed by Jian Li.
+
+        We query File metadata (e.g. size and modifiedTime) when File.size,
+        lastModifiedTime or webkitSlice() is accessed / called, but in some
+        platform-specific filesystems it may not be feasible since synchronous
+        metadata query could take very long time.
+
+        This patch adds new File constructor which takes metadata argument
+        to allow each FileSystem API implementation to pass snapshot metadata
+        so that File object could cache the given metadata not to make
+        synchronous query.
+
+        We only call this constructor if the filesystem type is neither
+        Temporary nor Persistent, therefore this patch should not affect
+        existing code behavior.
+
+        Test: fast/filesystem/file-read-after-write.html
+
+        * Modules/filesystem/DOMFileSystem.cpp:
+        (WebCore::DOMFileSystem::createFile):
+        * Modules/filesystem/DOMFileSystemSync.cpp:
+        * fileapi/Blob.cpp:
+        (WebCore::Blob::webkitSlice): Updated implementation.
+        * fileapi/Blob.h:
+        * fileapi/File.cpp:
+        (WebCore::File::File): Added new constructor.
+        (WebCore::File::lastModifiedDate): Updated implementation.
+        (WebCore::File::size): Updated implementation.
+        (WebCore::File::captureSnapshot): Updated implementation.
+        * fileapi/File.h:
+        (WebCore::File::createForFileSystemFile): Added.
+        * platform/AsyncFileSystem.h:
+        (AsyncFileSystem): Updated comment.
+
 2012-05-17  Gyuyoung Kim  <gyuyoung....@samsung.com>
 
         Convert setDomainRelaxationForbiddenForURLScheme to use InternalSettings interface

Modified: trunk/Source/WebCore/Modules/filesystem/DOMFileSystem.cpp (117431 => 117432)


--- trunk/Source/WebCore/Modules/filesystem/DOMFileSystem.cpp	2012-05-17 11:55:42 UTC (rev 117431)
+++ trunk/Source/WebCore/Modules/filesystem/DOMFileSystem.cpp	2012-05-17 12:07:55 UTC (rev 117432)
@@ -123,11 +123,11 @@
 
 namespace {
 
-class GetPathCallback : public FileSystemCallbacksBase {
+class GetMetadataCallback : public FileSystemCallbacksBase {
 public:
-    static PassOwnPtr<GetPathCallback> create(PassRefPtr<DOMFileSystem> filesystem, const String& name, PassRefPtr<FileCallback> successCallback, PassRefPtr<ErrorCallback> errorCallback)
+    static PassOwnPtr<GetMetadataCallback> create(PassRefPtr<DOMFileSystem> filesystem, const String& name, PassRefPtr<FileCallback> successCallback, PassRefPtr<ErrorCallback> errorCallback)
     {
-        return adoptPtr(new GetPathCallback(filesystem, name, successCallback, errorCallback));
+        return adoptPtr(new GetMetadataCallback(filesystem, name, successCallback, errorCallback));
     }
 
     virtual void didReadMetadata(const FileMetadata& metadata)
@@ -136,12 +136,18 @@
         if (!m_successCallback)
             return;
 
-        m_successCallback->handleEvent(File::createWithName(metadata.platformPath, m_name).get());
+        // For regular filesystem types (temporary or persistent), we should not cache file metadata as it could change File semantics.
+        // For other filesystem types (which could be platform-specific ones), there's a chance that the files are on remote filesystem. If the port has returned metadata just pass it to File constructor (so we may cache the metadata).
+        if (m_filesystem->type() == FileSystemTypeTemporary || m_filesystem->type() == FileSystemTypePersistent)
+            m_successCallback->handleEvent(File::createWithName(metadata.platformPath, m_name).get());
+        else
+            m_successCallback->handleEvent(File::createForFileSystemFile(m_name, metadata).get());
+
         m_successCallback.release();
     }
 
 private:
-    GetPathCallback(PassRefPtr<DOMFileSystem> filesystem, const String& name,  PassRefPtr<FileCallback> successCallback, PassRefPtr<ErrorCallback> errorCallback)
+    GetMetadataCallback(PassRefPtr<DOMFileSystem> filesystem, const String& name,  PassRefPtr<FileCallback> successCallback, PassRefPtr<ErrorCallback> errorCallback)
         : FileSystemCallbacksBase(errorCallback)
         , m_filesystem(filesystem)
         , m_name(name)
@@ -158,7 +164,7 @@
 
 void DOMFileSystem::createFile(const FileEntry* fileEntry, PassRefPtr<FileCallback> successCallback, PassRefPtr<ErrorCallback> errorCallback)
 {
-    m_asyncFileSystem->createSnapshotFileAndReadMetadata(createFileSystemURL(fileEntry), GetPathCallback::create(this, fileEntry->name(), successCallback, errorCallback));
+    m_asyncFileSystem->createSnapshotFileAndReadMetadata(createFileSystemURL(fileEntry), GetMetadataCallback::create(this, fileEntry->name(), successCallback, errorCallback));
 }
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/Modules/filesystem/DOMFileSystemSync.cpp (117431 => 117432)


--- trunk/Source/WebCore/Modules/filesystem/DOMFileSystemSync.cpp	2012-05-17 11:55:42 UTC (rev 117431)
+++ trunk/Source/WebCore/Modules/filesystem/DOMFileSystemSync.cpp	2012-05-17 12:07:55 UTC (rev 117432)
@@ -98,9 +98,9 @@
         friend class WTF::RefCounted<CreateFileResult>;
     };
 
-    static PassOwnPtr<CreateFileHelper> create(PassRefPtr<CreateFileResult> result, const String& name)
+    static PassOwnPtr<CreateFileHelper> create(PassRefPtr<CreateFileResult> result, const String& name, FileSystemType type)
     {
-        return adoptPtr(new CreateFileHelper(result, name));
+        return adoptPtr(new CreateFileHelper(result, name, type));
     }
 
     virtual void didFail(int code)
@@ -115,17 +115,24 @@
 
     void didReadMetadata(const FileMetadata& metadata)
     {
-        m_result->m_file = File::createWithName(metadata.platformPath, m_name);
+        // For regular filesystem types (temporary or persistent), we should not cache file metadata as it could change File semantics.
+        // For other filesystem types (which could be platform-specific ones), there's a chance that the files are on remote filesystem. If the port has returned metadata just pass it to File constructor (so we may cache the metadata).
+        if (m_type == FileSystemTypeTemporary || m_type == FileSystemTypePersistent)
+            m_result->m_file = File::createWithName(metadata.platformPath, m_name);
+        else
+            m_result->m_file = File::createForFileSystemFile(m_name, metadata);
     }
 private:
-    CreateFileHelper(PassRefPtr<CreateFileResult> result, const String& name)
+    CreateFileHelper(PassRefPtr<CreateFileResult> result, const String& name, FileSystemType type)
         : m_result(result)
         , m_name(name)
+        , m_type(type)
     {
     }
 
     RefPtr<CreateFileResult> m_result;
     String m_name;
+    FileSystemType m_type;
 };
 
 } // namespace
@@ -134,7 +141,7 @@
 {
     ec = 0;
     RefPtr<CreateFileHelper::CreateFileResult> result(CreateFileHelper::CreateFileResult::create());
-    m_asyncFileSystem->createSnapshotFileAndReadMetadata(createFileSystemURL(fileEntry), CreateFileHelper::create(result, fileEntry->name()));
+    m_asyncFileSystem->createSnapshotFileAndReadMetadata(createFileSystemURL(fileEntry), CreateFileHelper::create(result, fileEntry->name(), type()));
     if (!m_asyncFileSystem->waitForOperationToComplete()) {
         ec = FileException::ABORT_ERR;
         return 0;

Modified: trunk/Source/WebCore/fileapi/File.cpp (117431 => 117432)


--- trunk/Source/WebCore/fileapi/File.cpp	2012-05-17 11:55:42 UTC (rev 117431)
+++ trunk/Source/WebCore/fileapi/File.cpp	2012-05-17 12:07:55 UTC (rev 117432)
@@ -26,6 +26,7 @@
 #include "config.h"
 #include "File.h"
 
+#include "FileMetadata.h"
 #include "FileSystem.h"
 #include "MIMETypeRegistry.h"
 #include <wtf/CurrentTime.h>
@@ -33,6 +34,15 @@
 
 namespace WebCore {
 
+static String getContentTypeFromFileName(const String& name)
+{
+    String type;
+    int index = name.reverseFind('.');
+    if (index != -1)
+        type = MIMETypeRegistry::getWellKnownMIMETypeForExtension(name.substring(index + 1));
+    return type;
+}
+
 static PassOwnPtr<BlobData> createBlobDataForFileWithType(const String& path, const String& contentType)
 {
     OwnPtr<BlobData> blobData = BlobData::create();
@@ -43,22 +53,24 @@
 
 static PassOwnPtr<BlobData> createBlobDataForFile(const String& path)
 {
-    String type;
-    int index = path.reverseFind('.');
-    if (index != -1)
-        type = MIMETypeRegistry::getMIMETypeForExtension(path.substring(index + 1));
-    return createBlobDataForFileWithType(path, type);
+    return createBlobDataForFileWithType(path, getContentTypeFromFileName(path));
 }
 
 static PassOwnPtr<BlobData> createBlobDataForFileWithName(const String& path, const String& fileSystemName)
 {
-    String type;
-    int index = fileSystemName.reverseFind('.');
-    if (index != -1)
-        type = MIMETypeRegistry::getWellKnownMIMETypeForExtension(fileSystemName.substring(index + 1));
-    return createBlobDataForFileWithType(path, type);
+    return createBlobDataForFileWithType(path, getContentTypeFromFileName(fileSystemName));
 }
 
+#if ENABLE(FILE_SYSTEM)
+static PassOwnPtr<BlobData> createBlobDataForFileWithMetadata(const String& fileSystemName, const FileMetadata& metadata)
+{
+    OwnPtr<BlobData> blobData = BlobData::create();
+    blobData->setContentType(getContentTypeFromFileName(fileSystemName));
+    blobData->appendFile(metadata.platformPath, 0, metadata.length, metadata.modificationTime);
+    return blobData.release();
+}
+#endif
+
 #if ENABLE(DIRECTORY_UPLOAD)
 PassRefPtr<File> File::createWithRelativePath(const String& path, const String& relativePath)
 {
@@ -72,12 +84,20 @@
     : Blob(createBlobDataForFile(path), -1)
     , m_path(path)
     , m_name(pathGetFileName(path))
+#if ENABLE(FILE_SYSTEM)
+    , m_snapshotSize(-1)
+    , m_snapshotModificationTime(0)
+#endif
 {
 }
 
 File::File(const String& path, const KURL& url, const String& type)
     : Blob(url, type, -1)
     , m_path(path)
+#if ENABLE(FILE_SYSTEM)
+    , m_snapshotSize(-1)
+    , m_snapshotModificationTime(0)
+#endif
 {
     m_name = pathGetFileName(path);
     // FIXME: File object serialization/deserialization does not include
@@ -89,11 +109,31 @@
     : Blob(createBlobDataForFileWithName(path, name), -1)
     , m_path(path)
     , m_name(name)
+#if ENABLE(FILE_SYSTEM)
+    , m_snapshotSize(-1)
+    , m_snapshotModificationTime(0)
+#endif
 {
 }
 
+#if ENABLE(FILE_SYSTEM)
+File::File(const String& name, const FileMetadata& metadata)
+    : Blob(createBlobDataForFileWithMetadata(name, metadata), metadata.length)
+    , m_path(metadata.platformPath)
+    , m_name(name)
+    , m_snapshotSize(metadata.length)
+    , m_snapshotModificationTime(metadata.modificationTime)
+{
+}
+#endif
+
 double File::lastModifiedDate() const
 {
+#if ENABLE(FILE_SYSTEM)
+    if (m_snapshotSize >= 0 && m_snapshotModificationTime)
+        return m_snapshotModificationTime * 1000.0;
+#endif
+
     time_t modificationTime;
     if (!getFileModificationTime(m_path, modificationTime))
         return 0;
@@ -104,6 +144,11 @@
 
 unsigned long long File::size() const
 {
+#if ENABLE(FILE_SYSTEM)
+    if (m_snapshotSize >= 0 && m_snapshotModificationTime)
+        return m_snapshotSize;
+#endif
+
     // FIXME: _javascript_ cannot represent sizes as large as unsigned long long, we need to
     // come up with an exception to throw if file size is not representable.
     long long size;
@@ -114,6 +159,14 @@
 
 void File::captureSnapshot(long long& snapshotSize, double& snapshotModificationTime) const
 {
+#if ENABLE(FILE_SYSTEM)
+    if (m_snapshotSize >= 0 && m_snapshotModificationTime) {
+        snapshotSize = m_snapshotSize;
+        snapshotModificationTime = m_snapshotModificationTime;
+        return;
+    }
+#endif
+
     // Obtains a snapshot of the file by capturing its current size and modification time. This is used when we slice a file for the first time.
     // If we fail to retrieve the size or modification time, probably due to that the file has been deleted, 0 size is returned.
     // FIXME: Combine getFileSize and getFileModificationTime into one file system call.

Modified: trunk/Source/WebCore/fileapi/File.h (117431 => 117432)


--- trunk/Source/WebCore/fileapi/File.h	2012-05-17 11:55:42 UTC (rev 117431)
+++ trunk/Source/WebCore/fileapi/File.h	2012-05-17 12:07:55 UTC (rev 117432)
@@ -33,6 +33,7 @@
 
 namespace WebCore {
 
+class FileMetadata;
 class KURL;
 
 class File : public Blob {
@@ -52,6 +53,16 @@
     static PassRefPtr<File> createWithRelativePath(const String& path, const String& relativePath);
 #endif
 
+#if ENABLE(FILE_SYSTEM)
+    // If filesystem files live in the remote filesystem, the port might pass the valid metadata (non-zero modificationTime and non-negative file size) and cache in the File object.
+    //
+    // Otherwise calling size(), lastModifiedTime() and webkitSlice() will synchronously query the file metadata.
+    static PassRefPtr<File> createForFileSystemFile(const String& name, const FileMetadata& metadata)
+    {
+        return adoptRef(new File(name, metadata));
+    }
+#endif
+
     // Create a file with a name exposed to the author (via File.name and associated DOM properties) that differs from the one provided in the path.
     static PassRefPtr<File> createWithName(const String& path, const String& name)
     {
@@ -81,8 +92,20 @@
     File(const String& path, const KURL& srcURL, const String& type);
     File(const String& path, const String& name);
 
+# if ENABLE(FILE_SYSTEM)
+    File(const String& name, const FileMetadata&);
+#endif
+
     String m_path;
     String m_name;
+
+#if ENABLE(FILE_SYSTEM)
+    // If non-zero modificationTime and non-negative file size are given at construction time we use them in size(), lastModifiedTime() and webkitSlice().
+    // By default we initialize m_snapshotSize to -1 and m_snapshotModificationTime to 0 (so that we don't use them unless they are given).
+    const long long m_snapshotSize;
+    const double m_snapshotModificationTime;
+#endif
+
 #if ENABLE(DIRECTORY_UPLOAD)
     String m_relativePath;
 #endif

Modified: trunk/Source/WebCore/platform/AsyncFileSystem.h (117431 => 117432)


--- trunk/Source/WebCore/platform/AsyncFileSystem.h	2012-05-17 11:55:42 UTC (rev 117431)
+++ trunk/Source/WebCore/platform/AsyncFileSystem.h	2012-05-17 12:07:55 UTC (rev 117432)
@@ -127,6 +127,8 @@
     // while in remote filesystem case the backend may download the file into a temporary snapshot file and return the metadata of the temporary file.
     // AsyncFileSystemCallbacks::didReadMetadata() is called when the metadata for the snapshot file is successfully returned.
     // AsyncFileSystemCallbacks::didFail() is called otherwise.
+    //
+    // Note: the returned metadata info is cached in the File object for non-regular filesystem types (neither Temporary nor Persistent). The port could return valid metadata if it wants File object to cache metadata (e.g. if the file body is on a remote server), but otherwise should NOT return valid metadata.
     virtual void createSnapshotFileAndReadMetadata(const KURL& path, PassOwnPtr<AsyncFileSystemCallbacks>) = 0;
 
 protected:
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to