Skip to content

Commit

Permalink
Remove unused kFileSystemTypeRestrictedLocal
Browse files Browse the repository at this point in the history
This type was previously used for ChromeOS read-only /usr/share/oem
mount, but is no longer used.

Bug: 152216
Bug: 271946
Bug: b/293534917
Change-Id: I4a1f49e3db18c70ec440cd62ecb3cd400249c8d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4729028
Reviewed-by: Josh Simmons <simmonsjosh@google.com>
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Giovanni Ortuno Urquidi <ortuno@chromium.org>
Reviewed-by: Austin Sullivan <asully@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1177556}
  • Loading branch information
Joel Hockey authored and Chromium LUCI CQ committed Aug 1, 2023
1 parent b3cb1d1 commit a9e2d2e
Show file tree
Hide file tree
Showing 16 changed files with 11 additions and 89 deletions.
2 changes: 1 addition & 1 deletion ash/public/cpp/holding_space/holding_space_file.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ struct ASH_PUBLIC_EXPORT HoldingSpaceFile {
kExternal = 4,
kTest = 5,
kLocal = 6,
kRestrictedLocal = 7,
// kRestrictedLocal = 7,
kDragged = 8,
kLocalMedia = 9,
kDeviceMedia = 10,
Expand Down
49 changes: 2 additions & 47 deletions chrome/browser/ash/file_manager/external_filesystem_apitest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,6 @@
// The tests cover following external file system types:
// - local (kFileSystemTypeLocalNative): a local file system on which files are
// accessed using native local path.
// - restricted (kFileSystemTypeRestrictedLocalNative): a *read-only* local file
// system which can only be accessed by extensions that have full access to
// external file systems (i.e. extensions with fileManagerPrivate permission).
//
// The tests cover following scenarios:
// - Performing file system operations on external file systems from an
Expand All @@ -81,7 +78,6 @@ namespace {
// but the test will have to make sure the mount point is added before
// starting a test extension using WaitUntilDriveMountPointIsAdded().
constexpr char kLocalMountPointName[] = "local";
constexpr char kRestrictedMountPointName[] = "restricted";

// Default file content for the test files.
constexpr char kTestFileContent[] = "This is some test content.";
Expand Down Expand Up @@ -174,8 +170,8 @@ constexpr const TestDirConfig kDefaultDirConfig[] = {
""},
};

// Sets up the initial file system state for native local and restricted native
// local file systems. The hierarchy is the same as for the drive file system.
// Sets up the initial file system state for native local file systems. The
// hierarchy is the same as for the drive file system.
// The directory is created at unique_temp_dir/|mount_point_name| path.
bool InitializeLocalFileSystem(std::string mount_point_name,
base::ScopedTempDir* tmp_dir,
Expand Down Expand Up @@ -388,36 +384,6 @@ class LocalFileSystemExtensionApiTest : public FileSystemExtensionApiTestBase {
base::FilePath mount_point_dir_;
};

// Tests for restricted native local file systems.
class RestrictedFileSystemExtensionApiTest
: public FileSystemExtensionApiTestBase {
public:
RestrictedFileSystemExtensionApiTest() = default;
~RestrictedFileSystemExtensionApiTest() override = default;

// FileSystemExtensionApiTestBase override.
void InitTestFileSystem() override {
ASSERT_TRUE(InitializeLocalFileSystem(kRestrictedMountPointName, &tmp_dir_,
&mount_point_dir_,
GetTestDirContents()))
<< "Failed to initialize file system.";
}

// FileSystemExtensionApiTestBase override.
void AddTestMountPoint() override {
EXPECT_TRUE(profile()->GetMountPoints()->RegisterFileSystem(
kRestrictedMountPointName, storage::kFileSystemTypeRestrictedLocal,
storage::FileSystemMountOption(), mount_point_dir_));
VolumeManager::Get(profile())->AddVolumeForTesting(
mount_point_dir_, VOLUME_TYPE_TESTING, ash::DeviceType::kUnknown,
true /* read_only */);
}

private:
base::ScopedTempDir tmp_dir_;
base::FilePath mount_point_dir_;
};

// Tests for a drive file system.
class DriveFileSystemExtensionApiTest : public FileSystemExtensionApiTestBase {
public:
Expand Down Expand Up @@ -749,17 +715,6 @@ IN_PROC_BROWSER_TEST_F(LocalFileSystemExtensionApiTest, DefaultFileHandler) {
<< message_;
}

//
// RestrictedFileSystemExtensionApiTests.
//
IN_PROC_BROWSER_TEST_F(RestrictedFileSystemExtensionApiTest,
FileSystemOperations) {
EXPECT_TRUE(RunFileSystemExtensionApiTest(
"file_browser/filesystem_operations_test",
FILE_PATH_LITERAL("manifest.json"), "", FLAGS_NONE))
<< message_;
}

//
// DriveFileSystemExtensionApiTests.
//
Expand Down
4 changes: 1 addition & 3 deletions chrome/browser/ash/file_manager/file_browser_handlers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,7 @@ FileBrowserHandlerExecutor::SetupFileAccessPermissions(
base::FilePath local_path = url.path();
base::FilePath virtual_path = url.virtual_path();

const bool is_native_file =
url.type() == storage::kFileSystemTypeLocal ||
url.type() == storage::kFileSystemTypeRestrictedLocal;
const bool is_native_file = url.type() == storage::kFileSystemTypeLocal;

// If the file is from a physical volume, actual file must be found.
if (is_native_file) {
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/ash/file_manager/file_tasks_notifier.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ namespace {
bool IsSupportedFileSystemType(storage::FileSystemType type) {
switch (type) {
case storage::kFileSystemTypeLocal:
case storage::kFileSystemTypeRestrictedLocal:
case storage::kFileSystemTypeDriveFs:
return true;
default:
Expand Down
8 changes: 0 additions & 8 deletions chrome/browser/ash/fileapi/file_system_backend.cc
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ bool FileSystemBackend::CanHandleURL(const storage::FileSystemURL& url) {
if (!url.is_valid())
return false;
return url.type() == storage::kFileSystemTypeLocal ||
url.type() == storage::kFileSystemTypeRestrictedLocal ||
url.type() == storage::kFileSystemTypeProvided ||
url.type() == storage::kFileSystemTypeDeviceMediaAsFileStorage ||
url.type() == storage::kFileSystemTypeArcContent ||
Expand Down Expand Up @@ -111,7 +110,6 @@ void FileSystemBackend::AddSystemMountPoints() {
bool FileSystemBackend::CanHandleType(storage::FileSystemType type) const {
switch (type) {
case storage::kFileSystemTypeExternal:
case storage::kFileSystemTypeRestrictedLocal:
case storage::kFileSystemTypeLocal:
case storage::kFileSystemTypeLocalForPlatformApp:
case storage::kFileSystemTypeDeviceMediaAsFileStorage:
Expand Down Expand Up @@ -283,7 +281,6 @@ storage::AsyncFileUtil* FileSystemBackend::GetAsyncFileUtil(
case storage::kFileSystemTypeProvided:
return file_system_provider_delegate_->GetAsyncFileUtil(type);
case storage::kFileSystemTypeLocal:
case storage::kFileSystemTypeRestrictedLocal:
case storage::kFileSystemTypeFuseBox:
return local_file_util_.get();
case storage::kFileSystemTypeDeviceMediaAsFileStorage:
Expand Down Expand Up @@ -347,7 +344,6 @@ FileSystemBackend::CreateFileSystemOperation(
context, MediaFileSystemBackend::MediaTaskRunner().get()));
}
if (url.type() == storage::kFileSystemTypeLocal ||
url.type() == storage::kFileSystemTypeRestrictedLocal ||
url.type() == storage::kFileSystemTypeDriveFs ||
url.type() == storage::kFileSystemTypeSmbFs ||
url.type() == storage::kFileSystemTypeFuseBox) {
Expand Down Expand Up @@ -386,7 +382,6 @@ bool FileSystemBackend::HasInplaceCopyImplementation(
// crbug.com/953603.
case storage::kFileSystemTypeArcDocumentsProvider:
case storage::kFileSystemTypeLocal:
case storage::kFileSystemTypeRestrictedLocal:
case storage::kFileSystemTypeArcContent:
// TODO(crbug.com/939235): Implement in-place copy in SmbFs.
case storage::kFileSystemTypeSmbFs:
Expand Down Expand Up @@ -418,7 +413,6 @@ FileSystemBackend::CreateFileStreamReader(
url, offset, max_bytes_to_read, expected_modification_time, context);
// The dlp file_access callback is needed for the local filesystem only.
case storage::kFileSystemTypeLocal:
case storage::kFileSystemTypeRestrictedLocal:
return storage::FileStreamReader::CreateForLocalFile(
base::ThreadPool::CreateTaskRunner(
{base::MayBlock(), base::TaskPriority::USER_VISIBLE})
Expand Down Expand Up @@ -477,7 +471,6 @@ FileSystemBackend::CreateFileStreamWriter(
return arc_documents_provider_delegate_->CreateFileStreamWriter(
url, offset, context);
// Read only file systems.
case storage::kFileSystemTypeRestrictedLocal:
case storage::kFileSystemTypeArcContent:
return nullptr;
default:
Expand Down Expand Up @@ -511,7 +504,6 @@ void FileSystemBackend::GetRedirectURLForContents(
mtp_delegate_->GetRedirectURLForContents(url, std::move(callback));
return;
case storage::kFileSystemTypeLocal:
case storage::kFileSystemTypeRestrictedLocal:
case storage::kFileSystemTypeArcContent:
case storage::kFileSystemTypeArcDocumentsProvider:
case storage::kFileSystemTypeDriveFs:
Expand Down
13 changes: 3 additions & 10 deletions chrome/browser/ash/fileapi/file_system_backend.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,7 @@ constexpr char kSystemMountNameRemovable[] = "removable";
// - Create FileSystemOperation per file system type
// - Create FileStreamReader/Writer per file system type
//
// Chrome OS specific mount points:
//
// "Downloads" is a mount point for user's Downloads directory on the local
// disk, where downloaded files are stored by default.
// Chrome OS specific static mount points:
//
// "archive" is a mount point for an archive file, such as a zip file. This
// mount point exposes contents of an archive file via cros_disks and AVFS
Expand All @@ -59,17 +56,13 @@ constexpr char kSystemMountNameRemovable[] = "removable";
// "removable" is a mount point for removable media such as an SD card.
// Insertion and removal of removable media are handled by cros_disks.
//
// "oem" is a read-only mount point for a directory containing OEM data.
//
// "drive" is a mount point for Google Drive. Drive is integrated with the
// FileSystem API layer via drive::FileSystemProxy. This mount point is added
// by drive::DriveIntegrationService.
//
// These mount points are placed under the "external" namespace, and file
// system URLs for these mount points look like:
//
// filesystem:<origin>/external/<mount_name>/...
//
// Other mounts are also registered by VolumeManager for MyFiles, Drive, VMs
// (crostini, arc, etc), Android Document Providers, fileSystemProviders, etc.
class FileSystemBackend : public storage::ExternalFileSystemBackend {
public:
using storage::FileSystemBackend::ResolveURLCallback;
Expand Down
3 changes: 0 additions & 3 deletions chrome/browser/ash/fileapi/file_system_backend_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,6 @@ TEST(ChromeOSFileSystemBackendTest, AccessPermissions) {
"removable", storage::kFileSystemTypeLocal,
storage::FileSystemMountOption(),
base::FilePath(FPL("/media/removable"))));
ASSERT_TRUE(mount_points->RegisterFileSystem(
"oem", storage::kFileSystemTypeRestrictedLocal,
storage::FileSystemMountOption(), base::FilePath(FPL("/usr/share/oem"))));

// Backend specific mount point access.
EXPECT_FALSE(backend.IsAccessAllowed(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1655,7 +1655,6 @@ ExtensionFunction::ResponseAction DeveloperPrivateLoadDirectoryFunction::Run() {

if (directory_url.is_valid() &&
directory_url.type() != storage::kFileSystemTypeLocal &&
directory_url.type() != storage::kFileSystemTypeRestrictedLocal &&
directory_url.type() != storage::kFileSystemTypeDragged) {
return LoadByFileSystemAPI(directory_url);
}
Expand Down
2 changes: 0 additions & 2 deletions chrome/browser/ui/ash/holding_space/holding_space_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,6 @@ HoldingSpaceFile::FileSystemType ToHoldingSpaceFileSystemType(
return HoldingSpaceFile::FileSystemType::kPersistent;
case storage::FileSystemType::kFileSystemTypeProvided:
return HoldingSpaceFile::FileSystemType::kProvided;
case storage::FileSystemType::kFileSystemTypeRestrictedLocal:
return HoldingSpaceFile::FileSystemType::kRestrictedLocal;
case storage::FileSystemType::kFileSystemTypeSmbFs:
return HoldingSpaceFile::FileSystemType::kSmbFs;
case storage::FileSystemType::kFileSystemTypeSyncable:
Expand Down
1 change: 0 additions & 1 deletion content/browser/file_system/file_system_manager_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ blink::mojom::FileSystemType ToMojoFileSystemType(
case storage::FileSystemType::kFileSystemInternalTypeEnumStart:
case storage::FileSystemType::kFileSystemTypeTest:
case storage::FileSystemType::kFileSystemTypeLocal:
case storage::FileSystemType::kFileSystemTypeRestrictedLocal:
case storage::FileSystemType::kFileSystemTypeDragged:
case storage::FileSystemType::kFileSystemTypeLocalMedia:
case storage::FileSystemType::kFileSystemTypeDeviceMedia:
Expand Down
2 changes: 0 additions & 2 deletions storage/browser/file_system/external_mount_points_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,6 @@ TEST(ExternalMountPointsTest, HandlesFileSystemMountType) {
EXPECT_FALSE(mount_points->HandlesFileSystemMountType(kFileSystemTypeTest));
// Not even if it's external subtype.
EXPECT_FALSE(mount_points->HandlesFileSystemMountType(kFileSystemTypeLocal));
EXPECT_FALSE(
mount_points->HandlesFileSystemMountType(kFileSystemTypeRestrictedLocal));
EXPECT_FALSE(
mount_points->HandlesFileSystemMountType(kFileSystemTypeDriveFs));
EXPECT_FALSE(
Expand Down
3 changes: 0 additions & 3 deletions storage/browser/file_system/file_system_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -113,9 +113,6 @@ int FileSystemContext::GetPermissionPolicy(FileSystemType type) {
case kFileSystemTypeFuseBox:
return FILE_PERMISSION_USE_FILE_PERMISSION;

case kFileSystemTypeRestrictedLocal:
return FILE_PERMISSION_READ_ONLY | FILE_PERMISSION_USE_FILE_PERMISSION;

case kFileSystemTypeDeviceMedia:
case kFileSystemTypeLocalMedia:
return FILE_PERMISSION_USE_FILE_PERMISSION;
Expand Down
5 changes: 2 additions & 3 deletions storage/browser/file_system/file_system_context_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,7 @@ TEST_F(FileSystemContextTest, CrackFileSystemURL) {
// Register a system external mount point with the same name/id as the
// registered isolated mount point.
ASSERT_TRUE(ExternalMountPoints::GetSystemInstance()->RegisterFileSystem(
kIsolatedFileSystemID, kFileSystemTypeRestrictedLocal,
FileSystemMountOption(),
kIsolatedFileSystemID, kFileSystemTypeLocal, FileSystemMountOption(),
base::FilePath(DRIVE FPL("/test/system/isolated"))));
// Add a mount points with the same name as a system mount point to
// FileSystemContext's external mount points.
Expand Down Expand Up @@ -300,7 +299,7 @@ TEST_F(FileSystemContextTest, CrackFileSystemURL) {
{"system", "external", true /* is_valid */, kFileSystemTypeExternal,
kFileSystemTypeLocal, DRIVE FPL("/test/sys/root/file"), "system"},
{kIsolatedFileSystemID, "external", true /* is_valid */,
kFileSystemTypeExternal, kFileSystemTypeRestrictedLocal,
kFileSystemTypeExternal, kFileSystemTypeLocal,
DRIVE FPL("/test/system/isolated/root/file"), kIsolatedFileSystemID},
// Should be cracked by FileSystemContext's ExternalMountPoints.
{"ext", "external", true /* is_valid */, kFileSystemTypeExternal,
Expand Down
1 change: 0 additions & 1 deletion storage/browser/file_system/file_system_url.cc
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ bool FileSystemURL::TypeImpliesPathIsReal(FileSystemType type) {
break;

case kFileSystemTypeLocal:
case kFileSystemTypeRestrictedLocal:
case kFileSystemTypeLocalMedia:
case kFileSystemTypeLocalForPlatformApp:
case kFileSystemTypeDriveFs:
Expand Down
3 changes: 2 additions & 1 deletion storage/common/file_system/file_system_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ enum FileSystemType {
// Indicates a local filesystem where we can access files using local path,
// but with restricted access.
// Restricted local file system is in read-only mode.
kFileSystemTypeRestrictedLocal = 102,
// Previously used for ChromeOS /usr/share/oem, now obsolete.
// kFileSystemTypeRestrictedLocal = 102,

// Indicates a transient, isolated file system for dragged files (which could
// contain multiple dragged paths in the virtual root).
Expand Down
2 changes: 0 additions & 2 deletions storage/common/file_system/file_system_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,6 @@ std::string GetFileSystemTypeString(FileSystemType type) {
return "Test";
case kFileSystemTypeLocal:
return "Local";
case kFileSystemTypeRestrictedLocal:
return "RestrictedLocal";
case kFileSystemTypeDragged:
return "Dragged";
case kFileSystemTypeLocalMedia:
Expand Down

0 comments on commit a9e2d2e

Please sign in to comment.