change event for input[type=file] does not fire when different file…

… with same name is selected

https://bugs.webkit.org/show_bug.cgi?id=187084
rdar://98460303

Reviewed by Ryosuke Niwa and Aditya Keerthi.

When a different file with the same name as the currently selected file is selected,
the `change` event does not currently fire, because a file's path is it's
identifier, so the two files are considered the same by the system.

This PR works around this by saving each file's unique id and then comparing the files
both based on their name and now their id too.

* Source/WTF/wtf/FileSystem.h:
* Source/WTF/wtf/glib/FileSystemGlib.cpp:
(WTF::FileSystemImpl::fileID):
(WTF::FileSystemImpl::fileIDsAreEqual):
(WTF::FileSystemImpl::08c7e637e60a):
(WTF::FileSystemImpl::fileCreationTime): Deleted.
* Source/WTF/wtf/posix/FileSystemPOSIX.cpp:
(WTF::FileSystemImpl::fileID):
(WTF::FileSystemImpl::fileIDsAreEqual):
* Source/WTF/wtf/win/FileSystemWin.cpp:
(WTF::FileSystemImpl::fileID):
(WTF::FileSystemImpl::fileIDsAreEqual):
* Source/WebCore/fileapi/File.cpp:
(WebCore::File::create):
(WebCore::File::File):
* Source/WebCore/fileapi/File.h:
* Source/WebCore/html/FileInputType.cpp:
(WebCore::FileInputType::setFiles):
(WebCore::FileInputType::filesChosen):
* Source/WebCore/platform/FileChooser.cpp:
(WebCore::FileChooser::chooseFiles):
(WebCore::FileChooser::chooseMediaFiles):

Canonical link: https://commits.webkit.org/255297@main

Source/WTF/wtf/FileSystem.h

@@ -75,6 +75,15 @@ typedef int PlatformFileHandle;
 const PlatformFileHandle invalidPlatformFileHandle = -1;
 #endif
 
+// PlatformFileID
+#if USE(GLIB) && !OS(WINDOWS)
+typedef uint64_t PlatformFileID;
+#elif OS(WINDOWS)
+typedef FILE_ID_128 PlatformFileID;
+#else
+typedef ino_t PlatformFileID;
+#endif
+
 enum class FileOpenMode {
     Read,
     Write,
@@ -114,6 +123,8 @@ WTF_EXPORT_PRIVATE bool moveFile(const String& oldPath, const String& newPath);
 WTF_EXPORT_PRIVATE std::optional<uint64_t> fileSize(const String&); // Follows symlinks.
 WTF_EXPORT_PRIVATE std::optional<uint64_t> fileSize(PlatformFileHandle);
 WTF_EXPORT_PRIVATE std::optional<WallTime> fileModificationTime(const String&);
+WTF_EXPORT_PRIVATE std::optional<PlatformFileID> fileID(PlatformFileHandle);
+WTF_EXPORT_PRIVATE bool fileIDsAreEqual(std::optional<PlatformFileID>, std::optional<PlatformFileID>);
 WTF_EXPORT_PRIVATE bool updateFileModificationTime(const String& path); // Sets modification time to now.
 WTF_EXPORT_PRIVATE std::optional<WallTime> fileCreationTime(const String&); // Not all platforms store file creation time.
 WTF_EXPORT_PRIVATE bool isHiddenFile(const String&);

Source/WTF/wtf/glib/FileSystemGlib.cpp

@@ -134,6 +134,18 @@ std::optional<uint64_t> fileSize(PlatformFileHandle handle)
     return g_file_info_get_size(info.get());
 }
 
+std::optional<PlatformFileID> fileID(PlatformFileHandle handle)
+{
+    // FIXME (246118): Implement this function properly.
+    return std::nullopt;
+}
+
+bool fileIDsAreEqual(std::optional<PlatformFileID> a, std::optional<PlatformFileID> b)
+{
+    // FIXME (246118): Implement this function properly.
+    return true;
+}
+
 std::optional<WallTime> fileCreationTime(const String& path)
 {
     const auto filename = fileSystemRepresentation(path);

Source/WTF/wtf/posix/FileSystemPOSIX.cpp

@@ -190,6 +190,20 @@ std::optional<WallTime> fileCreationTime(const String& path)
 #endif
 }
 
