Skip to content

Commit

Permalink
Extensions: WFH: Support Lacros too and restrict platforms to it and Ash
Browse files Browse the repository at this point in the history
A preprocessor directive was added to account for operating systems
other than ChromeOS.

WebFileHandlersTest requires the channel definition, while others tests
do not need them to succeed. The channel has been removed from the
others so that they can remain channel agnostic when moving to stable.

Bug: chromium:1448893
Change-Id: I45fd5299b729d228a71725d454799f4aea3fd755
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4982203
Reviewed-by: Tim Sergeant <tsergeant@chromium.org>
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Commit-Queue: Solomon Kinard <solomonkinard@chromium.org>
Reviewed-by: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Ben Reich <benreich@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1216743}
  • Loading branch information
solomonkinardchromium authored and Chromium LUCI CQ committed Oct 30, 2023
1 parent adcf711 commit 53c1edd
Show file tree
Hide file tree
Showing 9 changed files with 26 additions and 25 deletions.
4 changes: 2 additions & 2 deletions chrome/browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9484,12 +9484,12 @@ const FeatureEntry kFeatureEntries[] = {
FEATURE_VALUE_TYPE(
feature_engagement::kIPHExtensionsRequestAccessButtonFeature)},

#if BUILDFLAG(IS_CHROMEOS_ASH)
#if BUILDFLAG(IS_CHROMEOS)
{"extension-web-file-handlers",
flag_descriptions::kExtensionWebFileHandlersName,
flag_descriptions::kExtensionWebFileHandlersDescription, kOsCrOS,
FEATURE_VALUE_TYPE(extensions_features::kExtensionWebFileHandlers)},
#endif // IS_CHROMEOS_ASH
#endif // IS_CHROMEOS
#if BUILDFLAG(IS_WIN)
{"launch-windows-native-hosts-directly",
flag_descriptions::kLaunchWindowsNativeHostsDirectlyName,
Expand Down
5 changes: 1 addition & 4 deletions chrome/browser/apps/app_service/intent_util_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,9 @@
#include "components/services/app_service/public/cpp/intent_filter.h"
#include "components/services/app_service/public/cpp/intent_filter_util.h"
#include "components/services/app_service/public/cpp/intent_util.h"
#include "components/version_info/channel.h"
#include "extensions/common/api/app_runtime.h"
#include "extensions/common/extension_builder.h"
#include "extensions/common/extension_features.h"
#include "extensions/common/features/feature_channel.h"
#include "mojo/public/cpp/bindings/struct_ptr.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "url/gurl.h"
Expand Down Expand Up @@ -436,14 +434,13 @@ TEST_F(IntentUtilsTest, CreateIntentFiltersForExtension_FileHandlers) {

class IntentUtilsForExtensionsTest : public IntentUtilsTest {
public:
IntentUtilsForExtensionsTest() : channel_(version_info::Channel::BETA) {
IntentUtilsForExtensionsTest() {
feature_list_.InitAndEnableFeature(
extensions_features::kExtensionWebFileHandlers);
}

private:
base::test::ScopedFeatureList feature_list_;
extensions::ScopedCurrentChannel channel_;
};

TEST_F(IntentUtilsForExtensionsTest,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
#include "base/files/file_util.h"
#include "base/memory/scoped_refptr.h"
#include "base/strings/string_piece_forward.h"
#include "base/test/scoped_feature_list.h"
#include "base/strings/stringprintf.h"
#include "base/threading/thread_restrictions.h"
#include "chrome/browser/apps/app_service/app_service_proxy.h"
#include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
Expand All @@ -21,7 +21,6 @@
#include "chrome/test/base/chrome_test_utils.h"
#include "components/services/app_service/public/cpp/intent.h"
#include "components/services/app_service/public/cpp/intent_util.h"
#include "components/version_info/channel.h"
#include "content/public/browser/web_contents.h"
#include "content/public/test/browser_test.h"
#include "extensions/common/extension.h"
Expand Down Expand Up @@ -134,8 +133,6 @@ class ExtensionAppsChromeOsBrowserTest
}

base::test::ScopedFeatureList feature_list_;
extensions::ScopedCurrentChannel current_channel_{
version_info::Channel::BETA};
};

// Open the extension action url when opening a matching file type.
Expand All @@ -161,6 +158,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionAppsChromeOsBrowserTest, LaunchWithFileIntent) {
const extensions::Extension* extension =
InstallDefaultInstalledExtension(extension_dir.UnpackedPath());
ASSERT_TRUE(extension);

ASSERT_TRUE(extensions::WebFileHandlers::SupportsWebFileHandlers(*extension));
LaunchExtensionAndCatchResult(*extension);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,10 @@
#include "components/sync_preferences/testing_pref_service_syncable.h"
#include "components/user_manager/scoped_user_manager.h"
#include "components/user_manager/user_type.h"
#include "components/version_info/channel.h"
#include "content/public/test/browser_task_environment.h"
#include "extensions/browser/entry_info.h"
#include "extensions/common/extension_builder.h"
#include "extensions/common/extension_features.h"
#include "extensions/common/features/feature_channel.h"
#include "storage/browser/file_system/external_mount_points.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down Expand Up @@ -670,14 +668,13 @@ TEST_F(AppServiceFileTasksTestEnabled,
// Enable MV3 File Handlers.
class AppServiceFileHandlersTest : public AppServiceFileTasksTestEnabled {
public:
AppServiceFileHandlersTest() : channel_(version_info::Channel::BETA) {
AppServiceFileHandlersTest() {
feature_list_.InitAndEnableFeature(
extensions_features::kExtensionWebFileHandlers);
}

private:
base::test::ScopedFeatureList feature_list_;
extensions::ScopedCurrentChannel channel_;
};

// Verify App Service tasks for extensions with MV3 File Handlers.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include <string>

#include "chrome/browser/extensions/extension_browsertest.h"
#include "components/version_info/channel.h"
#include "content/public/test/browser_test.h"
#include "extensions/common/extension_features.h"
#include "extensions/common/manifest_handlers/web_file_handlers_info.h"
Expand All @@ -16,15 +15,13 @@ namespace {

class FileHandlingWithoutFeatureBrowserTest : public ExtensionBrowserTest {
public:
FileHandlingWithoutFeatureBrowserTest()
: channel_(version_info::Channel::BETA) {
FileHandlingWithoutFeatureBrowserTest() {
feature_list_.InitAndDisableFeature(
extensions_features::kExtensionWebFileHandlers);
}

private:
base::test::ScopedFeatureList feature_list_;
extensions::ScopedCurrentChannel channel_;
};

// Web File Handlers are either parsed or emit a warning, depending on the
Expand Down Expand Up @@ -76,8 +73,15 @@ IN_PROC_BROWSER_TEST_F(FileHandlingWithoutFeatureBrowserTest, Warning) {
// Verify that there are no file handlers and that a warning was observed.
ASSERT_FALSE(WebFileHandlers::HasFileHandlers(*extension));
ASSERT_EQ(1u, extension->install_warnings().size());
EXPECT_EQ(std::string("Unrecognized manifest key 'file_handlers'."),
#if BUILDFLAG(IS_CHROMEOS)
EXPECT_EQ("Unrecognized manifest key 'file_handlers'.",
extension->install_warnings().front().message);
#else
EXPECT_EQ(
"'file_handlers' is only allowed for packaged apps, but this is a "
"extension.",
extension->install_warnings().front().message);
#endif // IS_CHROMEOS
}
}

Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/flag_descriptions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1583,13 +1583,13 @@ const char kSafetyCheckExtensionsDescription[] =
"desktop. The module will be shown if there are potentially unsafe "
"extensions to review.";

#if BUILDFLAG(IS_CHROMEOS_ASH)
#if BUILDFLAG(IS_CHROMEOS)
const char kExtensionWebFileHandlersName[] = "Extensions Web File Handlers";
const char kExtensionWebFileHandlersDescription[] =
"Enable Extension Web File Handlers, which allows extensions to operate on "
"the native file system. An extension can register to read and edit files, "
"specified in the manifest, by their file extension or mime type.";
#endif // IS_CHROMEOS_ASH
#endif // IS_CHROMEOS
#endif // ENABLE_EXTENSIONS

const char kExtensionsOnChromeUrlsName[] = "Extensions on chrome:// URLs";
Expand Down
4 changes: 2 additions & 2 deletions chrome/browser/flag_descriptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -899,10 +899,10 @@ extern const char kCWSInfoFastCheckDescription[];
extern const char kSafetyCheckExtensionsName[];
extern const char kSafetyCheckExtensionsDescription[];

#if BUILDFLAG(IS_CHROMEOS_ASH)
#if BUILDFLAG(IS_CHROMEOS)
extern const char kExtensionWebFileHandlersName[];
extern const char kExtensionWebFileHandlersDescription[];
#endif // IS_CHROMEOS_ASH
#endif // IS_CHROMEOS
#endif // ENABLE_EXTENSIONS

extern const char kExtensionsOnChromeUrlsName[];
Expand Down
5 changes: 4 additions & 1 deletion extensions/common/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -612,7 +612,6 @@ source_set("unit_tests") {
"manifest_handlers/default_locale_manifest_unittest.cc",
"manifest_handlers/extension_action_handler_unittest.cc",
"manifest_handlers/externally_connectable_unittest.cc",
"manifest_handlers/file_handler_manifest_unittest.cc",
"manifest_handlers/icons_handler_unittest.cc",
"manifest_handlers/incognito_manifest_unittest.cc",
"manifest_handlers/kiosk_mode_info_unittest.cc",
Expand Down Expand Up @@ -668,6 +667,10 @@ source_set("unit_tests") {
"//url",
]

if (is_chromeos) {
sources += [ "manifest_handlers/file_handler_manifest_unittest.cc" ]
}

if (is_chromeos_ash) {
sources += [ "manifest_handlers/action_handlers_handler_unittest.cc" ]
}
Expand Down
3 changes: 2 additions & 1 deletion extensions/common/api/_manifest_features.json
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,8 @@
{
"channel": "stable",
"extension_types": ["extension"],
"min_manifest_version": 3
"min_manifest_version": 3,
"platforms": ["chromeos", "lacros"]
}
],
"host_permissions": [{
Expand Down

0 comments on commit 53c1edd

Please sign in to comment.