REGRESSION: We should allow null modificationTime when snapshot metadata is given
authorkinuko@chromium.org <kinuko@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 30 May 2012 09:08:43 +0000 (09:08 +0000)
committerkinuko@chromium.org <kinuko@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 30 May 2012 09:08:43 +0000 (09:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=86811

Reviewed by Jian Li.

r117432 has introduced a new File constructor which allows the caller
to pass in a snapshot file metadata. In the change we had considered the
given metadata is valid if "metadata.length >= 0 AND metadata.lastModifiedDate != 0",
but we should drop the latter condition (lastModifiedDate != 0) because

1. the value 0 is used to indicate the time information is unavailable in File, and
2. it is valid per spec (http://dev.w3.org/2006/webapi/FileAPI/#dfn-lastModifiedDate says the UA must return null if the information is not available).

(Note: the current js/v8 binding returns Date(0) for the time value 0,
which is still valid as epoch time but would fail to indicate the
unavailability of the information. In this patch I added FIXME in
File.idl and filed a separate issue http://webkit.org/b/87709)

No new tests as this change does not affect regular files/filesystems behavior.
(Tests in Chrome OS port should be able to verify this)

* fileapi/File.cpp:
(WebCore::File::lastModifiedDate):
(WebCore::File::size):
(WebCore::File::captureSnapshot):
* fileapi/File.h:
(File):

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

Source/WebCore/ChangeLog
Source/WebCore/fileapi/File.cpp
Source/WebCore/fileapi/File.h
Source/WebCore/fileapi/File.idl

index cb86ac3..2d30bb1 100644 (file)
@@ -1,3 +1,33 @@
+2012-05-23  Kinuko Yasuda  <kinuko@chromium.org>
+
+        REGRESSION: We should allow null modificationTime when snapshot metadata is given
+        https://bugs.webkit.org/show_bug.cgi?id=86811
+
+        Reviewed by Jian Li.
+
+        r117432 has introduced a new File constructor which allows the caller
+        to pass in a snapshot file metadata. In the change we had considered the
+        given metadata is valid if "metadata.length >= 0 AND metadata.lastModifiedDate != 0",
+        but we should drop the latter condition (lastModifiedDate != 0) because
+
+        1. the value 0 is used to indicate the time information is unavailable in File, and
+        2. it is valid per spec (http://dev.w3.org/2006/webapi/FileAPI/#dfn-lastModifiedDate says the UA must return null if the information is not available).
+
+        (Note: the current js/v8 binding returns Date(0) for the time value 0,
+        which is still valid as epoch time but would fail to indicate the
+        unavailability of the information. In this patch I added FIXME in
+        File.idl and filed a separate issue http://webkit.org/b/87709)
+
+        No new tests as this change does not affect regular files/filesystems behavior.
+        (Tests in Chrome OS port should be able to verify this)
+
+        * fileapi/File.cpp:
+        (WebCore::File::lastModifiedDate):
+        (WebCore::File::size):
+        (WebCore::File::captureSnapshot):
+        * fileapi/File.h:
+        (File):
+
 2012-05-30  MORITA Hajime  <morrita@google.com>
 
         [Shadow DOM] HTMLContentElement and HTMLShadowElement constructors should be visible.
index 0980848..54aec51 100644 (file)
@@ -130,7 +130,7 @@ File::File(const String& name, const FileMetadata& metadata)
 double File::lastModifiedDate() const
 {
 #if ENABLE(FILE_SYSTEM)
-    if (m_snapshotSize >= 0 && m_snapshotModificationTime)
+    if (hasValidSnapshotMetadata())
         return m_snapshotModificationTime * 1000.0;
 #endif
 
@@ -145,7 +145,7 @@ double File::lastModifiedDate() const
 unsigned long long File::size() const
 {
 #if ENABLE(FILE_SYSTEM)
-    if (m_snapshotSize >= 0 && m_snapshotModificationTime)
+    if (hasValidSnapshotMetadata())
         return m_snapshotSize;
 #endif
 
@@ -160,7 +160,7 @@ unsigned long long File::size() const
 void File::captureSnapshot(long long& snapshotSize, double& snapshotModificationTime) const
 {
 #if ENABLE(FILE_SYSTEM)
-    if (m_snapshotSize >= 0 && m_snapshotModificationTime) {
+    if (hasValidSnapshotMetadata()) {
         snapshotSize = m_snapshotSize;
         snapshotModificationTime = m_snapshotModificationTime;
         return;
index e852c6a..8bc480d 100644 (file)
@@ -54,7 +54,7 @@ public:
 #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.
+    // If filesystem files live in the remote filesystem, the port might pass the valid metadata (whose length field is non-negative) 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)
@@ -76,7 +76,10 @@ public:
 
     const String& path() const { return m_path; }
     const String& name() const { return m_name; }
+
+    // This may return zero if getFileModificationTime() platform call has failed or zero snapshot lastModifiedTime is given at construction time.
     double lastModifiedDate() const;
+
 #if ENABLE(DIRECTORY_UPLOAD)
     // Returns the relative path of this file in the context of a directory selection.
     const String& webkitRelativePath() const { return m_relativePath; }
@@ -94,14 +97,17 @@ private:
 
 # if ENABLE(FILE_SYSTEM)
     File(const String& name, const FileMetadata&);
+
+    // Returns true if this has a valid snapshot metadata (i.e. m_snapshotSize >= 0).
+    bool hasValidSnapshotMetadata() const { return m_snapshotSize >= 0; }
 #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).
+    // If m_snapshotSize is negative (initialized to -1 by default), the snapshot metadata is invalid and we retrieve the latest metadata synchronously in size(), lastModifiedTime() and webkitSlice().
+    // Otherwise, the snapshot metadata are used directly in those methods.
     const long long m_snapshotSize;
     const double m_snapshotModificationTime;
 #endif
index 7c0de43..e88b9bf 100644 (file)
@@ -32,6 +32,7 @@ module html {
     ] File : Blob {
         readonly attribute DOMString name;
 #if !defined(LANGUAGE_GOBJECT) || !LANGUAGE_GOBJECT
+        // FIXME: this should return null Date if the value is 0.
         readonly attribute Date lastModifiedDate;
 #endif
 #if defined(ENABLE_DIRECTORY_UPLOAD) && ENABLE_DIRECTORY_UPLOAD