+std::optional<PlatformFileID> fileID(PlatformFileHandle handle)
+{
+    struct stat fileInfo;
+    if (fstat(handle, &fileInfo))
+        return std::nullopt;
+
+    return fileInfo.st_ino;
+}
+
+bool fileIDsAreEqual(std::optional<PlatformFileID> a, std::optional<PlatformFileID> b)
+{
+    return a == b;
+}
+
 std::optional<uint32_t> volumeFileBlockSize(const String& path)
 {
     struct statvfs fileStat;

Source/WTF/wtf/win/FileSystemWin.cpp

@@ -112,6 +112,18 @@ std::optional<uint64_t> fileSize(PlatformFileHandle fileHandle)
     return getFileSizeFromByHandleFileInformationStructure(fileInformation);
 }
 
+std::optional<PlatformFileID> fileID(PlatformFileHandle fileHandle)
+{
+    // FIXME (246118): Implement this function properly.
+    return std::nullopt;
+}
+
+bool fileIDsAreEqual(std::optional<PlatformFileID> a, std::optional<PlatformFileID> b)
+{
+    // FIXME (246118): Implement this function properly.
+    return true;
+}
+
 std::optional<WallTime> fileCreationTime(const String& path)
 {
     WIN32_FIND_DATAW findData;

Source/WebCore/fileapi/File.cpp

@@ -45,7 +45,7 @@ Ref<File> File::createWithRelativePath(ScriptExecutionContext* context, const St
     return file;
 }
 
-Ref<File> File::create(ScriptExecutionContext* context, const String& path, const String& replacementPath, const String& nameOverride)
+Ref<File> File::create(ScriptExecutionContext* context, const String& path, const String& replacementPath, const String& nameOverride, const std::optional<FileSystem::PlatformFileID>& fileID)
 {
     String name;
     String type;
@@ -55,7 +55,7 @@ Ref<File> File::create(ScriptExecutionContext* context, const String& path, cons
     auto internalURL = BlobURL::createInternalURL();
     ThreadableBlobRegistry::registerFileBlobURL(internalURL, path, replacementPath, type);
 
-    auto file = adoptRef(*new File(context, WTFMove(internalURL), WTFMove(type), WTFMove(effectivePath), WTFMove(name)));
+    auto file = adoptRef(*new File(context, WTFMove(internalURL), WTFMove(type), WTFMove(effectivePath), WTFMove(name), fileID));
     file->suspendIfNeeded();
     return file;
 }
@@ -67,6 +67,14 @@ File::File(ScriptExecutionContext* context, URL&& url, String&& type, String&& p
 {
 }
 
+File::File(ScriptExecutionContext* context, URL&& url, String&& type, String&& path, String&& name, const std::optional<FileSystem::PlatformFileID>& fileID)
+    : Blob(uninitializedContructor, context, WTFMove(url), WTFMove(type))
+    , m_path(WTFMove(path))
+    , m_name(WTFMove(name))
+    , m_fileID(fileID)
+{
+}
+
 File::File(DeserializationContructor, ScriptExecutionContext* context, const String& path, const URL& url, const String& type, const String& name, const std::optional<int64_t>& lastModified)
     : Blob(deserializationContructor, context, url, type, { }, 0, path)
     , m_path(path)

Source/WebCore/fileapi/File.h

@@ -26,6 +26,7 @@
 #pragma once
 
 #include "Blob.h"
+#include <wtf/FileSystem.h>
 #include <wtf/IsoMalloc.h>
 #include <wtf/Ref.h>
 #include <wtf/TypeCasts.h>
@@ -41,7 +42,7 @@ class File final : public Blob {
     };
 
     // 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(ScriptExecutionContext*, const String& path, const String& replacementPath = { }, const String& nameOverride = { });
+    WEBCORE_EXPORT static Ref<File> create(ScriptExecutionContext*, const String& path, const String& replacementPath = { }, const String& nameOverride = { }, const std::optional<FileSystem::PlatformFileID>& fileID = { });
 
     // Create a File using the 'new File' constructor.
     static Ref<File> create(ScriptExecutionContext& context, Vector<BlobPartVariant>&& blobPartVariants, const String& filename, const PropertyBag& propertyBag)
@@ -82,6 +83,7 @@ class File final : public Blob {
     const String& name() const { return m_name; }
     WEBCORE_EXPORT int64_t lastModified() const; // Number of milliseconds since Epoch.
     const std::optional<int64_t>& lastModifiedOverride() const { return m_lastModifiedDateOverride; } // Number of milliseconds since Epoch.
+    const std::optional<FileSystem::PlatformFileID> fileID() const { return m_fileID; }
 
     static String contentTypeForFile(const String& path);
 
@@ -95,6 +97,7 @@ class File final : public Blob {
     WEBCORE_EXPORT explicit File(ScriptExecutionContext*, const String& path);
     File(ScriptExecutionContext*, URL&&, String&& type, String&& path, String&& name);
     File(ScriptExecutionContext&, Vector<BlobPartVariant>&& blobPartVariants, const String& filename, const PropertyBag&);
+    File(ScriptExecutionContext*, URL&&, String&& type, String&& path, String&& name, const std::optional<FileSystem::PlatformFileID>&);
     File(ScriptExecutionContext*, const Blob&, const String& name);
     File(ScriptExecutionContext*, const File&, const String& name);
 
@@ -113,6 +116,7 @@ class File final : public Blob {
     String m_name;
 
     std::optional<int64_t> m_lastModifiedDateOverride;
+    std::optional<FileSystem::PlatformFileID> m_fileID;
     mutable std::optional<bool> m_isDirectory;
 };
 

Source/WebCore/html/FileInputType.cpp

@@ -374,7 +374,7 @@ void FileInputType::setFiles(RefPtr<FileList>&& files, RequestIcon shouldRequest
         pathsChanged = true;
     else {
         for (unsigned i = 0; i < length; ++i) {
-            if (files->file(i).path() != m_fileList->file(i).path()) {
+            if (files->file(i).path() != m_fileList->file(i).path() || !FileSystem::fileIDsAreEqual(files->file(i).fileID(), m_fileList->file(i).fileID())) {
                 pathsChanged = true;
                 break;
             }
@@ -415,7 +415,11 @@ void FileInputType::filesChosen(const Vector<FileChooserFileInfo>& paths, const
     auto* document = element() ? &element()->document() : nullptr;
     if (!allowsDirectories()) {
         auto files = paths.map([document](auto& fileInfo) {
-            return File::create(document, fileInfo.path, fileInfo.replacementPath, fileInfo.displayName);
+            auto handle = FileSystem::openFile(fileInfo.path, FileSystem::FileOpenMode::Read);
+            auto fileID = FileSystem::fileID(handle);
+            FileSystem::closeFile(handle);
+
+            return File::create(document, fileInfo.path, fileInfo.replacementPath, fileInfo.displayName, fileID);
         });
         didCreateFileList(FileList::create(WTFMove(files)), icon);
         return;

Source/WebCore/platform/FileChooser.cpp

@@ -58,10 +58,6 @@ void FileChooser::chooseFile(const String& filename)
 
 void FileChooser::chooseFiles(const Vector<String>& filenames, const Vector<String>& replacementNames)
 {
-    // FIXME: This is inelegant. We should not be looking at settings here.
-    if (m_settings.selectedFiles == filenames)
-        return;
-
     if (!m_client)
         return;
 
@@ -78,10 +74,6 @@ void FileChooser::chooseFiles(const Vector<String>& filenames, const Vector<Stri
 // with FileChooser::chooseFiles() and hence remove the PLATFORM(IOS_FAMILY)-guard.
 void FileChooser::chooseMediaFiles(const Vector<String>& filenames, const String& displayString, Icon* icon)
 {
-    // FIXME: This is inelegant. We should not be looking at settings here.
-    if (m_settings.selectedFiles == filenames)
-        return;
-
     if (!m_client)
         return;
 
@@ -99,10 +91,6 @@ void FileChooser::chooseFiles(const Vector<FileChooserFileInfo>& files)
         return file.path;
     });
 
-    // FIXME: This is inelegant. We should not be looking at settings here.
-    if (m_settings.selectedFiles == paths)
-        return;
-
     if (m_client)
         m_client->filesChosen(files);
 }