Commit a8007e4f authored by Robert Woods's avatar Robert Woods Committed by Commit Bot

WebApps: Serialize apps::FileHandlers when converting to Bookmark App.

This CL is related to two ongoing projects: the implementation of file
handling APIs for PWAs in Chrome (go/chrome-file-handling-api), and
BMO (go/chrome-bmo), which is concerned with moving PWAs off the
Extensions stack and establish them on an independent web apps
platform.

As part of the first project, we created a new representation for file
handlers (apps::FileHandler) that has some properties needed for OS
integration. Pre-BMO PWAs ("Bookmark Apps") use apps::FileHandlerInfo.
It's simple to convert from FileHandler to FileHandlerInfo, but
impossible to do the reverse. This means that if file handling lands
before BMO, migrating installed PWAs to the new system will be a
destructive operation.

The goal for the set of CLs to which this one belongs is to have BMO
web apps and Bookmark Apps share the apps::FileHandler representation.
We have added a new "web_app_file_handlers" key to the Extension
manifest, along with parsing for the new format (crrev.com/c/2081714).
Here we serialize the apps::FileHandlers attached to a web app (under
the new key) when installing it as a Bookmark App. In subsequent CLs,
we replace apps::FileHandlerInfo with apps::FileHandler across both
Bookmark Apps and BMO web apps (crrev.com/c/2087306), and finally
implement migrating Bookmark App file handlers to the new platform as
a simple copy (crrev.com/c/2086453).

