Skip to content

Commit

Permalink
Extensions: Stop file_handlers' support for hosted_apps
Browse files Browse the repository at this point in the history
Now BMO has landed, hosted_apps should be removed from the list of
valid extension types for "file_handling" in
extensions/common/api/_manifest_features.json.

Bug: 1233303
Change-Id: I716a2eb14b26516737011ca3ed980da059515789
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3146714
Reviewed-by: Ben Wells <benwells@chromium.org>
Commit-Queue: Fangzhen Song <songfangzhen@bytedance.com>
Cr-Commit-Position: refs/heads/main@{#919151}
  • Loading branch information
song-fangzhen authored and Chromium LUCI CQ committed Sep 8, 2021
1 parent 2a58c8e commit 8bcd484
Show file tree
Hide file tree
Showing 6 changed files with 1 addition and 95 deletions.
5 changes: 1 addition & 4 deletions extensions/common/api/_manifest_features.json
Original file line number Diff line number Diff line change
Expand Up @@ -154,10 +154,7 @@
"file_handlers": [
{
"channel": "stable",
"extension_types": [
"platform_app",
"hosted_app" // This should be removed when bookmark apps have migrated off hosted apps.
]
"extension_types": ["platform_app"]
}, {
"channel": "stable",
"extension_types": [ "extension"],
Expand Down
2 changes: 0 additions & 2 deletions extensions/common/manifest_constants.cc
Original file line number Diff line number Diff line change
Expand Up @@ -380,8 +380,6 @@ const char kInvalidFileFilterValue[] =
"Invalid value for 'file_filters[*]'.";
const char kInvalidFileHandlers[] =
"Invalid value for 'file_handlers'.";
const char kInvalidFileHandlersHostedAppsNotSupported[] =
"Hosted apps do not support file handlers";
const char kInvalidFileHandlersTooManyTypesAndExtensions[] =
"Too many MIME and extension file_handlers have been declared.";
const char kInvalidFileHandlerExtension[] =
Expand Down
1 change: 0 additions & 1 deletion extensions/common/manifest_constants.h
Original file line number Diff line number Diff line change
Expand Up @@ -298,7 +298,6 @@ extern const char kInvalidFileBrowserHandlerMissingPermission[];
extern const char kInvalidFileFiltersList[];
extern const char kInvalidFileFilterValue[];
extern const char kInvalidFileHandlers[];
extern const char kInvalidFileHandlersHostedAppsNotSupported[];
extern const char kInvalidFileHandlersTooManyTypesAndExtensions[];
extern const char kInvalidFileHandlerExtension[];
extern const char kInvalidFileHandlerExtensionElement[];
Expand Down
11 changes: 0 additions & 11 deletions extensions/common/manifest_handlers/file_handler_info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -159,17 +159,6 @@ bool LoadFileHandler(const std::string& handler_id,
}

bool FileHandlersParser::Parse(Extension* extension, std::u16string* error) {
// Don't load file handlers for hosted_apps unless they're also bookmark apps.
// This check can be removed when bookmark apps are migrated off hosted apps,
// and hosted_apps should be removed from the list of valid extension types
// for "file_handling" in extensions/common/api/_manifest_features.json.
if (extension->is_hosted_app() && !extension->from_bookmark()) {
extension->AddInstallWarning(
InstallWarning(errors::kInvalidFileHandlersHostedAppsNotSupported,
keys::kFileHandlers));
return true;
}

std::unique_ptr<FileHandlers> info(new FileHandlers);
const base::Value* all_handlers = nullptr;
if (!extension->manifest()->GetDictionary(keys::kFileHandlers,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include <vector>

#include "base/cxx17_backports.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"
#include "extensions/common/manifest_handlers/file_handler_info.h"
#include "extensions/common/manifest_test.h"
Expand Down Expand Up @@ -87,45 +83,4 @@ TEST_F(FileHandlersManifestTest, NotPlatformApp) {
ASSERT_TRUE(handlers == NULL);
}

TEST_F(FileHandlersManifestTest, HostedNotBookmarkApp) {
// This should load successfully but have the file handlers ignored.
scoped_refptr<const Extension> extension =
LoadAndExpectSuccess("file_handlers_valid_hosted_app.json",
extensions::mojom::ManifestLocation::kInternal);

ASSERT_TRUE(extension);

std::vector<InstallWarning> expected_warnings;
expected_warnings.push_back(
InstallWarning(errors::kInvalidFileHandlersHostedAppsNotSupported));
EXPECT_EQ(expected_warnings, extension->install_warnings());

EXPECT_TRUE(extension->is_hosted_app());
EXPECT_FALSE(extension->from_bookmark());

const FileHandlersInfo* handlers =
FileHandlers::GetFileHandlers(extension.get());
EXPECT_FALSE(handlers);
}

TEST_F(FileHandlersManifestTest, HostedBookmarkApp) {
// This should load successfully with file handlers.
scoped_refptr<const Extension> extension =
LoadAndExpectSuccess("file_handlers_valid_hosted_app.json",
extensions::mojom::ManifestLocation::kInternal,
extensions::Extension::FROM_BOOKMARK);

ASSERT_TRUE(extension);
EXPECT_TRUE(extension->install_warnings().empty());

// Check we're a hosted app and a bookmark app.
EXPECT_TRUE(extension->is_hosted_app());
EXPECT_TRUE(extension->from_bookmark());

const FileHandlersInfo* handlers =
FileHandlers::GetFileHandlers(extension.get());
ASSERT_TRUE(handlers);
EXPECT_GE(handlers->size(), 1u);
}

} // namespace extensions

This file was deleted.

0 comments on commit 8bcd484

Please sign in to comment.