Changeset 248538 in webkit

Timestamp:

Aug 12, 2019 12:13:17 PM (5 years ago)

Author:

youenn@apple.com

Message:

Make Blob::m_size an Optional
​https://bugs.webkit.org/show_bug.cgi?id=200617

Reviewed by Alex Christensen.

Use an Optional instead of -1 to know that m_size is initialized or not.
No change of behavior.

Refactoring to make all Blob members private.
Remove one static Blob create method.

Covered by existing tests.

• Modules/fetch/FetchBody.cpp:

(WebCore::FetchBody::fromFormData):

• fileapi/Blob.cpp:

(WebCore::Blob::Blob):
(WebCore::Blob::size const):

• fileapi/Blob.h:

(WebCore::Blob::setInternalURL):

• fileapi/File.cpp:

(WebCore::File::create):
(WebCore::File::File):
(WebCore::File::computeNameAndContentType):

• fileapi/File.h:
• html/FileListCreator.cpp:

(WebCore::FileListCreator::createFileList):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• Modules/fetch/FetchBody.cpp (modified) (1 diff)
• fileapi/Blob.cpp (modified) (5 diffs)
• fileapi/Blob.h (modified) (2 diffs)
• fileapi/File.cpp (modified) (2 diffs)
• fileapi/File.h (modified) (3 diffs)
• html/FileListCreator.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-08-12  Youenn Fablet  <youenn@apple.com>
+
+        Make Blob::m_size an Optional
+        https://bugs.webkit.org/show_bug.cgi?id=200617
+
+        Reviewed by Alex Christensen.
+
+        Use an Optional instead of -1 to know that m_size is initialized or not.
+        No change of behavior.
+
+        Refactoring to make all Blob members private.
+        Remove one static Blob create method.
+
+        Covered by existing tests.
+
+        * Modules/fetch/FetchBody.cpp:
+        (WebCore::FetchBody::fromFormData):
+        * fileapi/Blob.cpp:
+        (WebCore::Blob::Blob):
+        (WebCore::Blob::size const):
+        * fileapi/Blob.h:
+        (WebCore::Blob::setInternalURL):
+        * fileapi/File.cpp:
+        (WebCore::File::create):
+        (WebCore::File::File):
+        (WebCore::File::computeNameAndContentType):
+        * fileapi/File.h:
+        * html/FileListCreator.cpp:
+        (WebCore::FileListCreator::createFileList):
+
 2019-08-12  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/Modules/fetch/FetchBody.cpp

@@ -89 +89 @@
     if (!url.isNull()) {
         // FIXME: Properly set mime type and size of the blob.
-        Ref<const Blob> blob = Blob::deserialize(sessionID, url, { }, 0, { });
+        Ref<const Blob> blob = Blob::deserialize(sessionID, url, { }, { }, { });
         return FetchBody { WTFMove(blob) };
     }

trunk/Source/WebCore/fileapi/Blob.cpp

@@ -73 +73 @@
 }
 
-Blob::Blob(UninitializedContructor, PAL::SessionID sessionID)
-    : m_sessionID(sessionID)
+Blob::Blob(UninitializedContructor, PAL::SessionID sessionID, URL&& url, String&& type)
+    : m_sessionID(sessionID)
+    , m_internalURL(WTFMove(url))
+    , m_type(WTFMove(type))
 {
 }
@@ -90 +92 @@
     , m_internalURL(BlobURL::createInternalURL())
     , m_type(normalizedContentType(propertyBag.type))
-    , m_size(-1)
 {
     BlobBuilder builder(propertyBag.endings);
@@ -138 +139 @@
 }
 
-Blob::Blob(DeserializationContructor, PAL::SessionID sessionID, const URL& srcURL, const String& type, long long size, const String& fileBackedPath)
+Blob::Blob(DeserializationContructor, PAL::SessionID sessionID, const URL& srcURL, const String& type, Optional<unsigned long long> size, const String& fileBackedPath)
     : m_sessionID(sessionID)
     , m_type(normalizedContentType(type))
@@ -153 +154 @@
     : m_sessionID(sessionID)
     , m_type(normalizedContentType(type))
-    , m_size(-1) // size is not necessarily equal to end - start.
+    // m_size is not necessarily equal to end - start so we do not initialize it here.
 {
     m_internalURL = BlobURL::createInternalURL();
@@ -166 +167 @@
 unsigned long long Blob::size() const
 {
-    if (m_size < 0) {
+    if (!m_size) {
         // 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.
         unsigned long long actualSize = ThreadableBlobRegistry::blobSize(m_internalURL);
-        m_size = WTF::isInBounds<long long>(actualSize) ? static_cast<long long>(actualSize) : 0;
-    }
-
-    return static_cast<unsigned long long>(m_size);
+        m_size = WTF::isInBounds<long long>(actualSize) ? actualSize : 0;
+    }
+
+    return *m_size;
 }
 

trunk/Source/WebCore/fileapi/Blob.h

@@ -118 +118 @@
 
     enum UninitializedContructor { uninitializedContructor };
-    Blob(UninitializedContructor, PAL::SessionID);
+    Blob(UninitializedContructor, PAL::SessionID, URL&&, String&& type);
 
     enum DeserializationContructor { deserializationContructor };
-    Blob(DeserializationContructor, PAL::SessionID, const URL& srcURL, const String& type, long long size, const String& fileBackedPath);
+    Blob(DeserializationContructor, PAL::SessionID, const URL& srcURL, const String& type, Optional<unsigned long long> size, const String& fileBackedPath);
 
     // For slicing.
     Blob(PAL::SessionID, const URL& srcURL, long long start, long long end, const String& contentType);
 
+private:
     PAL::SessionID m_sessionID;
+
     // This is an internal URL referring to the blob data associated with this object. It serves
     // as an identifier for this blob. The internal URL is never used to source the blob's content
@@ -133 +135 @@
 
     String m_type;
-    mutable long long m_size;
+
+    mutable Optional<unsigned long long> m_size;
 };
 