Bug: 938103
Change-Id: Iec258386be92ae36b8016a21119ee324a955e5d7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2087327Reviewed-by: default avatarIstiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: default avatarAlexey Baskakov <loyso@chromium.org>
Commit-Queue: Robert Woods <robertwoods@google.com>
Cr-Commit-Position: refs/heads/master@{#750367}
parent 367befd9
...@@ -56,7 +56,11 @@ namespace { ...@@ -56,7 +56,11 @@ namespace {
const char kIconsDirName[] = "icons"; const char kIconsDirName[] = "icons";
const char kScopeUrlHandlerId[] = "scope"; const char kScopeUrlHandlerId[] = "scope";
std::unique_ptr<base::DictionaryValue> CreateFileHandlersForBookmarkApp( bool IsValidFileExtension(const std::string& file_extension) {
return !file_extension.empty() && file_extension[0] == '.';
}
base::Value CreateFileHandlersForBookmarkApp(
const std::vector<blink::Manifest::FileHandler>& manifest_file_handlers) { const std::vector<blink::Manifest::FileHandler>& manifest_file_handlers) {
base::Value file_handlers(base::Value::Type::DICTIONARY); base::Value file_handlers(base::Value::Type::DICTIONARY);
...@@ -78,10 +82,7 @@ std::unique_ptr<base::DictionaryValue> CreateFileHandlersForBookmarkApp( ...@@ -78,10 +82,7 @@ std::unique_ptr<base::DictionaryValue> CreateFileHandlersForBookmarkApp(
mime_types.Append(base::Value(type)); mime_types.Append(base::Value(type));
for (const auto& extensionUTF16 : it.second) { for (const auto& extensionUTF16 : it.second) {
std::string extension = base::UTF16ToUTF8(extensionUTF16); std::string extension = base::UTF16ToUTF8(extensionUTF16);
if (extension.empty()) if (!IsValidFileExtension(extension))
continue;
if (extension[0] != '.')
continue; continue;
// Remove the '.' before appending. // Remove the '.' before appending.
...@@ -96,8 +97,40 @@ std::unique_ptr<base::DictionaryValue> CreateFileHandlersForBookmarkApp( ...@@ -96,8 +97,40 @@ std::unique_ptr<base::DictionaryValue> CreateFileHandlersForBookmarkApp(
file_handlers.SetKey(entry.action.spec(), std::move(file_handler)); file_handlers.SetKey(entry.action.spec(), std::move(file_handler));
} }
return base::DictionaryValue::From( return file_handlers;
base::Value::ToUniquePtrValue(std::move(file_handlers))); }
base::Value CreateWebAppFileHandlersForBookmarkApp(
const std::vector<blink::Manifest::FileHandler>& manifest_file_handlers) {
base::Value file_handlers(base::Value::Type::LIST);
for (const auto& manifest_file_handler : manifest_file_handlers) {
base::Value file_handler(base::Value::Type::DICTIONARY);
base::Value accept(base::Value::Type::DICTIONARY);
for (const auto& manifest_accept_entry : manifest_file_handler.accept) {
std::string mime_type = base::UTF16ToUTF8(manifest_accept_entry.first);
if (mime_type.empty())
continue;
base::Value file_extensions(base::Value::Type::LIST);
for (const auto& manifest_file_extension : manifest_accept_entry.second) {
std::string file_extension = base::UTF16ToUTF8(manifest_file_extension);
if (!IsValidFileExtension(file_extension))
continue;
file_extensions.Append(base::Value(file_extension));
}
accept.SetKey(std::move(mime_type), std::move(file_extensions));
}
file_handler.SetKey(keys::kWebAppFileHandlerAction,
base::Value(manifest_file_handler.action.spec()));
file_handler.SetKey(keys::kWebAppFileHandlerAccept, std::move(accept));
file_handlers.Append(std::move(file_handler));
}
return file_handlers;
} }
} // namespace } // namespace
...@@ -235,9 +268,16 @@ scoped_refptr<Extension> ConvertWebAppToExtension( ...@@ -235,9 +268,16 @@ scoped_refptr<Extension> ConvertWebAppToExtension(
root->SetString(keys::kAppDisplayMode, root->SetString(keys::kAppDisplayMode,
blink::DisplayModeToString(web_app.display_mode)); blink::DisplayModeToString(web_app.display_mode));
// TODO(crbug.com/938103): The app's file handlers are serialized twice here,
// as apps::FileHandlerInfo and apps::FileHandler. This is clearly redundant,
// but only a temporary measure, until web apps move off Bookmark Apps with
// the launch of BMO (at which point the apps::FileHandlerInfo representation
// can be removed).
if (web_app.file_handlers.size() != 0) { if (web_app.file_handlers.size() != 0) {
root->SetDictionary(keys::kFileHandlers, CreateFileHandlersForBookmarkApp( root->SetKey(keys::kFileHandlers,
web_app.file_handlers)); CreateFileHandlersForBookmarkApp(web_app.file_handlers));
root->SetKey(keys::kWebAppFileHandlers,
CreateWebAppFileHandlersForBookmarkApp(web_app.file_handlers));
} }
// Add the icons and linked icon information. // Add the icons and linked icon information.
......
...@@ -23,6 +23,7 @@ ...@@ -23,6 +23,7 @@
#include "chrome/common/extensions/manifest_handlers/app_launch_info.h" #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
#include "chrome/common/extensions/manifest_handlers/linked_app_icons.h" #include "chrome/common/extensions/manifest_handlers/linked_app_icons.h"
#include "chrome/common/web_application_info.h" #include "chrome/common/web_application_info.h"
#include "components/services/app_service/public/cpp/file_handler.h"
#include "components/services/app_service/public/cpp/file_handler_info.h" #include "components/services/app_service/public/cpp/file_handler_info.h"
#include "extensions/common/extension.h" #include "extensions/common/extension.h"
#include "extensions/common/extension_icon_set.h" #include "extensions/common/extension_icon_set.h"
...@@ -30,6 +31,7 @@ ...@@ -30,6 +31,7 @@
#include "extensions/common/manifest_constants.h" #include "extensions/common/manifest_constants.h"
#include "extensions/common/manifest_handlers/file_handler_info.h" #include "extensions/common/manifest_handlers/file_handler_info.h"
#include "extensions/common/manifest_handlers/icons_handler.h" #include "extensions/common/manifest_handlers/icons_handler.h"
#include "extensions/common/manifest_handlers/web_app_file_handler.h"
#include "extensions/common/permissions/permission_set.h" #include "extensions/common/permissions/permission_set.h"
#include "extensions/common/permissions/permissions_data.h" #include "extensions/common/permissions/permissions_data.h"
#include "extensions/common/url_pattern.h" #include "extensions/common/url_pattern.h"
...@@ -481,4 +483,59 @@ TEST(ExtensionFromWebApp, FileHandlersAreCorrectlyConverted) { ...@@ -481,4 +483,59 @@ TEST(ExtensionFromWebApp, FileHandlersAreCorrectlyConverted) {
testing::UnorderedElementsAre("text/csv")); testing::UnorderedElementsAre("text/csv"));
} }
// Tests that |file_handler| on the WebAppManifest is correctly converted
// to |web_app_file_handlers| on an extension manifest.
TEST(ExtensionFromWebApp, WebAppFileHandlersAreCorrectlyConverted) {
base::ScopedTempDir extensions_dir;
ASSERT_TRUE(extensions_dir.CreateUniqueTempDir());
WebApplicationInfo web_app;
web_app.title = base::ASCIIToUTF16("Graphr");
web_app.description = base::ASCIIToUTF16("A magical graphy thing.");
web_app.app_url = GURL("https://graphr.n/");
web_app.scope = GURL("https://graphr.n");
{
blink::Manifest::FileHandler file_handler;
file_handler.action = GURL("https://graphr.n/open-graph/");
file_handler.name = base::ASCIIToUTF16("Graph");
file_handler.accept[base::ASCIIToUTF16("text/svg+xml")].push_back(
base::ASCIIToUTF16(""));
file_handler.accept[base::ASCIIToUTF16("text/svg+xml")].push_back(
base::ASCIIToUTF16(".svg"));
web_app.file_handlers.push_back(file_handler);
}
{
blink::Manifest::FileHandler file_handler;
file_handler.action = GURL("https://graphr.n/open-raw/");
file_handler.name = base::ASCIIToUTF16("Raw");
file_handler.accept[base::ASCIIToUTF16("text/csv")].push_back(
base::ASCIIToUTF16(".csv"));
web_app.file_handlers.push_back(file_handler);
}
scoped_refptr<Extension> extension = ConvertWebAppToExtension(
web_app, GetTestTime(1978, 12, 11, 0, 0, 0, 0), extensions_dir.GetPath(),
Extension::NO_FLAGS, Manifest::INTERNAL);
ASSERT_TRUE(extension.get());
const apps::FileHandlers file_handlers =
*extensions::WebAppFileHandlers::GetWebAppFileHandlers(extension.get());
EXPECT_EQ(2u, file_handlers.size());
EXPECT_EQ("https://graphr.n/open-graph/", file_handlers[0].action);
EXPECT_EQ(1u, file_handlers[0].accept.size());
EXPECT_EQ("text/svg+xml", file_handlers[0].accept[0].mime_type);
EXPECT_THAT(file_handlers[0].accept[0].file_extensions,
testing::UnorderedElementsAre(".svg"));
EXPECT_EQ("https://graphr.n/open-raw/", file_handlers[1].action);
EXPECT_EQ(1u, file_handlers[1].accept.size());
EXPECT_EQ("text/csv", file_handlers[1].accept[0].mime_type);
EXPECT_THAT(file_handlers[1].accept[0].file_extensions,
testing::UnorderedElementsAre(".csv"));
}
} // namespace extensions } // namespace extensions
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment