Skip to content

Commit

Permalink
Remove third_party/xdg_shared_mime_type
Browse files Browse the repository at this point in the history
Use base::nix::GetFileMimeType() instead which will read the local copy
of /usr/local/share/mime/mime.cache.

Partial revert of crrev.com/c/3848953.

net::GetMimeTypeFromFile() will now block for ChromeOS, so changed
some callers to use net::GetWellKnownMimeTypeFromExtension() which
does not block and is sufficient for what is required by the caller.

Bug: 1015353
Bug: b/300466236
Change-Id: I1094906181ccd0f22ce4c65c6e2a360df9dc1496
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4974772
Reviewed-by: Timothy Loh <timloh@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Reviewed-by: Ahmed Mehfooz <amehfooz@chromium.org>
Reviewed-by: David Black <dmblack@google.com>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1217298}
  • Loading branch information
Joel Hockey authored and Chromium LUCI CQ committed Oct 30, 2023
1 parent 9e4941a commit ac1c45a
Show file tree
Hide file tree
Showing 24 changed files with 93 additions and 9,728 deletions.
5 changes: 3 additions & 2 deletions ash/system/holding_space/holding_space_view_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -651,10 +651,11 @@ ui::SimpleMenuModel* HoldingSpaceViewDelegate::BuildMenuModel() {
.label_id = IDS_ASH_HOLDING_SPACE_CONTEXT_MENU_SHOW_IN_FOLDER,
.icon = raw_ref(kFolderIcon)});

std::string ext = selection.front()->item()->file().file_path.Extension();
std::string mime_type;
const bool is_image =
net::GetMimeTypeFromFile(selection.front()->item()->file().file_path,
&mime_type) &&
!ext.empty() &&
net::GetWellKnownMimeTypeFromExtension(ext.substr(1), &mime_type) &&
net::MatchesMimeType(kMimeTypeImage, mime_type);

if (is_image) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,19 @@ class GuestOSAppsTest : public testing::Test {
return intent_filters;
}

void UpdateMimeTypes(std::string container_name,
std::string extension,
std::string mime_type) {
vm_tools::apps::MimeTypes mime_types_list;
mime_types_list.set_vm_name(bruschetta::kBruschettaVmName);
mime_types_list.set_container_name(container_name);
(*mime_types_list.mutable_mime_type_mappings())[extension] = mime_type;
auto* mime_types_service =
guest_os::GuestOsMimeTypesServiceFactory::GetForProfile(profile());
mime_types_service->UpdateMimeTypes(mime_types_list);
task_environment_.RunUntilIdle();
}