trunk/Source/WebCore/fileapi/File.cpp

@@ -47 +47 @@
 }
 
-File::File(PAL::SessionID sessionID, const String& path)
-    : Blob(uninitializedContructor, sessionID)
-    , m_path(path)
+Ref<File> File::create(PAL::SessionID sessionID, const String& path, const String& nameOverride)
 {
-    m_internalURL = BlobURL::createInternalURL();
-    m_size = -1;
-    computeNameAndContentType(m_path, String(), m_name, m_type);
-    ThreadableBlobRegistry::registerFileBlobURL(m_internalURL, path, m_type);
+    String name;
+    String type;
+    computeNameAndContentType(path, nameOverride, name, type);
+
+    auto internalURL = BlobURL::createInternalURL();
+    ThreadableBlobRegistry::registerFileBlobURL(internalURL, path, type);
+
+    return adoptRef(*new File(sessionID, WTFMove(internalURL), WTFMove(type), String { path }, WTFMove(name)));
 }
 
-File::File(PAL::SessionID sessionID, const String& path, const String& nameOverride)
-    : Blob(uninitializedContructor, sessionID)
-    , m_path(path)
+File::File(PAL::SessionID sessionID, URL&& url, String&& type, String&& path, String&& name)
+    : Blob(uninitializedContructor, sessionID, WTFMove(url), WTFMove(type))
+    , m_path(WTFMove(path))
+    , m_name(WTFMove(name))
 {
-    m_internalURL = BlobURL::createInternalURL();
-    m_size = -1;
-    computeNameAndContentType(m_path, nameOverride, m_name, m_type);
-    ThreadableBlobRegistry::registerFileBlobURL(m_internalURL, path, m_type);
 }
 
 File::File(DeserializationContructor, PAL::SessionID sessionID, const String& path, const URL& url, const String& type, const String& name, const Optional<int64_t>& lastModified)
-    : Blob(deserializationContructor, sessionID, url, type, -1, path)
+    : Blob(deserializationContructor, sessionID, url, type, { }, path)
     , m_path(path)
     , m_name(name)
@@ -133 +132 @@
     }
 #endif
-    effectiveName = nameOverride.isNull() ? FileSystem::pathGetFileName(path) : nameOverride;
+    effectiveName = nameOverride.isEmpty() ? FileSystem::pathGetFileName(path) : nameOverride;
     size_t index = effectiveName.reverseFind('.');
     if (index != notFound)

trunk/Source/WebCore/fileapi/File.h

@@ -42 +42 @@
     };
 
-    static Ref<File> create(PAL::SessionID sessionID, const String& path)
-    {
-        return adoptRef(*new File(sessionID, path));
-    }
+    // Create a file with an optional name exposed to the author (via File.name and associated DOM properties) that differs from the one provided in the path.
+    WEBCORE_EXPORT static Ref<File> create(PAL::SessionID, const String& path, const String& nameOverride = { });
 
     // Create a File using the 'new File' constructor.
@@ -56 +54 @@
     {
         return adoptRef(*new File(deserializationContructor, sessionID, path, srcURL, type, name, lastModified));
-    }
-
-    // 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 Ref<File> createWithName(PAL::SessionID sessionID, const String& path, const String& nameOverride)
-    {
-        if (nameOverride.isEmpty())
-            return adoptRef(*new File(sessionID, path));
-        return adoptRef(*new File(sessionID, path, nameOverride));
     }
 
@@ -97 +87 @@
 private:
     WEBCORE_EXPORT explicit File(PAL::SessionID, const String& path);
-    File(PAL::SessionID, const String& path, const String& nameOverride);
+    File(PAL::SessionID, URL&&, String&& type, String&& path, String&& name);
     File(ScriptExecutionContext&, Vector<BlobPartVariant>&& blobPartVariants, const String& filename, const PropertyBag&);
     File(const Blob&, const String& name);

trunk/Source/WebCore/html/FileListCreator.cpp

@@ -84 +84 @@
             appendDirectoryFiles(sessionID, info.path, FileSystem::pathGetFileName(info.path), fileObjects);
         else
-            fileObjects.append(File::createWithName(sessionID, info.path, info.displayName));
+            fileObjects.append(File::create(sessionID, info.path, info.displayName));
     }
     return FileList::create(WTFMove(fileObjects));