Fix userVisibility for files in ChromeOS filesystems.

In Web applications, the Filesystem API is used to create sandboxed
filesystems. Files in these filesystems should never be shown in a
filepicker. ChromeOS supports a few special filesystems (Downloads,
Gallery) where the files should be shown in filepickers.

This change fixes the userVisibility property for File instances created
from such filesystems.

BUG=398366

Review URL: https://codereview.chromium.org/516763002

git-svn-id: svn://svn.chromium.org/blink/trunk@181530 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Source/core/fileapi/File.cpp

@@ -137,10 +137,10 @@ File::File(const String& name, double modificationTime, PassRefPtr<BlobDataHandl
 {
 }
 
-File::File(const String& name, const FileMetadata& metadata)
+File::File(const String& name, const FileMetadata& metadata, UserVisibility userVisibility)
     : Blob(BlobDataHandle::create(createBlobDataForFileWithMetadata(name, metadata), metadata.length))
     , m_hasBackingFile(true)
-    , m_userVisibility(File::IsNotUserVisible)
+    , m_userVisibility(userVisibility)
     , m_path(metadata.platformPath)
     , m_name(name)
     , m_snapshotSize(metadata.length)

third_party/WebKit/Source/core/fileapi/File.h

@@ -77,9 +77,9 @@ public:
     // 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 slice() will synchronously query the file metadata.
-    static PassRefPtrWillBeRawPtr<File> createForFileSystemFile(const String& name, const FileMetadata& metadata)
+    static PassRefPtrWillBeRawPtr<File> createForFileSystemFile(const String& name, const FileMetadata& metadata, UserVisibility userVisibility)
     {
-        return adoptRefWillBeNoop(new File(name, metadata));
+        return adoptRefWillBeNoop(new File(name, metadata, userVisibility));
     }
 
     static PassRefPtrWillBeRawPtr<File> createForFileSystemFile(const KURL& url, const FileMetadata& metadata)
@@ -140,7 +140,7 @@ private:
     File(const String& path, const String& name, ContentTypeLookupPolicy, UserVisibility);
     File(const String& path, const String& name, const String& relativePath, UserVisibility, bool hasSnaphotData, uint64_t size, double lastModified, PassRefPtr<BlobDataHandle>);
     File(const String& name, double modificationTime, PassRefPtr<BlobDataHandle>);
-    File(const String& name, const FileMetadata&);
+    File(const String& name, const FileMetadata&, UserVisibility);
     File(const KURL& fileSystemURL, const FileMetadata&);
 
     void invalidateSnapshotMetadata() { m_snapshotSize = -1; }

third_party/WebKit/Source/modules/filesystem/DOMFileSystemBase.cpp

@@ -32,6 +32,7 @@
 #include "modules/filesystem/DOMFileSystemBase.h"
 
 #include "core/dom/ExecutionContext.h"
+#include "core/fileapi/File.h"
 #include "core/fileapi/FileError.h"
 #include "core/html/VoidCallback.h"
 #include "modules/filesystem/DOMFilePath.h"
@@ -188,6 +189,25 @@ bool DOMFileSystemBase::pathPrefixToFileSystemType(const String& pathPrefix, Fil
     return false;
 }
 
+PassRefPtrWillBeRawPtr<File> DOMFileSystemBase::createFile(const FileMetadata& metadata, const KURL& fileSystemURL, FileSystemType type, const String 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).
+    // FIXME: We should use the snapshot metadata for all files.
+    // https://www.w3.org/Bugs/Public/show_bug.cgi?id=17746
+    if (type == FileSystemTypeTemporary || type == FileSystemTypePersistent)
+        return File::createForFileSystemFile(metadata.platformPath, name);
+
+    if (!metadata.platformPath.isEmpty()) {
+        // If the platformPath in the returned metadata is given, we create a File object for the path.
+        File::UserVisibility userVisibility = (type == FileSystemTypeExternal) ? File::IsUserVisible : File::IsNotUserVisible;
+        return File::createForFileSystemFile(name, metadata, userVisibility);
+    }
+
+    return File::createForFileSystemFile(fileSystemURL, metadata);
+}
+
 void DOMFileSystemBase::getMetadata(const EntryBase* entry, PassOwnPtrWillBeRawPtr<MetadataCallback> successCallback, PassOwnPtrWillBeRawPtr<ErrorCallback> errorCallback, SynchronousType synchronousType)
 {
     if (!fileSystem()) {

third_party/WebKit/Source/modules/filesystem/DOMFileSystemBase.h

@@ -49,7 +49,9 @@ class EntriesCallback;
 class EntryBase;
 class EntryCallback;
 class ErrorCallback;
+class File;
 class FileError;
+struct FileMetadata;
 class MetadataCallback;
 class ExecutionContext;
 class SecurityOrigin;
@@ -101,6 +103,7 @@ public:
     KURL createFileSystemURL(const String& fullPath) const;
     static bool pathToAbsolutePath(FileSystemType, const EntryBase*, String path, String& absolutePath);
     static bool pathPrefixToFileSystemType(const String& pathPrefix, FileSystemType&);
+    static PassRefPtrWillBeRawPtr<File> createFile(const FileMetadata&, const KURL& fileSystemURL, FileSystemType, const String name);
 
     // Actual FileSystem API implementations. All the validity checks on virtual paths are done at this level.
     void getMetadata(const EntryBase*, PassOwnPtrWillBeRawPtr<MetadataCallback>, PassOwnPtrWillBeRawPtr<ErrorCallback>, SynchronousType = Asynchronous);

third_party/WebKit/Source/modules/filesystem/DOMFileSystemBaseTest.cpp

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "config.h"
+#include "modules/filesystem/DOMFileSystemBase.h"
+
+#include "core/fileapi/File.h"
+#include "public/platform/Platform.h"
+#include "public/platform/WebUnitTestSupport.h"
+#include <gtest/gtest.h>
+
+
+namespace blink {
+
+class DOMFileSystemBaseTest : public ::testing::Test {
+public:
+    DOMFileSystemBaseTest()
+    {
+        m_filePath = Platform::current()->unitTestSupport()->webKitRootDir();
+        m_filePath.append("/Source/modules/filesystem/DOMFileSystemBaseTest.cpp");
+        getFileMetadata(m_filePath, m_fileMetadata);
+        m_fileMetadata.platformPath = m_filePath;
+    }
+
+protected:
+    String m_filePath;
+    FileMetadata m_fileMetadata;
+};
+
+
+TEST_F(DOMFileSystemBaseTest, externalFilesystemFilesAreUserVisible)
+{
+    KURL rootUrl = DOMFileSystemBase::createFileSystemRootURL("http://chromium.org/", FileSystemTypeExternal);
+
+    RefPtrWillBeRawPtr<File> file = DOMFileSystemBase::createFile(m_fileMetadata, rootUrl, FileSystemTypeExternal, "DOMFileSystemBaseTest.cpp");
+    EXPECT_TRUE(file);
+    EXPECT_TRUE(file->hasBackingFile());
+    EXPECT_EQ(File::IsUserVisible, file->userVisibility());
+    EXPECT_EQ("DOMFileSystemBaseTest.cpp", file->name());
+    EXPECT_EQ(m_filePath, file->path());
+}
+
+TEST_F(DOMFileSystemBaseTest, temporaryFilesystemFilesAreNotUserVisible)
+{
+    KURL rootUrl = DOMFileSystemBase::createFileSystemRootURL("http://chromium.org/", FileSystemTypeTemporary);
+
+    RefPtrWillBeRawPtr<File> file = DOMFileSystemBase::createFile(m_fileMetadata, rootUrl, FileSystemTypeTemporary, "UserVisibleName.txt");
+    EXPECT_TRUE(file);
+    EXPECT_TRUE(file->hasBackingFile());
+    EXPECT_EQ(File::IsNotUserVisible, file->userVisibility());
+    EXPECT_EQ("UserVisibleName.txt", file->name());
+    EXPECT_EQ(m_filePath, file->path());
+}
+
+TEST_F(DOMFileSystemBaseTest, persistentFilesystemFilesAreNotUserVisible)
+{
+    KURL rootUrl = DOMFileSystemBase::createFileSystemRootURL("http://chromium.org/", FileSystemTypePersistent);
+
+    RefPtrWillBeRawPtr<File> file = DOMFileSystemBase::createFile(m_fileMetadata, rootUrl, FileSystemTypePersistent, "UserVisibleName.txt");
+    EXPECT_TRUE(file);
+    EXPECT_TRUE(file->hasBackingFile());
+    EXPECT_EQ(File::IsNotUserVisible, file->userVisibility());
+    EXPECT_EQ("UserVisibleName.txt", file->name());
+    EXPECT_EQ(m_filePath, file->path());
+}
+
+} // namespace blink
+
+
+

third_party/WebKit/Source/modules/filesystem/DOMFileSystemSync.cpp

@@ -124,20 +124,7 @@ public:
         // *after* we've coined a File with a new handle that has the correct type set on it. This allows the
         // blob storage system to track when a temp file can and can't be safely deleted.
 
-        // 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).
-        // FIXME: We should use the snapshot metadata for all files.
-        // https://www.w3.org/Bugs/Public/show_bug.cgi?id=17746
-        if (m_type == FileSystemTypeTemporary || m_type == FileSystemTypePersistent) {
-            m_result->m_file = File::createForFileSystemFile(metadata.platformPath, m_name);
-        } else if (!metadata.platformPath.isEmpty()) {
-            // If the platformPath in the returned metadata is given, we create a File object for the path.
-            m_result->m_file = File::createForFileSystemFile(m_name, metadata).get();
-        } else {
-            // Otherwise create a File from the FileSystem URL.
-            m_result->m_file = File::createForFileSystemFile(m_url, metadata).get();
-        }
+        m_result->m_file = DOMFileSystemBase::createFile(metadata, m_url, m_type, m_name);
     }
 
     virtual bool shouldBlockUntilCompletion() const OVERRIDE

third_party/WebKit/Source/modules/filesystem/FileSystemCallbacks.cpp

@@ -312,19 +312,7 @@ void SnapshotFileCallback::didCreateSnapshotFile(const FileMetadata& metadata, P
     // *after* we've coined a File with a new handle that has the correct type set on it. This allows the
     // blob storage system to track when a temp file can and can't be safely deleted.
 
-    // 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).
-    // FIXME: We should use the snapshot metadata for all files.
-    // https://www.w3.org/Bugs/Public/show_bug.cgi?id=17746
-    if (m_fileSystem->type() == FileSystemTypeTemporary || m_fileSystem->type() == FileSystemTypePersistent) {
-        handleEventOrScheduleCallback(m_successCallback.release(), File::createForFileSystemFile(metadata.platformPath, m_name));
-    } else if (!metadata.platformPath.isEmpty()) {
-        // If the platformPath in the returned metadata is given, we create a File object for the path.
-        handleEventOrScheduleCallback(m_successCallback.release(), File::createForFileSystemFile(m_name, metadata));
-    } else {
-        // Otherwise create a File from the FileSystem URL.
-        handleEventOrScheduleCallback(m_successCallback.release(), File::createForFileSystemFile(m_url, metadata));
-    }
+    handleEventOrScheduleCallback(m_successCallback.release(), DOMFileSystemBase::createFile(metadata, m_url, m_fileSystem->type(), m_name));
 }
 
 // VoidCallbacks --------------------------------------------------------------

third_party/WebKit/Source/modules/modules.gypi

@@ -1021,6 +1021,7 @@
       'vibration/testing/InternalsVibration.h',
     ],
     'modules_unittest_files': [
+      'filesystem/DOMFileSystemBaseTest.cpp',
       'indexeddb/IDBKeyPathTest.cpp',
       'indexeddb/IDBRequestTest.cpp',
       'indexeddb/IDBTransactionTest.cpp',