0

Moves the FileHandlerInfo struct into the app_service component.

This will make it possible to add support for BMO agnostic file
handling in this CL:
https://chromium-review.googlesource.com/c/chromium/src/+/1743289.

In future, we may want to tweak the shape of the FileHandlerInfo struct.
At this point we may need to introduce some extensions/BMO abstraction.
Until then, this should suit.

Bug: 938103
Change-Id: Ie99200de6a12d4b4ee06d359981dc856659c1093
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1752567
Commit-Queue: Jay Harris <harrisjay@chromium.org>
Auto-Submit: Jay Harris <harrisjay@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: Nigel Tao <nigeltao@chromium.org>
Cr-Commit-Position: refs/heads/master@{#687526}
This commit is contained in:
Jay Harris
2019-08-16 01:23:39 +00:00
committed by Commit Bot
parent eab74540f4
commit d3191c0343
22 changed files with 144 additions and 91 deletions

@ -2,6 +2,7 @@ include_rules = [
"+content/public/browser",
"+content/public/common",
"+content/public/test",
"+components/services/app_service/public",
"+components/keyed_service",
"+components/user_manager",
"+extensions",

@ -18,6 +18,7 @@
#include "base/strings/utf_string_conversions.h"
#include "base/task/post_task.h"
#include "base/task/task_traits.h"
#include "components/services/app_service/public/cpp/file_handler_info.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_task_traits.h"
#include "content/public/browser/browser_thread.h"
@ -251,7 +252,7 @@ class PlatformAppPathLauncher
return;
// Find file handler from the platform app for the file being opened.
const extensions::FileHandlerInfo* handler = NULL;
const FileHandlerInfo* handler = nullptr;
if (!handler_id_.empty()) {
handler = FileHandlerForId(*app, handler_id_);
if (handler) {
@ -260,7 +261,7 @@ class PlatformAppPathLauncher
LOG(WARNING)
<< "Extension does not provide a valid file handler for "
<< entry_paths_[i].value();
handler = NULL;
handler = nullptr;
break;
}
}

@ -3520,6 +3520,7 @@ jumbo_split_static_library("browser") {
"//components/image_fetcher/core",
"//components/keep_alive_registry",
"//components/ntp_snippets",
"//components/services/app_service/public/cpp:app_file_handling",
"//components/vector_icons",
"//components/web_modal",
"//components/zoom",

@ -189,7 +189,7 @@ include_rules = [
"+components/security_state/content",
"+components/security_state/core",
"+components/send_tab_to_self",
"+components/services/app_service/public/mojom",
"+components/services/app_service/public",
"+components/services/filesystem/public/mojom",
"+components/services/heap_profiling",
"+components/services/patch/content",

@ -44,6 +44,7 @@
#include "components/drive/drive_api_util.h"
#include "components/prefs/pref_service.h"
#include "components/prefs/scoped_user_pref_update.h"
#include "components/services/app_service/public/cpp/file_handler_info.h"
#include "extensions/browser/api/file_handlers/mime_util.h"
#include "extensions/browser/entry_info.h"
#include "extensions/browser/extension_host.h"
@ -445,9 +446,8 @@ bool ExecuteFileTask(Profile* profile,
return false;
}
bool IsGoodMatchFileHandler(
const extensions::FileHandlerInfo& file_handler_info,
const std::vector<extensions::EntryInfo>& entries) {
bool IsGoodMatchFileHandler(const apps::FileHandlerInfo& file_handler_info,
const std::vector<extensions::EntryInfo>& entries) {
if (file_handler_info.extensions.count("*") > 0 ||
file_handler_info.types.count("*") > 0 ||
file_handler_info.types.count("*/*") > 0)
@ -517,7 +517,7 @@ void FindFileHandlerTasks(Profile* profile,
// entries that corresponds to the app. If there doesn't exist such handler,
// show the first matching handler of the verb.
for (const auto& handler_match : file_handlers) {
const extensions::FileHandlerInfo* handler = handler_match.handler;
const apps::FileHandlerInfo* handler = handler_match.handler;
bool good_match = IsGoodMatchFileHandler(*handler, entries);
auto it = handlers_for_entries.find(handler->verb);
if (it == handlers_for_entries.end() ||
@ -530,7 +530,7 @@ void FindFileHandlerTasks(Profile* profile,
for (const auto& entry : handlers_for_entries) {
const extensions::FileHandlerMatch* match = entry.second.first;
const extensions::FileHandlerInfo* handler = match->handler;
const apps::FileHandlerInfo* handler = match->handler;
std::string task_id = file_tasks::MakeTaskID(
extension->id(), file_tasks::TASK_TYPE_FILE_HANDLER, handler->id);
@ -544,16 +544,16 @@ void FindFileHandlerTasks(Profile* profile,
const bool is_generic_file_handler =
!IsGoodMatchFileHandler(*handler, entries);
Verb verb;
if (handler->verb == extensions::file_handler_verbs::kAddTo) {
if (handler->verb == apps::file_handler_verbs::kAddTo) {
verb = Verb::VERB_ADD_TO;
} else if (handler->verb == extensions::file_handler_verbs::kPackWith) {
} else if (handler->verb == apps::file_handler_verbs::kPackWith) {
verb = Verb::VERB_PACK_WITH;
} else if (handler->verb == extensions::file_handler_verbs::kShareWith) {
} else if (handler->verb == apps::file_handler_verbs::kShareWith) {
verb = Verb::VERB_SHARE_WITH;
} else {
// Only kOpenWith is a valid remaining verb. Invalid verbs should fall
// back to it.
DCHECK(handler->verb == extensions::file_handler_verbs::kOpenWith);
DCHECK(handler->verb == apps::file_handler_verbs::kOpenWith);
verb = Verb::VERB_OPEN_WITH;
}
// If the handler was matched purely on the file name extension then

@ -106,6 +106,10 @@
class PrefService;
class Profile;
namespace apps {
struct FileHandlerInfo;
}
namespace extensions {
struct EntryInfo;
}
@ -266,9 +270,8 @@ bool ExecuteFileTask(Profile* profile,
FileTaskFinishedCallback done);
// Returns true if a file handler matches with entries as good match.
bool IsGoodMatchFileHandler(
const extensions::FileHandlerInfo& file_handler_info,
const std::vector<extensions::EntryInfo>& entries);
bool IsGoodMatchFileHandler(const apps::FileHandlerInfo& file_handler_info,
const std::vector<extensions::EntryInfo>& entries);
// Finds the file handler tasks (apps declaring "file_handlers" in
// manifest.json) that can be used with the given entries, appending them to

@ -28,6 +28,7 @@
#include "chromeos/dbus/fake_concierge_client.h"
#include "components/prefs/pref_registry_simple.h"
#include "components/prefs/testing_pref_service.h"
#include "components/services/app_service/public/cpp/file_handler_info.h"
#include "content/public/test/test_browser_thread_bundle.h"
#include "extensions/browser/entry_info.h"
#include "extensions/browser/extension_prefs.h"
@ -306,7 +307,7 @@ TEST(FileManagerFileTasksTest, ChooseAndSetDefaultTask_FallbackOfficeEditing) {
// Test IsGoodMatchFileHandler which returns whether a file handle info matches
// with files as good match or not.
TEST(FileManagerFileTasksTest, IsGoodMatchFileHandler) {
using FileHandlerInfo = extensions::FileHandlerInfo;
using FileHandlerInfo = apps::FileHandlerInfo;
std::vector<extensions::EntryInfo> entries_1;
entries_1.emplace_back(base::FilePath(FILE_PATH_LITERAL("foo.jpg")),

@ -62,7 +62,7 @@ std::unique_ptr<base::DictionaryValue> CreateFileHandlersForBookmarkApp(
file_handler.SetKey(keys::kFileHandlerIncludeDirectories,
base::Value(false));
file_handler.SetKey(keys::kFileHandlerVerb,
base::Value(extensions::file_handler_verbs::kOpenWith));
base::Value(apps::file_handler_verbs::kOpenWith));
base::Value mime_types(base::Value::Type::LIST);
base::Value file_extensions(base::Value::Type::LIST);

@ -22,6 +22,7 @@
#include "chrome/common/chrome_paths.h"
#include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
#include "chrome/common/web_application_info.h"
#include "components/services/app_service/public/cpp/file_handler_info.h"
#include "extensions/common/extension.h"
#include "extensions/common/extension_icon_set.h"
#include "extensions/common/extension_resource.h"
@ -449,15 +450,14 @@ TEST(ExtensionFromWebApp, FileHandlersAreCorrectlyConverted) {
ASSERT_TRUE(extension.get());
const std::vector<extensions::FileHandlerInfo> file_handler_info =
const std::vector<apps::FileHandlerInfo> file_handler_info =
*extensions::FileHandlers::GetFileHandlers(extension.get());
EXPECT_EQ(2u, file_handler_info.size());
EXPECT_EQ("https://graphr.n/open-file/?name=Graph", file_handler_info[0].id);
EXPECT_FALSE(file_handler_info[0].include_directories);
EXPECT_EQ(extensions::file_handler_verbs::kOpenWith,
file_handler_info[0].verb);
EXPECT_EQ(apps::file_handler_verbs::kOpenWith, file_handler_info[0].verb);
// Extensions should contain SVG, and only SVG
EXPECT_THAT(file_handler_info[0].extensions,
testing::UnorderedElementsAre("svg"));
@ -467,8 +467,7 @@ TEST(ExtensionFromWebApp, FileHandlersAreCorrectlyConverted) {
EXPECT_EQ("https://graphr.n/open-file/?name=Raw", file_handler_info[1].id);
EXPECT_FALSE(file_handler_info[1].include_directories);
EXPECT_EQ(extensions::file_handler_verbs::kOpenWith,
file_handler_info[1].verb);
EXPECT_EQ(apps::file_handler_verbs::kOpenWith, file_handler_info[1].verb);
// Extensions should contain csv, and only csv
EXPECT_THAT(file_handler_info[1].extensions,
testing::UnorderedElementsAre("csv"));

@ -3636,6 +3636,7 @@ jumbo_split_static_library("ui") {
deps += [
"//chrome/services/app_service/public/cpp:app_update",
"//chrome/services/app_service/public/cpp:icon_loader",
"//components/services/app_service/public/cpp:app_file_handling",
]
}
}

@ -0,0 +1,10 @@
# Copyright 2019 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.
source_set("app_file_handling") {
sources = [
"file_handler_info.cc",
"file_handler_info.h",
]
}

@ -0,0 +1,25 @@
// Copyright 2019 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 "components/services/app_service/public/cpp/file_handler_info.h"
namespace apps {
namespace file_handler_verbs {
const char kOpenWith[] = "open_with";
const char kAddTo[] = "add_to";
const char kPackWith[] = "pack_with";
const char kShareWith[] = "share_with";
} // namespace file_handler_verbs
FileHandlerInfo::FileHandlerInfo()
: include_directories(false), verb(file_handler_verbs::kOpenWith) {}
FileHandlerInfo::FileHandlerInfo(const FileHandlerInfo& other) = default;
FileHandlerInfo::~FileHandlerInfo() {}
} // namespace apps

@ -0,0 +1,47 @@
// Copyright 2019 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.
#ifndef COMPONENTS_SERVICES_APP_SERVICE_PUBLIC_CPP_FILE_HANDLER_INFO_H_
#define COMPONENTS_SERVICES_APP_SERVICE_PUBLIC_CPP_FILE_HANDLER_INFO_H_
#include <set>
#include <string>
namespace apps {
namespace file_handler_verbs {
// Supported verbs for file handlers.
extern const char kOpenWith[];
extern const char kAddTo[];
extern const char kPackWith[];
extern const char kShareWith[];
} // namespace file_handler_verbs
// Contains information about a file handler for an app.
struct FileHandlerInfo {
FileHandlerInfo();
FileHandlerInfo(const FileHandlerInfo& other);
~FileHandlerInfo();
// The id of this handler.
std::string id;
// File extensions associated with this handler.
std::set<std::string> extensions;
// MIME types associated with this handler.
std::set<std::string> types;
// True if the handler can manage directories.
bool include_directories;
// A verb describing the intent of the handler.
std::string verb;
};
} // namespace apps
#endif // COMPONENTS_SERVICES_APP_SERVICE_PUBLIC_CPP_FILE_HANDLER_INFO_H_

@ -4,6 +4,7 @@ include_rules = [
"+components/crx_file",
"+components/guest_view",
"+components/prefs",
"+components/services/app_service/public",
"+components/url_matcher",
"+components/version_info",
"-content",

@ -11,6 +11,7 @@
#include "base/task/post_task.h"
#include "base/task/task_traits.h"
#include "build/build_config.h"
#include "components/services/app_service/public/cpp/file_handler_info.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/child_process_security_policy.h"
@ -37,7 +38,7 @@ const char kSecurityError[] = "Security error";
namespace {
bool FileHandlerCanHandleFileWithExtension(const FileHandlerInfo& handler,
bool FileHandlerCanHandleFileWithExtension(const apps::FileHandlerInfo& handler,
const base::FilePath& path) {
for (auto extension = handler.extensions.cbegin();
extension != handler.extensions.cend(); ++extension) {
@ -66,7 +67,7 @@ bool FileHandlerCanHandleFileWithExtension(const FileHandlerInfo& handler,
return false;
}
bool FileHandlerCanHandleFileWithMimeType(const FileHandlerInfo& handler,
bool FileHandlerCanHandleFileWithMimeType(const apps::FileHandlerInfo& handler,
const std::string& mime_type) {
for (auto type = handler.types.cbegin(); type != handler.types.cend();
++type) {
@ -208,8 +209,8 @@ void WritableFileChecker::OnPrepareFileDone(const base::FilePath& path,
} // namespace
const FileHandlerInfo* FileHandlerForId(const Extension& app,
const std::string& handler_id) {
const apps::FileHandlerInfo* FileHandlerForId(const Extension& app,
const std::string& handler_id) {
const FileHandlersInfo* file_handlers = FileHandlers::GetFileHandlers(&app);
if (!file_handlers)
return NULL;
@ -234,7 +235,7 @@ std::vector<FileHandlerMatch> FindFileHandlerMatchesForEntries(
if (!file_handlers)
return matches;
for (const FileHandlerInfo& handler : *file_handlers) {
for (const apps::FileHandlerInfo& handler : *file_handlers) {
bool handles_all_types = true;
FileHandlerMatch match;
@ -268,7 +269,7 @@ std::vector<FileHandlerMatch> FindFileHandlerMatchesForEntries(
return matches;
}
bool FileHandlerCanHandleEntry(const FileHandlerInfo& handler,
bool FileHandlerCanHandleEntry(const apps::FileHandlerInfo& handler,
const EntryInfo& entry) {
if (entry.is_directory)
return handler.include_directories;

@ -18,10 +18,13 @@ namespace content {
class BrowserContext;
}
namespace apps {
struct FileHandlerInfo;
}
namespace extensions {
struct EntryInfo;
struct FileHandlerInfo;
struct FileHandlerMatch;
struct GrantedFileEntry;
@ -34,8 +37,8 @@ extern const char kSecurityError[];
// Returns the file handler with the specified |handler_id|, or NULL if there
// is no such handler.
const FileHandlerInfo* FileHandlerForId(const Extension& app,
const std::string& handler_id);
const apps::FileHandlerInfo* FileHandlerForId(const Extension& app,
const std::string& handler_id);
// Returns the handlers that can handle all files in |entries|
// along with metadata about how the handler matched (MIME or file extension)
@ -43,7 +46,7 @@ std::vector<FileHandlerMatch> FindFileHandlerMatchesForEntries(
const Extension& extension,
const std::vector<EntryInfo>& entries);
bool FileHandlerCanHandleEntry(const FileHandlerInfo& handler,
bool FileHandlerCanHandleEntry(const apps::FileHandlerInfo& handler,
const EntryInfo& entry);
// Creates a new file entry and allows |renderer_id| to access |path|. This

@ -4,6 +4,7 @@
#include "extensions/browser/api/file_handlers/app_file_handler_util.h"
#include "components/services/app_service/public/cpp/file_handler_info.h"
#include "extensions/browser/entry_info.h"
#include "testing/gtest/include/gtest/gtest.h"
@ -11,15 +12,16 @@ namespace extensions {
namespace app_file_handler_util {
namespace {
FileHandlerInfo CreateHandlerInfoFromExtension(const std::string& extension) {
FileHandlerInfo handler_info;
apps::FileHandlerInfo CreateHandlerInfoFromExtension(
const std::string& extension) {
apps::FileHandlerInfo handler_info;
handler_info.extensions.insert(extension);
return handler_info;
}
FileHandlerInfo CreateHandlerInfoFromIncludeDirectories(
apps::FileHandlerInfo CreateHandlerInfoFromIncludeDirectories(
bool include_directories) {
FileHandlerInfo handler_info;
apps::FileHandlerInfo handler_info;
handler_info.include_directories = include_directories;
return handler_info;
}

@ -311,6 +311,7 @@ if (enable_extensions) {
public_deps = [
":common_constants",
":mojom",
"//components/services/app_service/public/cpp:app_file_handling",
"//content/public/common",
"//ipc",
"//skia",

@ -2,7 +2,6 @@ include_rules = [
"+components/crx_file",
"+components/url_formatter",
"+components/nacl/common/buildflags.h",
"+components/services/app_service/public/mojom",
"+device/bluetooth", # For BluetoothPermission
"+grit/extensions_strings.h",
"+libxml",

@ -21,34 +21,20 @@ namespace extensions {
namespace keys = manifest_keys;
namespace errors = manifest_errors;
namespace file_handler_verbs {
const char kOpenWith[] = "open_with";
const char kAddTo[] = "add_to";
const char kPackWith[] = "pack_with";
const char kShareWith[] = "share_with";
} // namespace file_handler_verbs
namespace {
const int kMaxTypeAndExtensionHandlers = 200;
const char kNotRecognized[] = "'%s' is not a recognized file handler property.";
bool IsSupportedVerb(const std::string& verb) {
return verb == file_handler_verbs::kOpenWith ||
verb == file_handler_verbs::kAddTo ||
verb == file_handler_verbs::kPackWith ||
verb == file_handler_verbs::kShareWith;
return verb == apps::file_handler_verbs::kOpenWith ||
verb == apps::file_handler_verbs::kAddTo ||
verb == apps::file_handler_verbs::kPackWith ||
verb == apps::file_handler_verbs::kShareWith;
}
} // namespace
FileHandlerInfo::FileHandlerInfo()
: include_directories(false), verb(file_handler_verbs::kOpenWith) {}
FileHandlerInfo::FileHandlerInfo(const FileHandlerInfo& other) = default;
FileHandlerInfo::~FileHandlerInfo() {}
FileHandlerMatch::FileHandlerMatch() = default;
FileHandlerMatch::~FileHandlerMatch() = default;
@ -75,7 +61,7 @@ bool LoadFileHandler(const std::string& handler_id,
base::string16* error,
std::vector<InstallWarning>* install_warnings) {
DCHECK(error);
FileHandlerInfo handler;
apps::FileHandlerInfo handler;
handler.id = handler_id;
@ -106,7 +92,7 @@ bool LoadFileHandler(const std::string& handler_id,
}
}
handler.verb = file_handler_verbs::kOpenWith;
handler.verb = apps::file_handler_verbs::kOpenWith;
const base::Value* file_handler =
handler_info.FindKey(keys::kFileHandlerVerb);
if (file_handler != nullptr) {

@ -10,38 +10,18 @@
#include <vector>
#include "base/macros.h"
#include "components/services/app_service/public/cpp/file_handler_info.h"
#include "extensions/common/extension.h"
#include "extensions/common/manifest_handler.h"
namespace extensions {
struct FileHandlerInfo {
FileHandlerInfo();
FileHandlerInfo(const FileHandlerInfo& other);
~FileHandlerInfo();
// The id of this handler.
std::string id;
// File extensions associated with this handler.
std::set<std::string> extensions;
// MIME types associated with this handler.
std::set<std::string> types;
// True if the handler can manage directories.
bool include_directories;
// A verb describing the intent of the handler.
std::string verb;
};
typedef std::vector<FileHandlerInfo> FileHandlersInfo;
using FileHandlersInfo = std::vector<apps::FileHandlerInfo>;
struct FileHandlerMatch {
FileHandlerMatch();
~FileHandlerMatch();
const FileHandlerInfo* handler = nullptr;
const apps::FileHandlerInfo* handler = nullptr;
// True if the handler matched on MIME type
bool matched_mime = false;
@ -73,16 +53,6 @@ class FileHandlersParser : public ManifestHandler {
DISALLOW_COPY_AND_ASSIGN(FileHandlersParser);
};
namespace file_handler_verbs {
// Supported verbs for file handlers.
extern const char kOpenWith[];
extern const char kAddTo[];
extern const char kPackWith[];
extern const char kShareWith[];
} // namespace file_handler_verbs
} // namespace extensions
#endif // EXTENSIONS_COMMON_MANIFEST_HANDLERS_FILE_HANDLER_INFO_H_

@ -5,6 +5,7 @@
#include <vector>
#include "base/stl_util.h"
#include "components/services/app_service/public/cpp/file_handler_info.h"
#include "extensions/common/install_warning.h"
#include "extensions/common/manifest.h"
#include "extensions/common/manifest_constants.h"
@ -52,7 +53,7 @@ TEST_F(FileHandlersManifestTest, ValidFileHandlers) {
ASSERT_TRUE(handlers != NULL);
ASSERT_EQ(3U, handlers->size());
FileHandlerInfo handler = handlers->at(0);
apps::FileHandlerInfo handler = handlers->at(0);
EXPECT_EQ("directories", handler.id);
EXPECT_EQ(0U, handler.types.size());
EXPECT_EQ(1U, handler.extensions.size());