Attachment 350169 Details for Bug 189777 – Patch

[patch] Patch

bug-189777-20180919221426.patch (text/plain), 41.08 KB, created by Alex Christensen on 2018-09-19 22:14:26 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Alex Christensen

Created: 2018-09-19 22:14:26 PDT

Size: 41.08 KB

patch

obsolete

>Index: Source/WebCore/ChangeLog
>===================================================================
>--- Source/WebCore/ChangeLog	(revision 236150)
>+++ Source/WebCore/ChangeLog	(working copy)
>@@ -1,3 +1,48 @@
>+2018-09-19  Alex Christensen  <achristensen@webkit.org>
>+
>+        Use a Variant for FormDataElement
>+        https://bugs.webkit.org/show_bug.cgi?id=189777
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * platform/network/FormData.cpp:
>+        (WebCore::FormData::FormData):
>+        (WebCore::FormDataElement::lengthInBytes const):
>+        (WebCore::FormDataElement::isolatedCopy const):
>+        (WebCore::FormData::appendData):
>+        (WebCore::FormData::flatten const):
>+        (WebCore::FormData::resolveBlobReferences):
>+        (WebCore::FormData::generateFiles):
>+        (WebCore::FormData::hasGeneratedFiles const):
>+        (WebCore::FormData::hasOwnedGeneratedFiles const):
>+        (WebCore::FormData::removeGeneratedFilesIfNeeded):
>+        (WebCore::FormData::asSharedBuffer const):
>+        (WebCore::FormData::asBlobURL const):
>+        (WebCore::FormData::expandDataStore): Deleted.
>+        * platform/network/FormData.h:
>+        (WebCore::FormDataElement::FormDataElement):
>+        (WebCore::FormDataElement::encode const):
>+        (WebCore::FormDataElement::decode):
>+        (WebCore::FormDataElement::EncodedFileData::isolatedCopy const):
>+        (WebCore::FormDataElement::EncodedFileData::operator== const):
>+        (WebCore::FormDataElement::EncodedFileData::encode const):
>+        (WebCore::FormDataElement::EncodedFileData::decode):
>+        (WebCore::FormDataElement::EncodedBlobData::operator== const):
>+        (WebCore::FormDataElement::EncodedBlobData::encode const):
>+        (WebCore::FormDataElement::EncodedBlobData::decode):
>+        (WebCore::FormDataElement::operator== const):
>+        (WebCore::FormDataElement::operator!= const):
>+        * platform/network/cf/FormDataStreamCFNet.cpp:
>+        (WebCore::advanceCurrentStream):
>+        (WebCore::createHTTPBodyCFReadStream):
>+        (WebCore::setHTTPBody):
>+        * platform/network/curl/CurlFormDataStream.cpp:
>+        (WebCore::CurlFormDataStream::computeContentLength):
>+        (WebCore::CurlFormDataStream::read):
>+        (WebCore::CurlFormDataStream::readFromFile):
>+        (WebCore::CurlFormDataStream::readFromData):
>+        * platform/network/curl/CurlFormDataStream.h:
>+
> 2018-09-18  Simon Fraser  <simon.fraser@apple.com>
> 
>         Remove the unused RenderLayerCompositor::enclosingCompositorFlushingLayers()
>Index: Source/WebCore/platform/network/FormData.cpp
>===================================================================
>--- Source/WebCore/platform/network/FormData.cpp	(revision 236150)
>+++ Source/WebCore/platform/network/FormData.cpp	(working copy)
>@@ -53,9 +53,9 @@ inline FormData::FormData(const FormData
>     // We shouldn't be copying FormData that hasn't already removed its generated files
>     // but just in case, make sure the new FormData is ready to generate its own files.
>     for (auto& element : m_elements) {
>-        if (element.m_type == FormDataElement::Type::EncodedFile) {
>-            element.m_generatedFilename = String();
>-            element.m_ownsGeneratedFile = false;
>+        if (auto* fileData = WTF::get_if<FormDataElement::EncodedFileData>(element.data)) {
>+            fileData->generatedFilename = { };
>+            fileData->ownsGeneratedFile = false;
>         }
>     }
> }
>@@ -132,41 +132,48 @@ Ref<FormData> FormData::isolatedCopy() c
> 
> uint64_t FormDataElement::lengthInBytes() const
> {
>-    switch (m_type) {
>-    case Type::Data:
>-        return m_data.size();
>-    case Type::EncodedFile: {
>-        if (m_fileLength != BlobDataItem::toEndOfFile)
>-            return m_fileLength;
>-        long long fileSize;
>-        if (FileSystem::getFileSize(m_shouldGenerateFile ? m_generatedFilename : m_filename, fileSize))
>-            return fileSize;
>-        return 0;
>-    }
>-    case Type::EncodedBlob:
>-        return ThreadableBlobRegistry::blobSize(m_url);
>-    }
>-    ASSERT_NOT_REACHED();
>-    return 0;
>+    return WTF::visit(WTF::makeVisitor(
>+        [] (const Vector<char>& bytes) {
>+            return bytes.size();
>+        }, [] (const FormDataElement::EncodedFileData& fileData) {
>+            if (fileData.fileLength != BlobDataItem::toEndOfFile)
>+                return fileData.fileLength;
>+            long long fileSize;
>+            if (FileSystem::getFileSize(fileData.shouldGenerateFile ? fileData.generatedFilename : fileData.filename, fileSize))
>+                return fileSize;
>+            return 0ll;
>+        }, [] (const FormDataElement::EncodedBlobData& blobData) {
>+            return ThreadableBlobRegistry::blobSize(blobData.url);
>+        }
>+    ), data);
> }
> 
> FormDataElement FormDataElement::isolatedCopy() const
> {
>-    switch (m_type) {
>-    case Type::Data:
>-        return FormDataElement(m_data);
>-    case Type::EncodedFile:
>-        return FormDataElement(m_filename.isolatedCopy(), m_fileStart, m_fileLength, m_expectedFileModificationTime, m_shouldGenerateFile);
>-    case Type::EncodedBlob:
>-        return FormDataElement(m_url.isolatedCopy());
>-    }
>-
>-    RELEASE_ASSERT_NOT_REACHED();
>+    return WTF::visit(WTF::makeVisitor(
>+        [] (const Vector<char>& bytes) {
>+            Vector<char> copy;
>+            copy.append(&bytes[0], bytes.size());
>+            return FormDataElement(WTFMove(copy));
>+        }, [] (const FormDataElement::EncodedFileData& fileData) {
>+            return FormDataElement(fileData.isolatedCopy());
>+        }, [] (const FormDataElement::EncodedBlobData& blobData) {
>+            return FormDataElement(blobData.url.isolatedCopy());
>+        }
>+    ), data);
> }
> 
> void FormData::appendData(const void* data, size_t size)
> {
>-    memcpy(expandDataStore(size), data, size);
>+    if (!m_elements.isEmpty()) {
>+        if (auto* data = WTF::get_if<Vector<char>>(m_elements.last().data)) {
>+            data->append(reinterpret_cast<const char*>(data), size);
>+            return;
>+        }
>+    }
>+    Vector<char> vector;
>+    vector.append(reinterpret_cast<const char*>(data), size);
>+    m_elements.append(WTFMove(vector));
> }
> 
> void FormData::appendFile(const String& filename, bool shouldGenerateFile)
>@@ -279,28 +286,13 @@ void FormData::appendNonMultiPartKeyValu
>     appendData(encodedData.data(), encodedData.size());
> }
> 
>-char* FormData::expandDataStore(size_t size)
>-{
>-    m_lengthInBytes = std::nullopt;
>-    if (m_elements.isEmpty() || m_elements.last().m_type != FormDataElement::Type::Data)
>-        m_elements.append({ });
>-
>-    auto& lastElement = m_elements.last();
>-    size_t oldSize = lastElement.m_data.size();
>-
>-    auto newSize = Checked<size_t>(oldSize) + size;
>-
>-    lastElement.m_data.grow(newSize.unsafeGet());
>-    return lastElement.m_data.data() + oldSize;
>-}
>-
> Vector<char> FormData::flatten() const
> {
>     // Concatenate all the byte arrays, but omit any files.
>     Vector<char> data;
>     for (auto& element : m_elements) {
>-        if (element.m_type == FormDataElement::Type::Data)
>-            data.append(element.m_data.data(), static_cast<size_t>(element.m_data.size()));
>+        if (auto* vector = WTF::get_if<Vector<char>>(element.data))
>+            data.append(vector->data(), vector->size());
>     }
>     return data;
> }
>@@ -340,7 +332,7 @@ Ref<FormData> FormData::resolveBlobRefer
>     // First check if any blobs needs to be resolved, or we can take the fast path.
>     bool hasBlob = false;
>     for (auto& element : m_elements) {
>-        if (element.m_type == FormDataElement::Type::EncodedBlob) {
>+        if (WTF::holds_alternative<FormDataElement::EncodedBlobData>(element.data)) {
>             hasBlob = true;
>             break;
>         }
>@@ -355,14 +347,15 @@ Ref<FormData> FormData::resolveBlobRefer
>     newFormData->setIdentifier(identifier());
> 
>     for (auto& element : m_elements) {
>-        if (element.m_type == FormDataElement::Type::Data)
>-            newFormData->appendData(element.m_data.data(), element.m_data.size());
>-        else if (element.m_type == FormDataElement::Type::EncodedFile)
>-            newFormData->appendFileRange(element.m_filename, element.m_fileStart, element.m_fileLength, element.m_expectedFileModificationTime, element.m_shouldGenerateFile);
>-        else if (element.m_type == FormDataElement::Type::EncodedBlob)
>-            appendBlobResolved(newFormData.ptr(), element.m_url);
>-        else
>-            ASSERT_NOT_REACHED();
>+        WTF::visit(WTF::makeVisitor(
>+            [&] (const Vector<char>& bytes) {
>+                newFormData->appendData(bytes.data(), bytes.size());
>+            }, [&] (const FormDataElement::EncodedFileData& fileData) {
>+                newFormData->appendFileRange(fileData.filename, fileData.fileStart, fileData.fileLength, fileData.expectedFileModificationTime, fileData.shouldGenerateFile);
>+            }, [&] (const FormDataElement::EncodedBlobData& blobData) {
>+                appendBlobResolved(newFormData.ptr(), blobData.url);
>+            }
>+        ), element.data);
>     }
>     return newFormData;
> }
>@@ -374,14 +367,16 @@ void FormData::generateFiles(Document* d
>         return;
> 
>     for (auto& element : m_elements) {
>-        if (element.m_type == FormDataElement::Type::EncodedFile && element.m_shouldGenerateFile) {
>-            ASSERT(!element.m_ownsGeneratedFile);
>-            ASSERT(element.m_generatedFilename.isEmpty());
>-            if (!element.m_generatedFilename.isEmpty())
>-                continue;
>-            element.m_generatedFilename = page->chrome().client().generateReplacementFile(element.m_filename);
>-            if (!element.m_generatedFilename.isEmpty())
>-                element.m_ownsGeneratedFile = true;
>+        if (auto* fileData = WTF::get_if<FormDataElement::EncodedFileData>(element.data)) {
>+            if (fileData->shouldGenerateFile) {
>+                ASSERT(!fileData->ownsGeneratedFile);
>+                ASSERT(fileData->generatedFilename.isEmpty());
>+                if (!fileData->generatedFilename.isEmpty())
>+                    continue;
>+                fileData->generatedFilename = page->chrome().client().generateReplacementFile(fileData->filename);
>+                if (!fileData->generatedFilename.isEmpty())
>+                    fileData->ownsGeneratedFile = true;
>+            }
>         }
>     }
> }
>@@ -389,8 +384,10 @@ void FormData::generateFiles(Document* d
> bool FormData::hasGeneratedFiles() const
> {
>     for (auto& element : m_elements) {
>-        if (element.m_type == FormDataElement::Type::EncodedFile && !element.m_generatedFilename.isEmpty())
>-            return true;
>+        if (auto* fileData = WTF::get_if<FormDataElement::EncodedFileData>(element.data)) {
>+            if (!fileData->generatedFilename.isEmpty())
>+                return true;
>+        }
>     }
>     return false;
> }
>@@ -398,9 +395,11 @@ bool FormData::hasGeneratedFiles() const
> bool FormData::hasOwnedGeneratedFiles() const
> {
>     for (auto& element : m_elements) {
>-        if (element.m_type == FormDataElement::Type::EncodedFile && element.m_ownsGeneratedFile) {
>-            ASSERT(!element.m_generatedFilename.isEmpty());
>-            return true;
>+        if (auto* fileData = WTF::get_if<FormDataElement::EncodedFileData>(element.data)) {
>+            if (fileData->ownsGeneratedFile) {
>+                ASSERT(!fileData->generatedFilename.isEmpty());
>+                return true;
>+            }
>         }
>     }
>     return false;
>@@ -409,14 +408,16 @@ bool FormData::hasOwnedGeneratedFiles() 
> void FormData::removeGeneratedFilesIfNeeded()
> {
>     for (auto& element : m_elements) {
>-        if (element.m_type == FormDataElement::Type::EncodedFile && element.m_ownsGeneratedFile) {
>-            ASSERT(!element.m_generatedFilename.isEmpty());
>-            ASSERT(element.m_shouldGenerateFile);
>-            String directory = FileSystem::directoryName(element.m_generatedFilename);
>-            FileSystem::deleteFile(element.m_generatedFilename);
>-            FileSystem::deleteEmptyDirectory(directory);
>-            element.m_generatedFilename = String();
>-            element.m_ownsGeneratedFile = false;
>+        if (auto* fileData = WTF::get_if<FormDataElement::EncodedFileData>(element.data)) {
>+            if (fileData->ownsGeneratedFile) {
>+                ASSERT(!fileData->generatedFilename.isEmpty());
>+                ASSERT(fileData->shouldGenerateFile);
>+                String directory = FileSystem::directoryName(fileData->generatedFilename);
>+                FileSystem::deleteFile(fileData->generatedFilename);
>+                FileSystem::deleteEmptyDirectory(directory);
>+                fileData->generatedFilename = String();
>+                fileData->ownsGeneratedFile = false;
>+            }
>         }
>     }
> }
>@@ -435,7 +436,7 @@ uint64_t FormData::lengthInBytes() const
> RefPtr<SharedBuffer> FormData::asSharedBuffer() const
> {
>     for (auto& element : m_elements) {
>-        if (element.m_type != FormDataElement::Type::Data)
>+        if (!WTF::holds_alternative<Vector<char>>(element.data))
>             return nullptr;
>     }
>     return SharedBuffer::create(flatten());
>@@ -446,8 +447,9 @@ URL FormData::asBlobURL() const
>     if (m_elements.size() != 1)
>         return { };
> 
>-    ASSERT(m_elements.first().m_type == FormDataElement::Type::EncodedBlob || m_elements.first().m_url.isNull());
>-    return m_elements.first().m_url;
>+    if (auto* blobData = WTF::get_if<FormDataElement::EncodedBlobData>(m_elements.first().data))
>+        return blobData->url;
>+    return { };
> }
> 
> } // namespace WebCore
>Index: Source/WebCore/platform/network/FormData.h
>===================================================================
>--- Source/WebCore/platform/network/FormData.h	(revision 236150)
>+++ Source/WebCore/platform/network/FormData.h	(working copy)
>@@ -23,6 +23,7 @@
> #include "URL.h"
> #include <wtf/Forward.h>
> #include <wtf/RefCounted.h>
>+#include <wtf/Variant.h>
> #include <wtf/Vector.h>
> #include <wtf/text/WTFString.h>
> 
>@@ -34,157 +35,158 @@ class File;
> class SharedBuffer;
> class TextEncoding;
> 
>-// FIXME: Convert this to a Variant of structs and remove "Type" and also the "m_" prefixes from the data members.
>-// The member functions can become non-member fucntions.
>-class FormDataElement {
>-public:
>-    enum class Type { Data, EncodedFile, EncodedBlob };
>-
>-    FormDataElement()
>-        : m_type(Type::Data)
>-    {
>-    }
>-
>-    explicit FormDataElement(const Vector<char>& array)
>-        : m_type(Type::Data)
>-        , m_data(array)
>-    {
>-    }
>-
>-    FormDataElement(const String& filename, long long fileStart, long long fileLength, double expectedFileModificationTime, bool shouldGenerateFile)
>-        : m_type(Type::EncodedFile)
>-        , m_filename(filename)
>-        , m_fileStart(fileStart)
>-        , m_fileLength(fileLength)
>-        , m_expectedFileModificationTime(expectedFileModificationTime)
>-        , m_shouldGenerateFile(shouldGenerateFile)
>-        , m_ownsGeneratedFile(false)
>-    {
>-    }
>-
>+struct FormDataElement {
>+    struct EncodedFileData;
>+    struct EncodedBlobData;
>+    using Data = Variant<Vector<char>, EncodedFileData, EncodedBlobData>;
>+
>+    FormDataElement() = default;
>+    FormDataElement(Data&& data)
>+        : data(WTFMove(data)) { }
>+    explicit FormDataElement(Vector<char>&& array)
>+        : data(WTFMove(array)) { }
>+    FormDataElement(const String& filename, int64_t fileStart, int64_t fileLength, double expectedFileModificationTime, bool shouldGenerateFile)
>+        : data(EncodedFileData { filename, fileStart, fileLength, expectedFileModificationTime, shouldGenerateFile, { }, false }) { }
>     explicit FormDataElement(const URL& blobURL)
>-        : m_type(Type::EncodedBlob)
>-        , m_url(blobURL)
>-    {
>-    }
>+        : data(EncodedBlobData { blobURL }) { }
> 
>     uint64_t lengthInBytes() const;
> 
>     FormDataElement isolatedCopy() const;
> 
>-    template<typename Encoder> void encode(Encoder&) const;
>-    template<typename Decoder> static std::optional<FormDataElement> decode(Decoder&);
>-
>-    Type m_type;
>-    Vector<char> m_data;
>-    String m_filename;
>-    URL m_url; // For Blob or URL.
>-    int64_t m_fileStart;
>-    int64_t m_fileLength;
>-    double m_expectedFileModificationTime;
>-    // FIXME: Generated file support in FormData is almost identical to Blob, they should be merged.
>-    // We can't just switch to using Blobs for all files because EncodedFile form data elements do not
>-    // have a valid m_expectedFileModificationTime, meaning we always upload the latest content from disk.
>-    String m_generatedFilename;
>-    bool m_shouldGenerateFile;
>-    bool m_ownsGeneratedFile;
>-};
>-
>-inline bool operator==(const FormDataElement& a, const FormDataElement& b)
>-{
>-    if (&a == &b)
>-        return true;
>-
>-    if (a.m_type != b.m_type)
>-        return false;
>-    if (a.m_type == FormDataElement::Type::Data)
>-        return a.m_data == b.m_data;
>-    if (a.m_type == FormDataElement::Type::EncodedFile)
>-        return a.m_filename == b.m_filename && a.m_fileStart == b.m_fileStart && a.m_fileLength == b.m_fileLength && a.m_expectedFileModificationTime == b.m_expectedFileModificationTime;
>-    if (a.m_type == FormDataElement::Type::EncodedBlob)
>-        return a.m_url == b.m_url;
>-
>-    return true;
>-}
>-
>-inline bool operator!=(const FormDataElement& a, const FormDataElement& b)
>-{
>-    return !(a == b);
>-}
>-
>-template<typename Encoder>
>-void FormDataElement::encode(Encoder& encoder) const
>-{
>-    encoder.encodeEnum(m_type);
>-
>-    switch (m_type) {
>-    case Type::Data:
>-        encoder << m_data;
>-        break;
>-
>-    case Type::EncodedFile:
>-        encoder << m_filename;
>-        encoder << m_generatedFilename;
>-        encoder << m_shouldGenerateFile;
>-        encoder << m_fileStart;
>-        encoder << m_fileLength;
>-        encoder << m_expectedFileModificationTime;
>-        break;
>-
>-    case Type::EncodedBlob:
>-        encoder << m_url.string();
>-        break;
>+    template<typename Encoder> void encode(Encoder& encoder) const
>+    {
>+        encoder << data;
>     }
>-}
>-
>-template<typename Decoder>
>-std::optional<FormDataElement> FormDataElement::decode(Decoder& decoder)
>-{
>-    FormDataElement result;
>-    if (!decoder.decodeEnum(result.m_type))
>-        return std::nullopt;
>-
>-    switch (result.m_type) {
>-    case Type::Data:
>-        if (!decoder.decode(result.m_data))
>-            return std::nullopt;
>-
>-        return WTFMove(result);
>-
>-    case Type::EncodedFile:
>-        if (!decoder.decode(result.m_filename))
>-            return std::nullopt;
>-        if (!decoder.decode(result.m_generatedFilename))
>-            return std::nullopt;
>-        if (!decoder.decode(result.m_shouldGenerateFile))
>-            return std::nullopt;
>-        result.m_ownsGeneratedFile = false;
>-        if (!decoder.decode(result.m_fileStart))
>-            return std::nullopt;
>-        if (!decoder.decode(result.m_fileLength))
>-            return std::nullopt;
>-
>-        if (result.m_fileLength != BlobDataItem::toEndOfFile && result.m_fileLength < result.m_fileStart)
>-            return std::nullopt;
>-
>-        if (!decoder.decode(result.m_expectedFileModificationTime))
>+    template<typename Decoder> static std::optional<FormDataElement> decode(Decoder& decoder)
>+    {
>+        std::optional<Data> data;
>+        decoder >> data;
>+        if (!data)
>             return std::nullopt;
>+        return {{ WTFMove(*data) }};
>+    }
> 
>-        return WTFMove(result);
>-
>-    case Type::EncodedBlob: {
>-        String blobURLString;
>-        if (!decoder.decode(blobURLString))
>-            return std::nullopt;
>+    struct EncodedFileData {
>+        String filename;
>+        int64_t fileStart { 0 };
>+        int64_t fileLength { 0 };
>+        double expectedFileModificationTime { 0 };
>+        bool shouldGenerateFile { false };
>+        String generatedFilename;
>+        bool ownsGeneratedFile { false };
>+
>+        // FIXME: Generated file support in FormData is almost identical to Blob, they should be merged.
>+        // We can't just switch to using Blobs for all files because EncodedFile form data elements do not
>+        // have a valid expectedFileModificationTime, meaning we always upload the latest content from disk.
>+
>+        EncodedFileData isolatedCopy() const
>+        {
>+            return { filename.isolatedCopy(), fileStart, fileLength, expectedFileModificationTime, shouldGenerateFile, generatedFilename.isolatedCopy(), ownsGeneratedFile };
>+        }
>+        
>+        bool operator==(const EncodedFileData other) const
>+        {
>+            return filename == other.filename
>+                && fileStart == other.fileStart
>+                && fileLength == other.fileLength
>+                && expectedFileModificationTime == other.expectedFileModificationTime
>+                && shouldGenerateFile == other.shouldGenerateFile
>+                && generatedFilename == other.generatedFilename
>+                && ownsGeneratedFile == other.ownsGeneratedFile;
>+        }
>+        template<typename Encoder> void encode(Encoder& encoder) const
>+        {
>+            encoder << filename << fileStart << expectedFileModificationTime << shouldGenerateFile << generatedFilename;
>+        }
>+        template<typename Decoder> static std::optional<EncodedFileData> decode(Decoder& decoder)
>+        {
>+            std::optional<String> filename;
>+            decoder >> filename;
>+            if (!filename)
>+                return std::nullopt;
>+            
>+            std::optional<int64_t> fileStart;
>+            decoder >> fileStart;
>+            if (!fileStart)
>+                return std::nullopt;
>+            
>+            std::optional<int64_t> fileLength;
>+            decoder >> fileLength;
>+            if (!fileLength)
>+                return std::nullopt;
>+            
>+            std::optional<double> expectedFileModificationTime;
>+            decoder >> expectedFileModificationTime;
>+            if (!expectedFileModificationTime)
>+                return std::nullopt;
>+            
>+            std::optional<bool> shouldGenerateFile;
>+            decoder >> shouldGenerateFile;
>+            if (!shouldGenerateFile)
>+                return std::nullopt;
>+            
>+            std::optional<String> generatedFilename;
>+            decoder >> generatedFilename;
>+            if (!generatedFilename)
>+                return std::nullopt;
>+            
>+            bool ownsGeneratedFile = false;
>+            
>+            return {{
>+                WTFMove(*filename),
>+                WTFMove(*fileStart),
>+                WTFMove(*fileLength),
>+                WTFMove(*expectedFileModificationTime),
>+                WTFMove(*shouldGenerateFile),
>+                WTFMove(*generatedFilename),
>+                WTFMove(ownsGeneratedFile)
>+            }};
>+        }
> 
>-        result.m_url = URL(URL(), blobURLString);
>+    };
>+    
>+    struct EncodedBlobData {
>+        URL url;
>+
>+        bool operator==(const EncodedBlobData other) const
>+        {
>+            return url == other.url;
>+        }
>+        template<typename Encoder> void encode(Encoder& encoder) const
>+        {
>+            encoder << url;
>+        }
>+        template<typename Decoder> static std::optional<EncodedBlobData> decode(Decoder& decoder)
>+        {
>+            std::optional<URL> url;
>+            decoder >> url;
>+            if (!url)
>+                return std::nullopt;
> 
>-        return WTFMove(result);
>+            return {{ WTFMove(*url) }};
>+        }
>+    };
>+    
>+    bool operator==(const FormDataElement& other) const
>+    {
>+        if (&other == this)
>+            return true;
>+        if (data.index() != other.data.index())
>+            return false;
>+        if (!data.index())
>+            return WTF::get<0>(data) == WTF::get<0>(other.data);
>+        if (data.index() == 1)
>+            return WTF::get<1>(data) == WTF::get<1>(other.data);
>+        return WTF::get<2>(data) == WTF::get<2>(other.data);
>     }
>+    bool operator!=(const FormDataElement& other) const
>+    {
>+        return !(*this == other);
>     }
>-
>-    return std::nullopt;
>-}
>+    
>+    Data data;
>+};
> 
> class FormData : public RefCounted<FormData> {
> public:
>@@ -217,7 +219,6 @@ public:
>     void appendFile(const String& filePath, bool shouldGenerateFile = false);
>     WEBCORE_EXPORT void appendFileRange(const String& filename, long long start, long long length, double expectedModificationTime, bool shouldGenerateFile = false);
>     WEBCORE_EXPORT void appendBlob(const URL& blobURL);
>-    char* expandDataStore(size_t);
> 
>     Vector<char> flatten() const; // omits files
>     String flattenToString() const; // omits files
>Index: Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp
>===================================================================
>--- Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp	(revision 236150)
>+++ Source/WebCore/platform/network/cf/FormDataStreamCFNet.cpp	(working copy)
>@@ -136,31 +136,40 @@ static bool advanceCurrentStream(FormStr
>     // Create the new stream.
>     FormDataElement& nextInput = form->remainingElements.last();
> 
>-    if (nextInput.m_type == FormDataElement::Type::Data) {
>-        size_t size = nextInput.m_data.size();
>-        MallocPtr<char> data = nextInput.m_data.releaseBuffer();
>-        form->currentStream = CFReadStreamCreateWithBytesNoCopy(0, reinterpret_cast<const UInt8*>(data.get()), size, kCFAllocatorNull);
>-        form->currentData = WTFMove(data);
>-    } else {
>-        // Check if the file has been changed or not if required.
>-        if (FileSystem::isValidFileTime(nextInput.m_expectedFileModificationTime)) {
>-            time_t fileModificationTime;
>-            if (!FileSystem::getFileModificationTime(nextInput.m_filename, fileModificationTime) || fileModificationTime != static_cast<time_t>(nextInput.m_expectedFileModificationTime))
>+    bool success = WTF::visit(WTF::makeVisitor(
>+        [form] (Vector<char>& bytes) {
>+            size_t size = bytes.size();
>+            MallocPtr<char> data = bytes.releaseBuffer();
>+            form->currentStream = CFReadStreamCreateWithBytesNoCopy(0, reinterpret_cast<const UInt8*>(data.get()), size, kCFAllocatorNull);
>+            form->currentData = WTFMove(data);
>+            return true;
>+        }, [form] (const FormDataElement::EncodedFileData& fileData) {
>+            // Check if the file has been changed or not if required.
>+            if (FileSystem::isValidFileTime(fileData.expectedFileModificationTime)) {
>+                time_t fileModificationTime;
>+                if (!FileSystem::getFileModificationTime(fileData.filename, fileModificationTime) || fileModificationTime != static_cast<time_t>(fileData.expectedFileModificationTime))
>                 return false;
>-        }
>-        const String& path = nextInput.m_shouldGenerateFile ? nextInput.m_generatedFilename : nextInput.m_filename;
>-        form->currentStream = CFReadStreamCreateWithFile(0, FileSystem::pathAsURL(path).get());
>-        if (!form->currentStream) {
>-            // The file must have been removed or become unreadable.
>+            }
>+            const String& path = fileData.shouldGenerateFile ? fileData.generatedFilename : fileData.filename;
>+            form->currentStream = CFReadStreamCreateWithFile(0, FileSystem::pathAsURL(path).get());
>+            if (!form->currentStream) {
>+                // The file must have been removed or become unreadable.
>+                return false;
>+            }
>+            if (fileData.fileStart > 0) {
>+                RetainPtr<CFNumberRef> position = adoptCF(CFNumberCreate(0, kCFNumberLongLongType, &fileData.fileStart));
>+                CFReadStreamSetProperty(form->currentStream, kCFStreamPropertyFileCurrentOffset, position.get());
>+            }
>+            form->currentStreamRangeLength = fileData.fileLength;
>+            return true;
>+        }, [] (const FormDataElement::EncodedBlobData&) {
>+            ASSERT_NOT_REACHED();
>             return false;
>         }
>-        if (nextInput.m_fileStart > 0) {
>-            RetainPtr<CFNumberRef> position = adoptCF(CFNumberCreate(0, kCFNumberLongLongType, &nextInput.m_fileStart));
>-            CFReadStreamSetProperty(form->currentStream, kCFStreamPropertyFileCurrentOffset, position.get());
>-        }
>-        form->currentStreamRangeLength = nextInput.m_fileLength;
>-    }
>-    form->remainingElements.removeLast();
>+    ), nextInput.data);
>+
>+    if (!success)
>+        return false;
> 
>     // Set up the callback.
>     CFStreamClientContext context = { 0, form, 0, 0, 0 };
>@@ -363,22 +372,8 @@ RetainPtr<CFReadStreamRef> createHTTPBod
> 
>     // Precompute the content length so CFNetwork doesn't use chunked mode.
>     unsigned long long length = 0;
>-
>-    for (auto& element : resolvedFormData->elements()) {
>-        if (element.m_type == FormDataElement::Type::Data)
>-            length += element.m_data.size();
>-        else {
>-            // If we're sending the file range, use the existing range length for now.
>-            // We will detect if the file has been changed right before we read the file and abort the operation if necessary.
>-            if (element.m_fileLength != BlobDataItem::toEndOfFile) {
>-                length += element.m_fileLength;
>-                continue;
>-            }
>-            long long fileSize;
>-            if (FileSystem::getFileSize(element.m_shouldGenerateFile ? element.m_generatedFilename : element.m_filename, fileSize))
>-                length += fileSize;
>-        }
>-    }
>+    for (auto& element : resolvedFormData->elements())
>+        length += element.lengthInBytes();
> 
>     FormCreationContext formContext = { WTFMove(resolvedFormData), length };
>     CFReadStreamCallBacksV1 callBacks = { 1, formCreate, formFinalize, nullptr, formOpen, nullptr, formRead, nullptr, formCanRead, formClose, formCopyProperty, nullptr, nullptr, formSchedule, formUnschedule };
>@@ -393,10 +388,8 @@ void setHTTPBody(CFMutableURLRequestRef 
>     // Handle the common special case of one piece of form data, with no files.
>     auto& elements = formData->elements();
>     if (elements.size() == 1 && !formData->alwaysStream()) {
>-        auto& element = elements[0];
>-        if (element.m_type == FormDataElement::Type::Data) {
>-            auto& vector = element.m_data;
>-            auto data = adoptCF(CFDataCreate(nullptr, reinterpret_cast<const UInt8*>(vector.data()), vector.size()));
>+        if (auto* vector = WTF::get_if<Vector<char>>(elements[0].data)) {
>+            auto data = adoptCF(CFDataCreate(nullptr, reinterpret_cast<const UInt8*>(vector->data()), vector->size()));
>             CFURLRequestSetHTTPRequestBody(request, data.get());
>             return;
>         }
>Index: Source/WebCore/platform/network/curl/CurlFormDataStream.cpp
>===================================================================
>--- Source/WebCore/platform/network/curl/CurlFormDataStream.cpp	(revision 236150)
>+++ Source/WebCore/platform/network/curl/CurlFormDataStream.cpp	(working copy)
>@@ -103,21 +103,8 @@ void CurlFormDataStream::computeContentL
> 
>     m_isContentLengthUpdated = true;
> 
>-    for (const auto& element : m_formData->elements()) {
>-        if (element.m_type == FormDataElement::Type::EncodedFile) {
>-            long long fileSize;
>-            if (FileSystem::getFileSize(element.m_filename, fileSize)) {
>-                if (fileSize > maxCurlOffT) {
>-                    // File size is too big for specifying it to curl
>-                    m_shouldUseChunkTransfer = true;
>-                }
>-
>-                m_totalSize += fileSize;
>-            } else
>-                m_shouldUseChunkTransfer = true;
>-        } else
>-            m_totalSize += element.m_data.size();
>-    }
>+    for (const auto& element : m_formData->elements())
>+        m_totalSize += element.lengthInBytes();
> }
> 
> std::optional<size_t> CurlFormDataStream::read(char* buffer, size_t size)
>@@ -138,10 +125,19 @@ std::optional<size_t> CurlFormDataStream
>         size_t bufferSize = size - totalReadBytes;
>         char* bufferPosition = buffer + totalReadBytes;
> 
>-        if (element.m_type == FormDataElement::Type::EncodedFile)
>-            readBytes = readFromFile(element, bufferPosition, bufferSize);
>-        else
>-            readBytes = readFromData(element, bufferPosition, bufferSize);
>+        readBytes = WTF::visit(WTF::makeVisitor(
>+            [&] (const Vector<char>& bytes) {
>+                return readFromData(bytes, bufferPosition, bufferSize);
>+                Vector<char> copy;
>+                copy.append(&bytes[0], bytes.size());
>+                return FormDataElement(WTFMove(copy));
>+            }, [&] (const FormDataElement::EncodedFileData& fileData) {
>+                return readFromFile(fileData, bufferPosition, bufferSize);
>+            }, [] (const FormDataElement::EncodedBlobData& blobData) {
>+                ASSERT_NOT_REACHED();
>+                return std::nullopt;
>+            }
>+        ), element.data);
> 
>         if (!readBytes)
>             return std::nullopt;
>@@ -154,20 +150,20 @@ std::optional<size_t> CurlFormDataStream
>     return totalReadBytes;
> }
> 
>-std::optional<size_t> CurlFormDataStream::readFromFile(const FormDataElement& element, char* buffer, size_t size)
>+std::optional<size_t> CurlFormDataStream::readFromFile(const FormDataElement::EncodedFileData& fileData, char* buffer, size_t size)
> {
>     if (m_fileHandle == FileSystem::invalidPlatformFileHandle)
>-        m_fileHandle = FileSystem::openFile(element.m_filename, FileSystem::FileOpenMode::Read);
>+        m_fileHandle = FileSystem::openFile(fileData.filename, FileSystem::FileOpenMode::Read);
> 
>     if (!FileSystem::isHandleValid(m_fileHandle)) {
>-        LOG(Network, "Curl - Failed while trying to open %s for upload\n", element.m_filename.utf8().data());
>+        LOG(Network, "Curl - Failed while trying to open %s for upload\n", fileData.filename.utf8().data());
>         m_fileHandle = FileSystem::invalidPlatformFileHandle;
>         return std::nullopt;
>     }
> 
>     auto readBytes = FileSystem::readFromFile(m_fileHandle, buffer, size);
>     if (readBytes < 0) {
>-        LOG(Network, "Curl - Failed while trying to read %s for upload\n", element.m_filename.utf8().data());
>+        LOG(Network, "Curl - Failed while trying to read %s for upload\n", fileData.filename.utf8().data());
>         FileSystem::closeFile(m_fileHandle);
>         m_fileHandle = FileSystem::invalidPlatformFileHandle;
>         return std::nullopt;
>@@ -182,10 +178,10 @@ std::optional<size_t> CurlFormDataStream
>     return readBytes;
> }
> 
>-std::optional<size_t> CurlFormDataStream::readFromData(const FormDataElement& element, char* buffer, size_t size)
>+std::optional<size_t> CurlFormDataStream::readFromData(const Vector<char>& data, char* buffer, size_t size)
> {
>-    size_t elementSize = element.m_data.size() - m_dataOffset;
>-    const char* elementBuffer = element.m_data.data() + m_dataOffset;
>+    size_t elementSize = data.size() - m_dataOffset;
>+    const char* elementBuffer = data.data() + m_dataOffset;
> 
>     size_t readBytes = elementSize > size ? size : elementSize;
>     memcpy(buffer, elementBuffer, readBytes);
>Index: Source/WebCore/platform/network/curl/CurlFormDataStream.h
>===================================================================
>--- Source/WebCore/platform/network/curl/CurlFormDataStream.h	(revision 236150)
>+++ Source/WebCore/platform/network/curl/CurlFormDataStream.h	(working copy)
>@@ -51,8 +51,8 @@ public:
> private:
>     void computeContentLength();
> 
>-    std::optional<size_t> readFromFile(const FormDataElement&, char*, size_t);
>-    std::optional<size_t> readFromData(const FormDataElement&, char*, size_t);
>+    std::optional<size_t> readFromFile(const FormDataElement::EncodedFileData&, char*, size_t);
>+    std::optional<size_t> readFromData(const Vector<char>&, char*, size_t);
> 
>     RefPtr<FormData> m_formData;
> 
>Index: Source/WebKit/ChangeLog
>===================================================================
>--- Source/WebKit/ChangeLog	(revision 236239)
>+++ Source/WebKit/ChangeLog	(working copy)
>@@ -1,3 +1,17 @@
>+2018-09-19  Alex Christensen  <achristensen@webkit.org>
>+
>+        Use a Variant for FormDataElement
>+        https://bugs.webkit.org/show_bug.cgi?id=189777
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * NetworkProcess/NetworkResourceLoadParameters.cpp:
>+        (WebKit::NetworkResourceLoadParameters::encode const):
>+        * NetworkProcess/NetworkResourceLoader.cpp:
>+        * Shared/SessionState.h:
>+        * WebProcess/WebCoreSupport/SessionStateConversion.cpp:
>+        (WebKit::toHTTPBody):
>+
> 2018-09-19  Dean Jackson  <dino@grorg.org>
> 
>         Temporarily move fullscreen back to experimental features
>Index: Source/WebKit/NetworkProcess/NetworkResourceLoadParameters.cpp
>===================================================================
>--- Source/WebKit/NetworkProcess/NetworkResourceLoadParameters.cpp	(revision 236150)
>+++ Source/WebKit/NetworkProcess/NetworkResourceLoadParameters.cpp	(working copy)
>@@ -49,7 +49,7 @@ void NetworkResourceLoadParameters::enco
>         const Vector<FormDataElement>& elements = request.httpBody()->elements();
>         size_t fileCount = 0;
>         for (size_t i = 0, count = elements.size(); i < count; ++i) {
>-            if (elements[i].m_type == FormDataElement::Type::EncodedFile)
>+            if (WTF::holds_alternative<FormDataElement::EncodedFileData>(elements[i].data))
>                 ++fileCount;
>         }
> 
>@@ -58,8 +58,8 @@ void NetworkResourceLoadParameters::enco
>         size_t extensionIndex = 0;
>         for (size_t i = 0, count = elements.size(); i < count; ++i) {
>             const FormDataElement& element = elements[i];
>-            if (element.m_type == FormDataElement::Type::EncodedFile) {
>-                const String& path = element.m_shouldGenerateFile ? element.m_generatedFilename : element.m_filename;
>+            if (auto* fileData = WTF::get_if<FormDataElement::EncodedFileData>(element.data)) {
>+                const String& path = fileData->shouldGenerateFile ? fileData->generatedFilename : fileData->filename;
>                 SandboxExtension::createHandle(path, SandboxExtension::Type::ReadOnly, requestBodySandboxExtensions[extensionIndex++]);
>             }
>         }
>Index: Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp
>===================================================================
>--- Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp	(revision 236150)
>+++ Source/WebKit/NetworkProcess/NetworkResourceLoader.cpp	(working copy)
>@@ -112,8 +112,8 @@ NetworkResourceLoader::NetworkResourceLo
> 
>     if (originalRequest().httpBody()) {
>         for (const auto& element : originalRequest().httpBody()->elements()) {
>-            if (element.m_type == FormDataElement::Type::EncodedBlob)
>-                m_fileReferences.appendVector(NetworkBlobRegistry::singleton().filesInBlob(connection, element.m_url));
>+            if (auto* blobData = WTF::get_if<FormDataElement::EncodedBlobData>(element.data))
>+                m_fileReferences.appendVector(NetworkBlobRegistry::singleton().filesInBlob(connection, blobData->url));
>         }
>     }
> 
>Index: Source/WebKit/Shared/SessionState.h
>===================================================================
>--- Source/WebKit/Shared/SessionState.h	(revision 236150)
>+++ Source/WebKit/Shared/SessionState.h	(working copy)
>@@ -59,6 +59,7 @@ struct HTTPBody {
>             Blob,
>         };
> 
>+        // FIXME: This should be a Variant. It's also unclear why we don't just use FormDataElement here.
>         Type type = Type::Data;
> 
>         // Data.
>Index: Source/WebKit/WebProcess/WebCoreSupport/SessionStateConversion.cpp
>===================================================================
>--- Source/WebKit/WebProcess/WebCoreSupport/SessionStateConversion.cpp	(revision 236150)
>+++ Source/WebKit/WebProcess/WebCoreSupport/SessionStateConversion.cpp	(working copy)
>@@ -42,25 +42,21 @@ static HTTPBody toHTTPBody(const FormDat
>     for (const auto& formDataElement : formData.elements()) {
>         HTTPBody::Element element;
> 
>-        switch (formDataElement.m_type) {
>-        case FormDataElement::Type::Data:
>-            element.type = HTTPBody::Element::Type::Data;
>-            element.data = formDataElement.m_data;
>-            break;
>-
>-        case FormDataElement::Type::EncodedFile:
>-            element.filePath = formDataElement.m_filename;
>-            element.fileStart = formDataElement.m_fileStart;
>-            if (formDataElement.m_fileLength != BlobDataItem::toEndOfFile)
>-                element.fileLength = formDataElement.m_fileLength;
>-            if (formDataElement.m_expectedFileModificationTime != FileSystem::invalidFileTime())
>-                element.expectedFileModificationTime = formDataElement.m_expectedFileModificationTime;
>-            break;
>-
>-        case FormDataElement::Type::EncodedBlob:
>-            element.blobURLString = formDataElement.m_url.string();
>-            break;
>-        }
>+        WTF::visit(WTF::makeVisitor(
>+            [&] (const Vector<char>& bytes) {
>+                element.type = HTTPBody::Element::Type::Data;
>+                element.data = bytes;
>+            }, [&] (const FormDataElement::EncodedFileData& fileData) {
>+                element.filePath = fileData.filename;
>+                element.fileStart = fileData.fileStart;
>+                if (fileData.fileLength != BlobDataItem::toEndOfFile)
>+                    element.fileLength = fileData.fileLength;
>+                if (fileData.expectedFileModificationTime != FileSystem::invalidFileTime())
>+                    element.expectedFileModificationTime = fileData.expectedFileModificationTime;
>+            }, [&] (const FormDataElement::EncodedBlobData& blobData) {
>+                element.blobURLString = blobData.url.string();
>+            }
>+        ), formDataElement.data);
> 
>         httpBody.elements.append(WTFMove(element));
>     }

Actions: View | Formatted Diff | Diff

Attachments on bug 189777: 350169 | 350171 | 350173 | 350176 | 350177 | 350240 | 350245 | 350246 | 350249 | 350260 | 350270 | 350285 | 350299 | 350318 | 350324 | 350330 | 350335 | 350428 | 350430