private:
content::BrowserTaskEnvironment task_environment_;
std::unique_ptr<TestingProfile> profile_;
Expand Down Expand Up @@ -235,15 +248,7 @@ TEST_F(GuestOSAppsTest, IntentFilterWithTextPlainAddsTextWildcardMimeType) {
TEST_F(GuestOSAppsTest, IntentFilterHasExtensionsFromPrefs) {
const std::string mime_type = "test/mime1";
const std::string extension = "test_extension";

// Update the mime_types_service to map the extension to the mime type.
vm_tools::apps::MimeTypes mime_types_list;
mime_types_list.set_vm_name(bruschetta::kBruschettaVmName);
mime_types_list.set_container_name("test_container");
(*mime_types_list.mutable_mime_type_mappings())[extension] = mime_type;
auto* mime_types_service =
guest_os::GuestOsMimeTypesServiceFactory::GetForProfile(profile());
mime_types_service->UpdateMimeTypes(mime_types_list);
UpdateMimeTypes("test_container", extension, mime_type);

// Create app and get its registered intent filters.
const std::string app_id =
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/ash/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -4239,7 +4239,6 @@ source_set("ash") {
"//third_party/protobuf:protobuf_lite",
"//third_party/re2",
"//third_party/securemessage/proto",
"//third_party/xdg_shared_mime_info",
"//third_party/zlib",
"//third_party/zlib/google:compression_utils",
"//third_party/zlib/google:zip",
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/ash/file_manager/file_manager_test_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ base::WeakPtr<file_manager::Volume> InstallFileSystemProviderChromeApp(
std::vector<file_tasks::FullTaskDescriptor> GetTasksForFile(
Profile* profile,
const base::FilePath& file) {
base::ScopedAllowBlockingForTesting allow_blocking;
std::string mime_type;
net::GetMimeTypeFromFile(file, &mime_type);
CHECK(!mime_type.empty());
Expand Down
19 changes: 10 additions & 9 deletions chrome/browser/ash/file_manager/file_tasks_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "base/test/bind.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/test_future.h"
#include "base/threading/thread_restrictions.h"
#include "build/branding_buildflags.h"
#include "chrome/browser/apps/app_service/app_launch_params.h"
#include "chrome/browser/apps/app_service/app_service_proxy.h"
Expand Down Expand Up @@ -184,12 +185,9 @@ void ConvertExpectation(const Expectation& test,
for (base::StringPiece extension : all_extensions) {
base::FilePath path = prefix.AddExtension(extension);
std::string mime_type;
base::ScopedAllowBlockingForTesting allow_blocking;
net::GetMimeTypeFromFile(path, &mime_type);
if (test.mime_type != nullptr) {
// Sniffing isn't used when GetMimeTypeFromFile() succeeds, so there
// shouldn't be a hard-coded mime type configured.
EXPECT_TRUE(mime_type.empty())
<< "Did not expect mime match " << mime_type << " for " << path;
mime_type = test.mime_type;
} else {
EXPECT_FALSE(mime_type.empty()) << "No mime type for " << path;
Expand Down Expand Up @@ -268,7 +266,7 @@ IN_PROC_BROWSER_TEST_P(FileTasksBrowserTest, ExtensionToMimeMapping) {
{"cr2"},
{"dng"},
{"nef"},
{"nrw"},
{"nrw", false},
{"orf"},
{"raf"},
{"rw2"},
Expand Down Expand Up @@ -305,8 +303,11 @@ IN_PROC_BROWSER_TEST_P(FileTasksBrowserTest, ExtensionToMimeMapping) {
for (const auto& test : kExpectations) {
base::FilePath path = prefix.AddExtension(test.file_extension);

EXPECT_EQ(test.has_mime, net::GetMimeTypeFromFile(path, &mime_type))
<< test.file_extension;
base::ScopedAllowBlockingForTesting allow_blocking;
if (test.has_mime) {
EXPECT_TRUE(net::GetMimeTypeFromFile(path, &mime_type))
<< test.file_extension;
}
}
}

Expand Down Expand Up @@ -334,11 +335,11 @@ IN_PROC_BROWSER_TEST_P(FileTasksBrowserTest, ImageHandlerChangeDetector) {
{"cr2", kMediaAppId},
{"dng", kMediaAppId},
{"nef", kMediaAppId},
{"nrw", kMediaAppId},
{"nrw", kMediaAppId, "image/tiff"},
{"orf", kMediaAppId},
{"raf", kMediaAppId},
{"rw2", kMediaAppId},
{"NRW", kMediaAppId}, // Uppercase extension.
{"NRW", kMediaAppId, "image/tiff"}, // Uppercase extension.
};
TestExpectationsAgainstDefaultTasks(expectations);
}
Expand Down
52 changes: 36 additions & 16 deletions chrome/browser/ash/guest_os/guest_os_mime_types_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,34 @@

#include "base/containers/contains.h"
#include "base/logging.h"
#include "base/nix/mime_util_xdg.h"
#include "base/strings/string_util.h"
#include "base/task/thread_pool.h"
#include "chrome/browser/ash/guest_os/guest_os_pref_names.h"
#include "chrome/browser/profiles/profile.h"
#include "chromeos/ash/components/dbus/vm_applications/apps.pb.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h"
#include "components/prefs/scoped_user_pref_update.h"
#include "third_party/xdg_shared_mime_info/mime_cache.h"

namespace guest_os {
namespace {

base::Value::Dict GetOverrideMimeTypes(
const vm_tools::apps::MimeTypes& mime_type_mappings) {
base::Value::Dict overrides;
for (const auto& mapping : mime_type_mappings.mime_type_mappings()) {
// Only store mappings from container that are different to host.
base::FilePath dummy_path("foo." + mapping.first);
std::string type = base::nix::GetFileMimeType(dummy_path);
if (mapping.second != type) {
overrides.Set(mapping.first, mapping.second);
}
}
return overrides;
}

} // namespace

GuestOsMimeTypesService::GuestOsMimeTypesService(Profile* profile)
: prefs_(profile->GetPrefs()) {}
Expand Down Expand Up @@ -116,22 +134,24 @@ void GuestOsMimeTypesService::UpdateMimeTypes(
return;
}

base::Value::Dict exts;
for (const auto& mapping : mime_type_mappings.mime_type_mappings()) {
// Only store mappings from container that are different to host.
std::string type;
if (!xdg_shared_mime_info::GetMimeCacheTypeFromExtension(mapping.first,
&type) ||
mapping.second != type) {
exts.Set(mapping.first, mapping.second);
}
}
VLOG(1) << "UpdateMimeTypes(" << mime_type_mappings.vm_name() << ", "
<< mime_type_mappings.container_name() << ")=" << exts;
base::ThreadPool::PostTaskAndReplyWithResult(
FROM_HERE, {base::TaskPriority::BEST_EFFORT, base::MayBlock()},
base::BindOnce(&GetOverrideMimeTypes, mime_type_mappings),
base::BindOnce(&GuestOsMimeTypesService::UpdateOverrideMimeTypes,
weak_ptr_factory_.GetWeakPtr(),
mime_type_mappings.vm_name(),
mime_type_mappings.container_name()));
}

void GuestOsMimeTypesService::UpdateOverrideMimeTypes(
std::string vm_name,
std::string container_name,
base::Value::Dict overrides) {
VLOG(1) << "UpdateMimeTypes(" << vm_name << ", " << container_name
<< ")=" << overrides;
ScopedDictPrefUpdate update(prefs_, prefs::kGuestOsMimeTypes);
base::Value::Dict& mime_types = update.Get();
base::Value::Dict* vm = mime_types.EnsureDict(mime_type_mappings.vm_name());
vm->Set(mime_type_mappings.container_name(), base::Value(std::move(exts)));
base::Value::Dict* vm = update.Get().EnsureDict(vm_name);
vm->Set(container_name, base::Value(std::move(overrides)));
}

} // namespace guest_os
8 changes: 8 additions & 0 deletions chrome/browser/ash/guest_os/guest_os_mime_types_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

#include "base/files/file_path.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/weak_ptr.h"
#include "base/values.h"
#include "components/keyed_service/core/keyed_service.h"

class Profile;
Expand Down Expand Up @@ -59,8 +61,14 @@ class GuestOsMimeTypesService : public KeyedService {
void UpdateMimeTypes(const vm_tools::apps::MimeTypes& mime_type_mappings);

private:
void UpdateOverrideMimeTypes(std::string vm_name,
std::string container_name,
base::Value::Dict overrides);

// Owned by the Profile.
const raw_ptr<PrefService, ExperimentalAsh> prefs_;

base::WeakPtrFactory<GuestOsMimeTypesService> weak_ptr_factory_{this};
};

} // namespace guest_os
Expand Down
39 changes: 15 additions & 24 deletions chrome/browser/ash/guest_os/guest_os_mime_types_service_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,10 @@ class GuestOsMimeTypesServiceTest : public testing::Test {

Profile* profile() { return &profile_; }

MimeTypes CreateMimeTypesProto(
const std::vector<std::string>& file_extensions,
const std::vector<std::string>& mime_types,
const std::string& vm_name,
const std::string& container_name) {
void UpdateMimeTypes(const std::vector<std::string>& file_extensions,
const std::vector<std::string>& mime_types,
const std::string& vm_name,
const std::string& container_name) {
CHECK_EQ(file_extensions.size(), mime_types.size());
MimeTypes mime_types_list;
mime_types_list.set_vm_name(vm_name);
Expand All @@ -54,7 +53,8 @@ class GuestOsMimeTypesServiceTest : public testing::Test {
(*mime_types_list.mutable_mime_type_mappings())[file_extensions[i]] =
mime_types[i];
}
return mime_types_list;
service_->UpdateMimeTypes(mime_types_list);
task_environment_.RunUntilIdle();
}

std::string GetMimeType(const std::string& filename) {
Expand Down Expand Up @@ -87,8 +87,7 @@ TEST_F(GuestOsMimeTypesServiceTest, SetAndGetMimeTypes) {
// Mime types not registered yet.
EXPECT_EQ("", GetMimeType("test.foo"));

service()->UpdateMimeTypes(CreateMimeTypesProto(
file_extensions, mime_types, kTestVmName, kTestContainerName));
UpdateMimeTypes(file_extensions, mime_types, kTestVmName, kTestContainerName);

EXPECT_EQ("x/foo", GetMimeType("test.foo"));
EXPECT_EQ("x/bar", GetMimeType("test.bar"));
Expand All @@ -111,12 +110,9 @@ TEST_F(GuestOsMimeTypesServiceTest, SetAndGetMimeTypes) {
// Test that UpdateMimeTypes doesn't clobber MIME types from different VMs or
// containers.
TEST_F(GuestOsMimeTypesServiceTest, MultipleContainers) {
service()->UpdateMimeTypes(
CreateMimeTypesProto({"foo"}, {"foo/mime"}, "vm 1", "container 1"));
service()->UpdateMimeTypes(
CreateMimeTypesProto({"bar"}, {"bar/mime"}, "vm 1", "container 2"));
service()->UpdateMimeTypes(
CreateMimeTypesProto({"foobar"}, {"foobar/mime"}, "vm 2", "container 1"));
UpdateMimeTypes({"foo"}, {"foo/mime"}, "vm 1", "container 1");
UpdateMimeTypes({"bar"}, {"bar/mime"}, "vm 1", "container 2");
UpdateMimeTypes({"foobar"}, {"foobar/mime"}, "vm 2", "container 1");

EXPECT_EQ("foo/mime", service()->GetMimeType(base::FilePath("test.foo"),
"vm 1", "container 1"));
Expand All @@ -134,8 +130,7 @@ TEST_F(GuestOsMimeTypesServiceTest, MultipleContainers) {

// Clobber bar with bar2 and ensure the old association is gone and new one is
// there.
service()->UpdateMimeTypes(
CreateMimeTypesProto({"bar2"}, {"bar2/mime"}, "vm 1", "container 2"));
UpdateMimeTypes({"bar2"}, {"bar2/mime"}, "vm 1", "container 2");
EXPECT_EQ("bar2/mime", service()->GetMimeType(base::FilePath("test.bar2"),
"vm 1", "container 2"));
EXPECT_EQ("", service()->GetMimeType(base::FilePath("test.bar"), "vm 1",
Expand All @@ -145,12 +140,9 @@ TEST_F(GuestOsMimeTypesServiceTest, MultipleContainers) {
// Test that ClearMimeTypes works, and only removes apps from the
// specified VM.
TEST_F(GuestOsMimeTypesServiceTest, ClearMimeTypes) {
service()->UpdateMimeTypes(
CreateMimeTypesProto({"foo"}, {"foo/mime"}, "vm 1", "container 1"));
service()->UpdateMimeTypes(
CreateMimeTypesProto({"bar"}, {"bar/mime"}, "vm 1", "container 2"));
service()->UpdateMimeTypes(
CreateMimeTypesProto({"foobar"}, {"foobar/mime"}, "vm 2", "container 1"));
UpdateMimeTypes({"foo"}, {"foo/mime"}, "vm 1", "container 1");
UpdateMimeTypes({"bar"}, {"bar/mime"}, "vm 1", "container 2");
UpdateMimeTypes({"foobar"}, {"foobar/mime"}, "vm 2", "container 1");

EXPECT_EQ("foo/mime", service()->GetMimeType(base::FilePath("test.foo"),
"vm 1", "container 1"));
Expand Down Expand Up @@ -178,8 +170,7 @@ TEST_F(GuestOsMimeTypesServiceTest, SetMimeTypesAndGetExtensionTypes) {
std::vector<std::string> result = GetExtensionTypesFromMimeTypes({"x/foo"});
EXPECT_EQ(0u, result.size());

service()->UpdateMimeTypes(CreateMimeTypesProto(
file_extensions, mime_types, kTestVmName, kTestContainerName));
UpdateMimeTypes(file_extensions, mime_types, kTestVmName, kTestContainerName);
result = GetExtensionTypesFromMimeTypes({"x/foo"});
EXPECT_EQ(1u, result.size());
EXPECT_EQ("foo", result[0]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,10 @@ void HoldingSpaceClientImpl::CopyImageToClipboard(const HoldingSpaceItem& item,
holding_space_metrics::RecordItemAction(
{&item}, holding_space_metrics::ItemAction::kCopy);

std::string ext = item.file().file_path.Extension();
std::string mime_type;
if (item.file().file_path.empty() ||
!net::GetMimeTypeFromFile(item.file().file_path, &mime_type) ||
if (ext.empty() ||
!net::GetWellKnownMimeTypeFromExtension(ext.substr(1), &mime_type) ||
!net::MatchesMimeType(kMimeTypeImage, mime_type)) {
std::move(callback).Run(/*success=*/false);
return;
Expand Down
4 changes: 3 additions & 1 deletion chrome/browser/ui/ash/thumbnail_loader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,10 @@ bool IsSupported(const base::FilePath& file_path) {
}};

// First attempt to match based on `mime_type`.
std::string ext = file_path.Extension();
std::string mime_type;
if (net::GetMimeTypeFromFile(file_path, &mime_type)) {
if (!ext.empty() &&
net::GetWellKnownMimeTypeFromExtension(ext.substr(1), &mime_type)) {
for (const auto& file_match_pattern : kFileMatchPatterns) {
if (file_match_pattern.second &&
re2::RE2::FullMatch(mime_type, file_match_pattern.second)) {
Expand Down
4 changes: 0 additions & 4 deletions net/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -1240,10 +1240,6 @@ component("net") {
]
}

if (is_chromeos) {
deps += [ "//third_party/xdg_shared_mime_info" ]
}

if (is_mac) {
sources += [
"base/network_notification_thread_mac.cc",
Expand Down
6 changes: 0 additions & 6 deletions net/base/DEPS
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
include_rules = [
"+grit", # For generated headers
]

specific_include_rules = {
"platform_mime_util_linux\.cc": [
"+third_party/xdg_shared_mime_info",
]
}
1 change: 0 additions & 1 deletion net/base/mime_util_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ TEST(MimeUtilTest, ExtensionTest) {
"application/x-mpegurl", // Chrome's secondary mapping.
"audio/x-mpegurl", // https://crbug.com/1273061, system override for
// android-arm[64]-test and Linux. Possibly more.
"application/vnd.apple.mpegurl", // System override for ChromeOS.
"audio/mpegurl", // System override for mac.
}},
{FILE_PATH_LITERAL("csv"), {"text/csv"}},
Expand Down
8 changes: 0 additions & 8 deletions net/base/platform_mime_util_linux.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@

#if BUILDFLAG(IS_ANDROID)
#include "net/android/network_library.h"
#elif BUILDFLAG(IS_CHROMEOS)
#include "third_party/xdg_shared_mime_info/mime_cache.h"
#else
#include "base/nix/mime_util_xdg.h"
#endif
Expand All @@ -25,12 +23,6 @@ bool PlatformMimeUtil::GetPlatformMimeTypeFromExtension(
std::string* result) const {
return android::GetMimeTypeFromExtension(ext, result);
}
#elif BUILDFLAG(IS_CHROMEOS)
bool PlatformMimeUtil::GetPlatformMimeTypeFromExtension(
const base::FilePath::StringType& ext,
std::string* result) const {
return xdg_shared_mime_info::GetMimeCacheTypeFromExtension(ext, result);
}
#else
bool PlatformMimeUtil::GetPlatformMimeTypeFromExtension(
const base::FilePath::StringType& ext,
Expand Down

0 comments on commit ac1c45a

Please sign in to comment.