Commit 579999e1 authored by Toby Huang's avatar Toby Huang Committed by Chromium LUCI CQ

Extensions: Remove SupervisedUserInitiatedExtensionInstall feature flag

This CL cleans up a feature flag that has already been on stable for
over three milestones.

Bug: 1158494
Change-Id: I6a529e3b31e34fb5af19e6b5e5e471c475ff80e4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2602251Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarDan S <danan@chromium.org>
Commit-Queue: Toby Huang <tobyhuang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840371}
parent ff9183ee
...@@ -48,10 +48,8 @@ ...@@ -48,10 +48,8 @@
// flag to #if defined(OS_CHROMEOS) // flag to #if defined(OS_CHROMEOS)
#include "base/test/metrics/histogram_tester.h" #include "base/test/metrics/histogram_tester.h"
#include "base/test/metrics/user_action_tester.h" #include "base/test/metrics/user_action_tester.h"
#include "base/test/scoped_feature_list.h"
#include "chrome/browser/supervised_user/logged_in_user_mixin.h" #include "chrome/browser/supervised_user/logged_in_user_mixin.h"
#include "chrome/browser/supervised_user/supervised_user_constants.h" #include "chrome/browser/supervised_user/supervised_user_constants.h"
#include "chrome/browser/supervised_user/supervised_user_features.h"
#include "chrome/browser/supervised_user/supervised_user_service.h" #include "chrome/browser/supervised_user/supervised_user_service.h"
#include "chrome/browser/supervised_user/supervised_user_service_factory.h" #include "chrome/browser/supervised_user/supervised_user_service_factory.h"
#include "chrome/browser/supervised_user/supervised_user_test_util.h" #include "chrome/browser/supervised_user/supervised_user_test_util.h"
...@@ -366,11 +364,23 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTest, EmptyCrx) { ...@@ -366,11 +364,23 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTest, EmptyCrx) {
static constexpr char kTestChildEmail[] = "test_child_user@google.com"; static constexpr char kTestChildEmail[] = "test_child_user@google.com";
static constexpr char kTestChildGaiaId[] = "8u8tuw09sufncmnaos"; static constexpr char kTestChildGaiaId[] = "8u8tuw09sufncmnaos";
static constexpr char kTestAppId[] = "iladmdjkfniedhfhcfoefgojhgaiaccc";
static constexpr char kTestAppVersion[] = "0.1";
// Test fixture for various cases of installation for child accounts.
class ExtensionWebstorePrivateApiTestChild class ExtensionWebstorePrivateApiTestChild
: public ExtensionWebstorePrivateApiTest { : public ExtensionWebstorePrivateApiTest,
public TestParentPermissionDialogViewObserver {
public: public:
// The next dialog action to take.
enum class NextDialogAction {
kCancel,
kAccept,
};
ExtensionWebstorePrivateApiTestChild() ExtensionWebstorePrivateApiTestChild()
: embedded_test_server_(std::make_unique<net::EmbeddedTestServer>()), : TestParentPermissionDialogViewObserver(this),
embedded_test_server_(std::make_unique<net::EmbeddedTestServer>()),
logged_in_user_mixin_( logged_in_user_mixin_(
&mixin_host_, &mixin_host_,
chromeos::LoggedInUserMixin::LogInType::kChild, chromeos::LoggedInUserMixin::LogInType::kChild,
...@@ -428,67 +438,6 @@ class ExtensionWebstorePrivateApiTestChild ...@@ -428,67 +438,6 @@ class ExtensionWebstorePrivateApiTestChild
->SetNextReAuthStatus(next_status); ->SetNextReAuthStatus(next_status);
} }
protected:
std::unique_ptr<signin::IdentityTestEnvironment> identity_test_env_;
private:
// Create another embedded test server to avoid starting the same one twice.
std::unique_ptr<net::EmbeddedTestServer> embedded_test_server_;
chromeos::LoggedInUserMixin logged_in_user_mixin_;
};
class ExtensionWebstorePrivateApiTestChildInstallDisabled
: public ExtensionWebstorePrivateApiTestChild {
public:
ExtensionWebstorePrivateApiTestChildInstallDisabled() {
feature_list_.InitWithFeatures(
{}, {supervised_users::kSupervisedUserInitiatedExtensionInstall});
}
private:
base::test::ScopedFeatureList feature_list_;
};
// Tests that extension installation is blocked for child accounts when
// the feature flag is disabled.
IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTestChildInstallDisabled,
InstallBlocked) {
base::HistogramTester histogram_tester;
base::UserActionTester user_action_tester;
ASSERT_TRUE(RunInstallTest("install_blocked_child.html", "app.crx"));
histogram_tester.ExpectUniqueSample(
SupervisedUserExtensionsMetricsRecorder::kEnablementHistogramName,
SupervisedUserExtensionsMetricsRecorder::EnablementState::kFailedToEnable,
1);
histogram_tester.ExpectTotalCount(
SupervisedUserExtensionsMetricsRecorder::kEnablementHistogramName, 1);
EXPECT_EQ(
1,
user_action_tester.GetActionCount(
SupervisedUserExtensionsMetricsRecorder::kFailedToEnableActionName));
}
static constexpr char kTestAppId[] = "iladmdjkfniedhfhcfoefgojhgaiaccc";
static constexpr char kTestAppVersion[] = "0.1";
// Test fixture for various cases of installation for child accounts
// when the feature flag is enabled.
class ExtensionWebstorePrivateApiTestChildInstallEnabled
: public ExtensionWebstorePrivateApiTestChild,
public TestParentPermissionDialogViewObserver {
public:
// The next dialog action to take.
enum class NextDialogAction {
kCancel,
kAccept,
};
ExtensionWebstorePrivateApiTestChildInstallEnabled()
: TestParentPermissionDialogViewObserver(this) {
feature_list_.InitWithFeatures(
{supervised_users::kSupervisedUserInitiatedExtensionInstall}, {});
}
// TestParentPermissionDialogViewObserver override: // TestParentPermissionDialogViewObserver override:
void OnTestParentPermissionDialogViewCreated( void OnTestParentPermissionDialogViewCreated(
ParentPermissionDialogView* view) override { ParentPermissionDialogView* view) override {
...@@ -511,13 +460,18 @@ class ExtensionWebstorePrivateApiTestChildInstallEnabled ...@@ -511,13 +460,18 @@ class ExtensionWebstorePrivateApiTestChildInstallEnabled
next_dialog_action_ = action; next_dialog_action_ = action;
} }
protected:
std::unique_ptr<signin::IdentityTestEnvironment> identity_test_env_;
private: private:
base::test::ScopedFeatureList feature_list_; // Create another embedded test server to avoid starting the same one twice.
std::unique_ptr<net::EmbeddedTestServer> embedded_test_server_;
chromeos::LoggedInUserMixin logged_in_user_mixin_;
base::Optional<NextDialogAction> next_dialog_action_; base::Optional<NextDialogAction> next_dialog_action_;
}; };
// Tests install for a child when parent permission is granted. // Tests install for a child when parent permission is granted.
IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTestChildInstallEnabled, IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTestChild,
ParentPermissionGranted) { ParentPermissionGranted) {
WebstoreInstallListener listener; WebstoreInstallListener listener;
WebstorePrivateApi::SetWebstoreInstallerDelegateForTesting(&listener); WebstorePrivateApi::SetWebstoreInstallerDelegateForTesting(&listener);
...@@ -543,7 +497,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTestChildInstallEnabled, ...@@ -543,7 +497,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTestChildInstallEnabled,
// Tests no install occurs for a child when the parent permission // Tests no install occurs for a child when the parent permission
// dialog is canceled. // dialog is canceled.
IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTestChildInstallEnabled, IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTestChild,
ParentPermissionCanceled) { ParentPermissionCanceled) {
WebstoreInstallListener listener; WebstoreInstallListener listener;
set_next_dialog_action(NextDialogAction::kCancel); set_next_dialog_action(NextDialogAction::kCancel);
...@@ -565,7 +519,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTestChildInstallEnabled, ...@@ -565,7 +519,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTestChildInstallEnabled,
} }
// Tests that no parent permission is required for a child to install a theme. // Tests that no parent permission is required for a child to install a theme.
IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTestChildInstallEnabled, IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTestChild,
NoParentPermissionRequiredForTheme) { NoParentPermissionRequiredForTheme) {
WebstoreInstallListener listener; WebstoreInstallListener listener;
WebstorePrivateApi::SetWebstoreInstallerDelegateForTesting(&listener); WebstorePrivateApi::SetWebstoreInstallerDelegateForTesting(&listener);
...@@ -578,7 +532,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTestChildInstallEnabled, ...@@ -578,7 +532,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTestChildInstallEnabled,
// Tests that even if the kSupervisedUserInitiatedExtensionInstall feature flag // Tests that even if the kSupervisedUserInitiatedExtensionInstall feature flag
// is enabled, supervised user extension installs are blocked if the // is enabled, supervised user extension installs are blocked if the
// "Permissions for sites, apps and extensions" toggle is off. // "Permissions for sites, apps and extensions" toggle is off.
IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTestChildInstallEnabled, IN_PROC_BROWSER_TEST_F(ExtensionWebstorePrivateApiTestChild,
InstallBlockedWhenPermissionsToggleOff) { InstallBlockedWhenPermissionsToggleOff) {
base::HistogramTester histogram_tester; base::HistogramTester histogram_tester;
base::UserActionTester user_action_tester; base::UserActionTester user_action_tester;
......
...@@ -3,11 +3,9 @@ ...@@ -3,11 +3,9 @@
// found in the LICENSE file. // found in the LICENSE file.
#include "base/files/file_path.h" #include "base/files/file_path.h"
#include "base/test/scoped_feature_list.h"
#include "chrome/browser/extensions/extension_browsertest.h" #include "chrome/browser/extensions/extension_browsertest.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/supervised_user/logged_in_user_mixin.h" #include "chrome/browser/supervised_user/logged_in_user_mixin.h"
#include "chrome/browser/supervised_user/supervised_user_features.h"
#include "chrome/browser/supervised_user/supervised_user_service.h" #include "chrome/browser/supervised_user/supervised_user_service.h"
#include "chrome/browser/supervised_user/supervised_user_service_factory.h" #include "chrome/browser/supervised_user/supervised_user_service_factory.h"
#include "chrome/test/base/mixin_based_in_process_browser_test.h" #include "chrome/test/base/mixin_based_in_process_browser_test.h"
...@@ -37,8 +35,6 @@ class SupervisedUserExtensionTest : public ExtensionBrowserTest { ...@@ -37,8 +35,6 @@ class SupervisedUserExtensionTest : public ExtensionBrowserTest {
// We have to essentially replicate what MixinBasedInProcessBrowserTest does // We have to essentially replicate what MixinBasedInProcessBrowserTest does
// here because ExtensionBrowserTest doesn't inherit from that class. // here because ExtensionBrowserTest doesn't inherit from that class.
void SetUp() override { void SetUp() override {
scoped_feature_list_.InitAndEnableFeature(
supervised_users::kSupervisedUserInitiatedExtensionInstall);
mixin_host_.SetUp(); mixin_host_.SetUp();
ExtensionBrowserTest::SetUp(); ExtensionBrowserTest::SetUp();
} }
...@@ -118,8 +114,6 @@ class SupervisedUserExtensionTest : public ExtensionBrowserTest { ...@@ -118,8 +114,6 @@ class SupervisedUserExtensionTest : public ExtensionBrowserTest {
content::IsPreTest() ? chromeos::LoggedInUserMixin::LogInType::kChild content::IsPreTest() ? chromeos::LoggedInUserMixin::LogInType::kChild
: chromeos::LoggedInUserMixin::LogInType::kRegular, : chromeos::LoggedInUserMixin::LogInType::kRegular,
embedded_test_server(), this}; embedded_test_server(), this};
base::test::ScopedFeatureList scoped_feature_list_;
}; };
// Removing supervision should also remove associated disable reasons, such as // Removing supervision should also remove associated disable reasons, such as
......
...@@ -6,14 +6,12 @@ ...@@ -6,14 +6,12 @@
#include "base/test/metrics/histogram_tester.h" #include "base/test/metrics/histogram_tester.h"
#include "base/test/metrics/user_action_tester.h" #include "base/test/metrics/user_action_tester.h"
#include "base/test/scoped_feature_list.h"
#include "chrome/browser/extensions/api/webstore_private/webstore_private_api.h" #include "chrome/browser/extensions/api/webstore_private/webstore_private_api.h"
#include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/extension_service_test_with_install.h" #include "chrome/browser/extensions/extension_service_test_with_install.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/supervised_user/supervised_user_constants.h" #include "chrome/browser/supervised_user/supervised_user_constants.h"
#include "chrome/browser/supervised_user/supervised_user_extensions_metrics_recorder.h" #include "chrome/browser/supervised_user/supervised_user_extensions_metrics_recorder.h"
#include "chrome/browser/supervised_user/supervised_user_features.h"
#include "chrome/browser/supervised_user/supervised_user_service.h" #include "chrome/browser/supervised_user/supervised_user_service.h"
#include "chrome/browser/supervised_user/supervised_user_service_factory.h" #include "chrome/browser/supervised_user/supervised_user_service_factory.h"
#include "chrome/common/pref_names.h" #include "chrome/common/pref_names.h"
...@@ -39,34 +37,6 @@ class SupervisedUserExtensionTest : public ExtensionServiceTestWithInstall { ...@@ -39,34 +37,6 @@ class SupervisedUserExtensionTest : public ExtensionServiceTestWithInstall {
SupervisedUserExtensionTest() = default; SupervisedUserExtensionTest() = default;
protected: protected:
// These enum values represent various feature flags for enabling supervised
// users to install extensions.
enum class ExtensionInstallMode {
// Turn off all the feature flags.
kNone,
// Refers to the full extensions feature where each install has be approved
// through the parent permissions dialog.
kFull
};
// Enables or disables features for allowing supervised users to install
// extensions.
void InitSupervisedUserExtensionInstallFeatures(ExtensionInstallMode mode) {
std::vector<base::Feature> enabled_features;
std::vector<base::Feature> disabled_features;
switch (mode) {
case ExtensionInstallMode::kNone:
disabled_features.push_back(
supervised_users::kSupervisedUserInitiatedExtensionInstall);
break;
case ExtensionInstallMode::kFull:
enabled_features.push_back(
supervised_users::kSupervisedUserInitiatedExtensionInstall);
break;
}
scoped_feature_list_.InitWithFeatures(enabled_features, disabled_features);
}
void SetSupervisedUserExtensionsMayRequestPermissionsPref(bool enabled) { void SetSupervisedUserExtensionsMayRequestPermissionsPref(bool enabled) {
supervised_user_service() supervised_user_service()
->SetSupervisedUserExtensionsMayRequestPermissionsPrefForTesting( ->SetSupervisedUserExtensionsMayRequestPermissionsPrefForTesting(
...@@ -198,22 +168,8 @@ class SupervisedUserExtensionTest : public ExtensionServiceTestWithInstall { ...@@ -198,22 +168,8 @@ class SupervisedUserExtensionTest : public ExtensionServiceTestWithInstall {
base::FilePath pem_path() const { base::FilePath pem_path() const {
return base_path().AppendASCII("permissions.pem"); return base_path().AppendASCII("permissions.pem");
} }
base::test::ScopedFeatureList scoped_feature_list_;
}; };
// Tests that extension installation is blocked for child accounts without any
// features.
TEST_F(SupervisedUserExtensionTest, InstallBlocked) {
InitSupervisedUserExtensionInstallFeatures(ExtensionInstallMode::kNone);
InitServices(/*profile_is_supervised=*/true);
base::FilePath path = data_dir().AppendASCII("good.crx");
const Extension* extension = InstallCRX(path, INSTALL_FAILED);
// The extension should not have been installed.
EXPECT_FALSE(extension);
}
// Tests that regular users are not affecting supervised user UMA metrics. // Tests that regular users are not affecting supervised user UMA metrics.
TEST_F(SupervisedUserExtensionTest, TEST_F(SupervisedUserExtensionTest,
RegularUsersNotAffectingSupervisedUserMetrics) { RegularUsersNotAffectingSupervisedUserMetrics) {
...@@ -240,7 +196,6 @@ TEST_F(SupervisedUserExtensionTest, ...@@ -240,7 +196,6 @@ TEST_F(SupervisedUserExtensionTest,
// unexpected behavior. // unexpected behavior.
TEST_F(SupervisedUserExtensionTest, TEST_F(SupervisedUserExtensionTest,
CustodianApprovalDoesNotAffectRegularUsers) { CustodianApprovalDoesNotAffectRegularUsers) {
InitSupervisedUserExtensionInstallFeatures(ExtensionInstallMode::kFull);
InitServices(/*profile_is_supervised=*/false); InitServices(/*profile_is_supervised=*/false);
SetSupervisedUserExtensionsMayRequestPermissionsPref(false); SetSupervisedUserExtensionsMayRequestPermissionsPref(false);
...@@ -262,7 +217,6 @@ TEST_F(SupervisedUserExtensionTest, ...@@ -262,7 +217,6 @@ TEST_F(SupervisedUserExtensionTest,
// Tests that adding supervision to a regular account (Gellerization) disables // Tests that adding supervision to a regular account (Gellerization) disables
// previously installed extensions. // previously installed extensions.
TEST_F(SupervisedUserExtensionTest, ExtensionsDisabledAfterGellerization) { TEST_F(SupervisedUserExtensionTest, ExtensionsDisabledAfterGellerization) {
InitSupervisedUserExtensionInstallFeatures(ExtensionInstallMode::kFull);
InitServices(/*profile_is_supervised=*/false); InitServices(/*profile_is_supervised=*/false);
SetSupervisedUserExtensionsMayRequestPermissionsPref(true); SetSupervisedUserExtensionsMayRequestPermissionsPref(true);
...@@ -299,7 +253,6 @@ TEST_F(SupervisedUserExtensionTest, ExtensionsDisabledAfterGellerization) { ...@@ -299,7 +253,6 @@ TEST_F(SupervisedUserExtensionTest, ExtensionsDisabledAfterGellerization) {
// newly-installed extensions are disabled until approved by the parent. // newly-installed extensions are disabled until approved by the parent.
TEST_F(SupervisedUserExtensionTest, TEST_F(SupervisedUserExtensionTest,
InstallAllowedButDisabledForSupervisedUser) { InstallAllowedButDisabledForSupervisedUser) {
InitSupervisedUserExtensionInstallFeatures(ExtensionInstallMode::kFull);
InitServices(/*profile_is_supervised=*/true); InitServices(/*profile_is_supervised=*/true);
SetSupervisedUserExtensionsMayRequestPermissionsPref(true); SetSupervisedUserExtensionsMayRequestPermissionsPref(true);
...@@ -327,7 +280,6 @@ TEST_F(SupervisedUserExtensionTest, ...@@ -327,7 +280,6 @@ TEST_F(SupervisedUserExtensionTest,
// Tests that supervised users may approve permission updates without parent // Tests that supervised users may approve permission updates without parent
// approval if kSupervisedUserExtensionsMayRequestPermissions is true. // approval if kSupervisedUserExtensionsMayRequestPermissions is true.
TEST_F(SupervisedUserExtensionTest, UpdateWithPermissionsIncrease) { TEST_F(SupervisedUserExtensionTest, UpdateWithPermissionsIncrease) {
InitSupervisedUserExtensionInstallFeatures(ExtensionInstallMode::kFull);
InitServices(/*profile_is_supervised=*/true); InitServices(/*profile_is_supervised=*/true);
SetSupervisedUserExtensionsMayRequestPermissionsPref(true); SetSupervisedUserExtensionsMayRequestPermissionsPref(true);
...@@ -404,7 +356,6 @@ TEST_F(SupervisedUserExtensionTest, UpdateWithPermissionsIncrease) { ...@@ -404,7 +356,6 @@ TEST_F(SupervisedUserExtensionTest, UpdateWithPermissionsIncrease) {
// cannot approve permission updates. // cannot approve permission updates.
TEST_F(SupervisedUserExtensionTest, TEST_F(SupervisedUserExtensionTest,
ChildUserCannotApproveAdditionalPermissions) { ChildUserCannotApproveAdditionalPermissions) {
InitSupervisedUserExtensionInstallFeatures(ExtensionInstallMode::kFull);
InitServices(/*profile_is_supervised=*/true); InitServices(/*profile_is_supervised=*/true);
// Keep the toggle on initially just to install the extension. // Keep the toggle on initially just to install the extension.
SetSupervisedUserExtensionsMayRequestPermissionsPref(true); SetSupervisedUserExtensionsMayRequestPermissionsPref(true);
...@@ -461,7 +412,6 @@ TEST_F(SupervisedUserExtensionTest, ...@@ -461,7 +412,6 @@ TEST_F(SupervisedUserExtensionTest,
// Tests that if an approved extension is updated to a newer version that // Tests that if an approved extension is updated to a newer version that
// doesn't require additional permissions, it is still enabled. // doesn't require additional permissions, it is still enabled.
TEST_F(SupervisedUserExtensionTest, UpdateWithoutPermissionIncrease) { TEST_F(SupervisedUserExtensionTest, UpdateWithoutPermissionIncrease) {
InitSupervisedUserExtensionInstallFeatures(ExtensionInstallMode::kFull);
InitServices(/*profile_is_supervised=*/true); InitServices(/*profile_is_supervised=*/true);
SetSupervisedUserExtensionsMayRequestPermissionsPref(true); SetSupervisedUserExtensionsMayRequestPermissionsPref(true);
...@@ -507,7 +457,6 @@ TEST_F(SupervisedUserExtensionTest, UpdateWithoutPermissionIncrease) { ...@@ -507,7 +457,6 @@ TEST_F(SupervisedUserExtensionTest, UpdateWithoutPermissionIncrease) {
// Tests that the kApprovalGranted UMA metric only increments once without // Tests that the kApprovalGranted UMA metric only increments once without
// duplication for the same extension id. // duplication for the same extension id.
TEST_F(SupervisedUserExtensionTest, DontTriggerMetricsIfAlreadyApproved) { TEST_F(SupervisedUserExtensionTest, DontTriggerMetricsIfAlreadyApproved) {
InitSupervisedUserExtensionInstallFeatures(ExtensionInstallMode::kFull);
InitServices(/*profile_is_supervised=*/true); InitServices(/*profile_is_supervised=*/true);
SetSupervisedUserExtensionsMayRequestPermissionsPref(true); SetSupervisedUserExtensionsMayRequestPermissionsPref(true);
...@@ -565,7 +514,6 @@ TEST_F(SupervisedUserExtensionTest, DontTriggerMetricsIfAlreadyApproved) { ...@@ -565,7 +514,6 @@ TEST_F(SupervisedUserExtensionTest, DontTriggerMetricsIfAlreadyApproved) {
// kSupervisedUserExtensionsMayRequestPermissions set to false, then child users // kSupervisedUserExtensionsMayRequestPermissions set to false, then child users
// cannot install new extensions. // cannot install new extensions.
TEST_F(SupervisedUserExtensionTest, SupervisedUserCannotInstallExtension) { TEST_F(SupervisedUserExtensionTest, SupervisedUserCannotInstallExtension) {
InitSupervisedUserExtensionInstallFeatures(ExtensionInstallMode::kFull);
InitServices(/*profile_is_supervised=*/true); InitServices(/*profile_is_supervised=*/true);
SetSupervisedUserExtensionsMayRequestPermissionsPref(false); SetSupervisedUserExtensionsMayRequestPermissionsPref(false);
...@@ -578,7 +526,6 @@ TEST_F(SupervisedUserExtensionTest, SupervisedUserCannotInstallExtension) { ...@@ -578,7 +526,6 @@ TEST_F(SupervisedUserExtensionTest, SupervisedUserCannotInstallExtension) {
// Tests that disabling the "Permissions for sites, apps and extensions" toggle // Tests that disabling the "Permissions for sites, apps and extensions" toggle
// has no effect on regular users. // has no effect on regular users.
TEST_F(SupervisedUserExtensionTest, RegularUserCanInstallExtension) { TEST_F(SupervisedUserExtensionTest, RegularUserCanInstallExtension) {
InitSupervisedUserExtensionInstallFeatures(ExtensionInstallMode::kFull);
InitServices(/*profile_is_supervised=*/false); InitServices(/*profile_is_supervised=*/false);
SetSupervisedUserExtensionsMayRequestPermissionsPref(false); SetSupervisedUserExtensionsMayRequestPermissionsPref(false);
...@@ -594,7 +541,6 @@ TEST_F(SupervisedUserExtensionTest, RegularUserCanInstallExtension) { ...@@ -594,7 +541,6 @@ TEST_F(SupervisedUserExtensionTest, RegularUserCanInstallExtension) {
// kSupervisedUserExtensionsMayRequestPermissions set to false, previously // kSupervisedUserExtensionsMayRequestPermissions set to false, previously
// approved extensions are still enabled. // approved extensions are still enabled.
TEST_F(SupervisedUserExtensionTest, ToggleOffDoesNotAffectAlreadyEnabled) { TEST_F(SupervisedUserExtensionTest, ToggleOffDoesNotAffectAlreadyEnabled) {
InitSupervisedUserExtensionInstallFeatures(ExtensionInstallMode::kFull);
InitServices(/*profile_is_supervised=*/true); InitServices(/*profile_is_supervised=*/true);
SetSupervisedUserExtensionsMayRequestPermissionsPref(true); SetSupervisedUserExtensionsMayRequestPermissionsPref(true);
...@@ -619,7 +565,6 @@ TEST_F(SupervisedUserExtensionTest, ToggleOffDoesNotAffectAlreadyEnabled) { ...@@ -619,7 +565,6 @@ TEST_F(SupervisedUserExtensionTest, ToggleOffDoesNotAffectAlreadyEnabled) {
// Tests the case when the extension approval arrives through sync before the // Tests the case when the extension approval arrives through sync before the
// extension itself is installed. // extension itself is installed.
TEST_F(SupervisedUserExtensionTest, ExtensionApprovalBeforeInstallation) { TEST_F(SupervisedUserExtensionTest, ExtensionApprovalBeforeInstallation) {
InitSupervisedUserExtensionInstallFeatures(ExtensionInstallMode::kFull);
InitServices(/*profile_is_supervised=*/true); InitServices(/*profile_is_supervised=*/true);
SetSupervisedUserExtensionsMayRequestPermissionsPref(true); SetSupervisedUserExtensionsMayRequestPermissionsPref(true);
...@@ -638,7 +583,6 @@ TEST_F(SupervisedUserExtensionTest, ExtensionApprovalBeforeInstallation) { ...@@ -638,7 +583,6 @@ TEST_F(SupervisedUserExtensionTest, ExtensionApprovalBeforeInstallation) {
// extensions when both disable reasons custodian_approval_required and // extensions when both disable reasons custodian_approval_required and
// permissions_increase are present. // permissions_increase are present.
TEST_F(SupervisedUserExtensionTest, ParentApprovalNecessaryButNotSufficient) { TEST_F(SupervisedUserExtensionTest, ParentApprovalNecessaryButNotSufficient) {
InitSupervisedUserExtensionInstallFeatures(ExtensionInstallMode::kFull);
InitServices(/*profile_is_supervised=*/true); InitServices(/*profile_is_supervised=*/true);
SetSupervisedUserExtensionsMayRequestPermissionsPref(true); SetSupervisedUserExtensionsMayRequestPermissionsPref(true);
......
...@@ -11,10 +11,6 @@ namespace supervised_users { ...@@ -11,10 +11,6 @@ namespace supervised_users {
const base::Feature kSupervisedUserIframeFilter{ const base::Feature kSupervisedUserIframeFilter{
"SupervisedUserIframeFilter", base::FEATURE_ENABLED_BY_DEFAULT}; "SupervisedUserIframeFilter", base::FEATURE_ENABLED_BY_DEFAULT};
const base::Feature kSupervisedUserInitiatedExtensionInstall{
"SupervisedUserInitiatedExtensionInstall",
base::FEATURE_ENABLED_BY_DEFAULT};
const base::Feature kEduCoexistenceFlowV2{"EduCoexistenceV2", const base::Feature kEduCoexistenceFlowV2{"EduCoexistenceV2",
base::FEATURE_DISABLED_BY_DEFAULT}; base::FEATURE_DISABLED_BY_DEFAULT};
......
...@@ -11,8 +11,6 @@ namespace supervised_users { ...@@ -11,8 +11,6 @@ namespace supervised_users {
extern const base::Feature kSupervisedUserIframeFilter; extern const base::Feature kSupervisedUserIframeFilter;
extern const base::Feature kSupervisedUserInitiatedExtensionInstall;
extern const base::Feature kEduCoexistenceFlowV2; extern const base::Feature kEduCoexistenceFlowV2;
bool IsEduCoexistenceFlowV2Enabled(); bool IsEduCoexistenceFlowV2Enabled();
......
...@@ -281,11 +281,6 @@ bool SupervisedUserService::IsChild() const { ...@@ -281,11 +281,6 @@ bool SupervisedUserService::IsChild() const {
return profile_->IsChild(); return profile_->IsChild();
} }
bool SupervisedUserService::IsSupervisedUserExtensionInstallEnabled() const {
return base::FeatureList::IsEnabled(
supervised_users::kSupervisedUserInitiatedExtensionInstall);
}
bool SupervisedUserService::HasACustodian() const { bool SupervisedUserService::HasACustodian() const {
return !GetCustodianEmailAddress().empty() || return !GetCustodianEmailAddress().empty() ||
!GetSecondCustodianEmailAddress().empty(); !GetSecondCustodianEmailAddress().empty();
...@@ -389,7 +384,7 @@ void SupervisedUserService:: ...@@ -389,7 +384,7 @@ void SupervisedUserService::
} }
bool SupervisedUserService::CanInstallExtensions() const { bool SupervisedUserService::CanInstallExtensions() const {
return IsSupervisedUserExtensionInstallEnabled() && HasACustodian() && return HasACustodian() &&
GetSupervisedUserExtensionsMayRequestPermissionsPref(); GetSupervisedUserExtensionsMayRequestPermissionsPref();
} }
...@@ -791,12 +786,6 @@ SupervisedUserService::ExtensionState SupervisedUserService::GetExtensionState( ...@@ -791,12 +786,6 @@ SupervisedUserService::ExtensionState SupervisedUserService::GetExtensionState(
return ExtensionState::ALLOWED; return ExtensionState::ALLOWED;
} }
// Feature flag for gating new behavior.
if (!base::FeatureList::IsEnabled(
supervised_users::kSupervisedUserInitiatedExtensionInstall)) {
return ExtensionState::BLOCKED;
}
if (ShouldBlockExtension(extension.id())) { if (ShouldBlockExtension(extension.id())) {
return ExtensionState::BLOCKED; return ExtensionState::BLOCKED;
} }
......
...@@ -15,7 +15,6 @@ ...@@ -15,7 +15,6 @@
#include "base/path_service.h" #include "base/path_service.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/test/scoped_feature_list.h"
#include "base/threading/thread_task_runner_handle.h" #include "base/threading/thread_task_runner_handle.h"
#include "base/values.h" #include "base/values.h"
#include "build/build_config.h" #include "build/build_config.h"
...@@ -23,7 +22,6 @@ ...@@ -23,7 +22,6 @@
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/signin/identity_test_environment_profile_adaptor.h" #include "chrome/browser/signin/identity_test_environment_profile_adaptor.h"
#include "chrome/browser/supervised_user/permission_request_creator.h" #include "chrome/browser/supervised_user/permission_request_creator.h"
#include "chrome/browser/supervised_user/supervised_user_features.h"
#include "chrome/browser/supervised_user/supervised_user_service_factory.h" #include "chrome/browser/supervised_user/supervised_user_service_factory.h"
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
...@@ -347,17 +345,9 @@ class SupervisedUserServiceExtensionTestBase ...@@ -347,17 +345,9 @@ class SupervisedUserServiceExtensionTestBase
return extension; return extension;
} }
void InitSupervisedUserInitiatedExtensionInstallFeature(bool enabled) {
if (enabled) {
scoped_feature_list_.InitAndEnableFeature(
supervised_users::kSupervisedUserInitiatedExtensionInstall);
}
}
bool is_supervised_; bool is_supervised_;
extensions::ScopedCurrentChannel channel_; extensions::ScopedCurrentChannel channel_;
SupervisedUserURLFilterObserver url_filter_observer_; SupervisedUserURLFilterObserver url_filter_observer_;
base::test::ScopedFeatureList scoped_feature_list_;
}; };
class SupervisedUserServiceExtensionTestUnsupervised class SupervisedUserServiceExtensionTestUnsupervised
...@@ -376,8 +366,6 @@ class SupervisedUserServiceExtensionTest ...@@ -376,8 +366,6 @@ class SupervisedUserServiceExtensionTest
TEST_F(SupervisedUserServiceExtensionTest, TEST_F(SupervisedUserServiceExtensionTest,
ExtensionManagementPolicyProviderWithoutSUInitiatedInstalls) { ExtensionManagementPolicyProviderWithoutSUInitiatedInstalls) {
InitSupervisedUserInitiatedExtensionInstallFeature(true);
SupervisedUserService* supervised_user_service = SupervisedUserService* supervised_user_service =
SupervisedUserServiceFactory::GetForProfile(profile_.get()); SupervisedUserServiceFactory::GetForProfile(profile_.get());
supervised_user_service supervised_user_service
...@@ -429,12 +417,10 @@ TEST_F(SupervisedUserServiceExtensionTest, ...@@ -429,12 +417,10 @@ TEST_F(SupervisedUserServiceExtensionTest,
TEST_F(SupervisedUserServiceExtensionTest, TEST_F(SupervisedUserServiceExtensionTest,
ExtensionManagementPolicyProviderWithSUInitiatedInstalls) { ExtensionManagementPolicyProviderWithSUInitiatedInstalls) {
// Enable child users to initiate extension installs by simulating the
// toggling of "Permissions for sites and apps" to enabled.
InitSupervisedUserInitiatedExtensionInstallFeature(true);
SupervisedUserService* supervised_user_service = SupervisedUserService* supervised_user_service =
SupervisedUserServiceFactory::GetForProfile(profile_.get()); SupervisedUserServiceFactory::GetForProfile(profile_.get());
// Enable child users to initiate extension installs by simulating the
// toggling of "Permissions for sites, apps and extensions" to enabled.
supervised_user_service supervised_user_service
->SetSupervisedUserExtensionsMayRequestPermissionsPrefForTesting(true); ->SetSupervisedUserExtensionsMayRequestPermissionsPrefForTesting(true);
EXPECT_TRUE(supervised_user_service EXPECT_TRUE(supervised_user_service
......
...@@ -13,7 +13,6 @@ ...@@ -13,7 +13,6 @@
#include "base/strings/utf_string_conversions.h" #include "base/strings/utf_string_conversions.h"
#include "base/test/metrics/histogram_tester.h" #include "base/test/metrics/histogram_tester.h"
#include "base/test/metrics/user_action_tester.h" #include "base/test/metrics/user_action_tester.h"
#include "base/test/scoped_feature_list.h"
#include "chrome/browser/chromeos/login/test/fake_gaia_mixin.h" #include "chrome/browser/chromeos/login/test/fake_gaia_mixin.h"
#include "chrome/browser/extensions/chrome_test_extension_loader.h" #include "chrome/browser/extensions/chrome_test_extension_loader.h"
#include "chrome/browser/extensions/extension_service.h" #include "chrome/browser/extensions/extension_service.h"
...@@ -21,7 +20,6 @@ ...@@ -21,7 +20,6 @@
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/supervised_user/logged_in_user_mixin.h" #include "chrome/browser/supervised_user/logged_in_user_mixin.h"
#include "chrome/browser/supervised_user/supervised_user_extensions_metrics_recorder.h" #include "chrome/browser/supervised_user/supervised_user_extensions_metrics_recorder.h"
#include "chrome/browser/supervised_user/supervised_user_features.h"
#include "chrome/browser/supervised_user/supervised_user_service.h" #include "chrome/browser/supervised_user/supervised_user_service.h"
#include "chrome/browser/supervised_user/supervised_user_service_factory.h" #include "chrome/browser/supervised_user/supervised_user_service_factory.h"
#include "chrome/browser/supervised_user/supervised_user_test_util.h" #include "chrome/browser/supervised_user/supervised_user_test_util.h"
...@@ -60,12 +58,7 @@ class ParentPermissionDialogViewTest ...@@ -60,12 +58,7 @@ class ParentPermissionDialogViewTest
}; };
ParentPermissionDialogViewTest() ParentPermissionDialogViewTest()
: TestParentPermissionDialogViewObserver(this) { : TestParentPermissionDialogViewObserver(this) {}
feature_list_.InitWithFeatures(
/*enabled_features=*/{supervised_users::
kSupervisedUserInitiatedExtensionInstall},
/*disabled_features=*/{});
}
ParentPermissionDialogViewTest(const ParentPermissionDialogViewTest&) = ParentPermissionDialogViewTest(const ParentPermissionDialogViewTest&) =
delete; delete;
...@@ -278,8 +271,6 @@ class ParentPermissionDialogViewTest ...@@ -278,8 +271,6 @@ class ParentPermissionDialogViewTest
std::unique_ptr<signin::IdentityTestEnvironment> identity_test_env_; std::unique_ptr<signin::IdentityTestEnvironment> identity_test_env_;
base::Optional<NextDialogAction> next_dialog_action_; base::Optional<NextDialogAction> next_dialog_action_;
base::test::ScopedFeatureList feature_list_;
}; };
// Tests that a plain dialog widget is shown using the TestBrowserUi // Tests that a plain dialog widget is shown using the TestBrowserUi
......
...@@ -37,10 +37,9 @@ class SupervisedUserExtensionsDelegate { ...@@ -37,10 +37,9 @@ class SupervisedUserExtensionsDelegate {
const extensions::Extension& extension, const extensions::Extension& extension,
content::BrowserContext* context) const = 0; content::BrowserContext* context) const = 0;
// If the current user is a child, the child user has a custodian/parent, the // If the current user is a child, the child user has a custodian/parent, and
// kSupervisedUserInitiatedExtensionInstall feature flag is enabled, and the // the parent has enabled the "Permissions for sites, apps and extensions"
// parent has enabled the "Permissions for sites, apps and extensions" toggle, // toggle, then display the Parent Permission Dialog and call
// then display the Parent Permission Dialog and call
// |parent_permission_callback|. Otherwise, display the Extension Install // |parent_permission_callback|. Otherwise, display the Extension Install
// Blocked by Parent Dialog and call |error_callback|. The two paths are // Blocked by Parent Dialog and call |error_callback|. The two paths are
// mutually exclusive. // mutually exclusive.
......
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