[iOS Enterprise] BrowserSignin: Enable policy handler by default

Installs the BrowserSignin policy handler by default. It was previously
hidden behind the --install-browser-signin-handler command-line flag
while the different parts of the implementation were being finished. The
policy now fully works, so the command-line flag is no longer needed.

Bug: 1155745
Change-Id: I58dda5bdc143cbfbd21cecbea13d184bc60100ec
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2633352Reviewed-by: Nohemi Fernandez <fernandex@chromium.org>
Reviewed-by: Sylvain Defresne <sdefresne@chromium.org>
Commit-Queue: Guillaume Jenkins <gujen@google.com>
Cr-Commit-Position: refs/heads/master@{#844766}

ios/chrome/browser/chrome_switches.cc

@@ -39,9 +39,6 @@ const char kEnableSpotlightActions[] = "enable-spotlight-actions";
 const char kEnableThirdPartyKeyboardWorkaround[] =
     "enable-third-party-keyboard-workaround";
 
-// Installs the BrowserSignin policy handler.
-const char kInstallBrowserSigninHandler[] = "install-browser-signin-handler";
-
 // Installs the URLBlocklist and URLAllowlist handlers.
 const char kInstallURLBlocklistHandlers[] = "install-url-blocklist-handlers";
 

ios/chrome/browser/chrome_switches.h

@@ -17,7 +17,6 @@ extern const char kEnableEnterprisePolicy[];
 extern const char kEnableIOSHandoffToOtherDevices[];
 extern const char kEnableSpotlightActions[];
 extern const char kEnableThirdPartyKeyboardWorkaround[];
-extern const char kInstallBrowserSigninHandler[];
 extern const char kInstallURLBlocklistHandlers[];
 
 extern const char kUserAgent[];

ios/chrome/browser/policy/configuration_policy_handler_list_factory.mm

@@ -124,6 +124,8 @@ std::unique_ptr<policy::ConfigurationPolicyHandlerList> BuildPolicyHandlerList(
       std::make_unique<autofill::AutofillAddressPolicyHandler>());
   handlers->AddHandler(
       std::make_unique<autofill::AutofillCreditCardPolicyHandler>());
+  handlers->AddHandler(
+      std::make_unique<policy::BrowserSigninPolicyHandler>(chrome_schema));
   handlers->AddHandler(std::make_unique<policy::DefaultSearchPolicyHandler>());
   handlers->AddHandler(
       std::make_unique<safe_browsing::SafeBrowsingPolicyHandler>());
@@ -131,11 +133,6 @@ std::unique_ptr<policy::ConfigurationPolicyHandlerList> BuildPolicyHandlerList(
       std::make_unique<bookmarks::ManagedBookmarksPolicyHandler>(
           chrome_schema));
 
-  if (ShouldInstallBrowserSigninPolicyHandler()) {
-    handlers->AddHandler(
-        std::make_unique<policy::BrowserSigninPolicyHandler>(chrome_schema));
-  }
-
   if (ShouldInstallURLBlocklistPolicyHandlers()) {
     handlers->AddHandler(std::make_unique<policy::URLBlocklistPolicyHandler>(
         policy::key::kURLBlocklist));

ios/chrome/browser/policy/policy_features.cc

@@ -63,10 +63,6 @@ bool IsManagedBookmarksEnabled() {
   return base::FeatureList::IsEnabled(kManagedBookmarksIOS);
 }
 
-bool ShouldInstallBrowserSigninPolicyHandler() {
-  return HasSwitch(switches::kInstallBrowserSigninHandler);
-}
-
 bool ShouldInstallURLBlocklistPolicyHandlers() {
   return HasSwitch(switches::kInstallURLBlocklistHandlers);
 }

ios/chrome/browser/policy/policy_features.h

@@ -39,10 +39,6 @@ bool IsIncognitoModeAvailable();
 // policy data and make it user visible.
 bool ShouldInstallEnterprisePolicyHandlers();
 
-// Returns true if the BrowserSignin policy handler should be installed to
-// parse policy data and make it user visible.
-bool ShouldInstallBrowserSigninPolicyHandler();
-
 // Returns true if the ManagedBookmarks policy handler should be installed to
 // parse policy data and make it user visible.
 bool ShouldInstallManagedBookmarksPolicyHandler();

ios/chrome/browser/policy/policy_unittest.mm

@@ -37,8 +37,6 @@ class PolicyTest : public PlatformTest {
   PolicyTest() {
     base::CommandLine::ForCurrentProcess()->AppendSwitch(
         switches::kEnableEnterprisePolicy);
-    base::CommandLine::ForCurrentProcess()->AppendSwitch(
-        switches::kInstallBrowserSigninHandler);
     base::CommandLine::ForCurrentProcess()->AppendSwitch(
         switches::kInstallURLBlocklistHandlers);
   }

ios/chrome/browser/policy/policy_watcher_browser_agent.mm

@@ -10,7 +10,6 @@
 #include "components/prefs/pref_service.h"
 #include "components/signin/public/base/signin_pref_names.h"
 #import "ios/chrome/browser/browser_state/chrome_browser_state.h"
-#import "ios/chrome/browser/policy/policy_features.h"
 #import "ios/chrome/browser/ui/commands/command_dispatcher.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
@@ -34,8 +33,7 @@ void PolicyWatcherBrowserAgent::SetApplicationCommandsHandler(
   // BrowserSignin policy: start observing the kSigninAllowed pref for non-OTR
   // browsers. When the pref becomes false, send a UI command to sign the user
   // out. This requires the given command dispatcher to be fully configured.
-  if (!ShouldInstallBrowserSigninPolicyHandler() ||
-      browser_->GetBrowserState()->IsOffTheRecord()) {
+  if (browser_->GetBrowserState()->IsOffTheRecord()) {
     return;
   }
   prefs_change_observer_->Add(

ios/chrome/browser/policy/policy_watcher_browser_agent_unittest.mm

@@ -4,14 +4,12 @@
 
 #include "ios/chrome/browser/policy/policy_watcher_browser_agent.h"
 
-#import "base/test/scoped_command_line.h"
 #import "components/pref_registry/pref_registry_syncable.h"
 #import "components/prefs/pref_service.h"
 #import "components/signin/public/base/signin_pref_names.h"
 #import "components/sync_preferences/pref_service_mock_factory.h"
 #import "components/sync_preferences/pref_service_syncable.h"
 #import "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
-#import "ios/chrome/browser/chrome_switches.h"
 #import "ios/chrome/browser/main/test_browser.h"
 #import "ios/chrome/browser/prefs/browser_prefs.h"
 #import "ios/chrome/browser/ui/commands/application_commands.h"
@@ -57,10 +55,7 @@ class PolicyWatcherBrowserAgentTest : public PlatformTest {
 // Tests that the browser agent monitors the kSigninAllowed pref and dispatches
 // the appropriate command when the pref becomes false.
 TEST_F(PolicyWatcherBrowserAgentTest, observesSigninAllowed) {
-  // Set up the policy handler and the initial pref value.
-  base::test::ScopedCommandLine scoped_command_line;
-  scoped_command_line.GetProcessCommandLine()->AppendSwitch(
-      switches::kInstallBrowserSigninHandler);
+  // Set the initial pref value.
   chrome_browser_state_->GetPrefs()->SetBoolean(prefs::kSigninAllowed, true);
 
   // Set up the test browser and attach the browser agent under test.

ios/chrome/browser/ui/authentication/signin/BUILD.gn

@@ -48,7 +48,6 @@ source_set("signin_impl") {
     "//components/version_info",
     "//ios/chrome/app:tests_hook",
     "//ios/chrome/browser/main:public",
-    "//ios/chrome/browser/policy:feature_flags",
     "//ios/chrome/browser/signin",
     "//ios/chrome/browser/ui/authentication/signin/add_account_signin",
     "//ios/chrome/browser/ui/authentication/signin/advanced_settings_signin",
@@ -68,11 +67,9 @@ source_set("unit_tests") {
   deps = [
     ":signin_impl",
     "//base",
-    "//base/test:test_support",
     "//components/pref_registry",
     "//components/sync_preferences",
     "//components/sync_preferences:test_support",
-    "//ios/chrome/browser:utils",
     "//ios/chrome/browser/browser_state:test_support",
     "//ios/chrome/browser/main:test_support",
     "//ios/chrome/browser/prefs:browser_prefs",

ios/chrome/browser/ui/authentication/signin/signin_utils.mm

@@ -13,7 +13,6 @@
 #import "ios/chrome/app/tests_hook.h"
 #import "ios/chrome/browser/browser_state/chrome_browser_state.h"
 #import "ios/chrome/browser/main/browser.h"
-#import "ios/chrome/browser/policy/policy_features.h"
 #import "ios/chrome/browser/signin/authentication_service.h"
 #import "ios/chrome/browser/signin/authentication_service_factory.h"
 #import "ios/chrome/browser/ui/authentication/signin/user_signin/user_signin_constants.h"
@@ -134,9 +133,7 @@ void SetCurrentVersionForTesting(Version* version) {
 }
 
 bool IsSigninAllowed(const PrefService* prefs) {
-  // Sign-in is always allowed if the policy handler isn't installed.
-  return !ShouldInstallBrowserSigninPolicyHandler() ||
-         prefs->GetBoolean(prefs::kSigninAllowed);
+  return prefs->GetBoolean(prefs::kSigninAllowed);
 }
 
 }  // namespace signin

ios/chrome/browser/ui/authentication/signin/signin_utils_unittest.mm

@@ -9,15 +9,12 @@
 #include <memory>
 
 #import "base/bind.h"
-#import "base/command_line.h"
-#import "base/test/scoped_command_line.h"
 #import "base/version.h"
 #import "components/pref_registry/pref_registry_syncable.h"
 #import "components/signin/public/base/signin_pref_names.h"
 #import "components/sync_preferences/pref_service_mock_factory.h"
 #import "components/sync_preferences/pref_service_syncable.h"
 #import "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
-#import "ios/chrome/browser/chrome_switches.h"
 #import "ios/chrome/browser/main/test_browser.h"
 #import "ios/chrome/browser/prefs/browser_prefs.h"
 #import "ios/chrome/browser/signin/authentication_service_factory.h"
@@ -243,10 +240,6 @@ TEST_F(SigninUtilsTest, TestWillNotShowNewAccountUntilTwoVersionBis) {
 
 // Should not show the sign-in upgrade if sign-in is disabled by policy.
 TEST_F(SigninUtilsTest, TestWillNotShowIfDisabledByPolicy) {
-  base::test::ScopedCommandLine scoped_command_line;
-  scoped_command_line.GetProcessCommandLine()->AppendSwitch(
-      switches::kInstallBrowserSigninHandler);
-
   ios::FakeChromeIdentityService::GetInstanceFromChromeProvider()
       ->AddIdentities(@[ @"foo1" ]);
   chrome_browser_state_->GetPrefs()->SetBoolean(prefs::kSigninAllowed, false);
@@ -257,10 +250,6 @@ TEST_F(SigninUtilsTest, TestWillNotShowIfDisabledByPolicy) {
 
 // signin::IsSigninAllowed should respect the kSigninAllowed pref.
 TEST_F(SigninUtilsTest, TestSigninAllowedPref) {
-  base::test::ScopedCommandLine scoped_command_line;
-  scoped_command_line.GetProcessCommandLine()->AppendSwitch(
-      switches::kInstallBrowserSigninHandler);
-
   // Sign-in is allowed by default.
   EXPECT_TRUE(signin::IsSigninAllowed(chrome_browser_state_.get()->GetPrefs()));
 

ios/chrome/browser/ui/authentication/signin_promo_view_mediator_unittest.mm

@@ -7,7 +7,6 @@
 #include "base/run_loop.h"
 #include "base/strings/sys_string_conversions.h"
 #import "base/test/ios/wait_util.h"
-#import "base/test/scoped_command_line.h"
 #import "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
 #include "components/signin/public/base/signin_metrics.h"
@@ -16,7 +15,6 @@
 #import "components/sync_preferences/pref_service_mock_factory.h"
 #import "components/sync_preferences/pref_service_syncable.h"
 #import "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
-#import "ios/chrome/browser/chrome_switches.h"
 #import "ios/chrome/browser/prefs/browser_prefs.h"
 #import "ios/chrome/browser/signin/authentication_service_factory.h"
 #import "ios/chrome/browser/signin/authentication_service_fake.h"
@@ -420,9 +418,6 @@ TEST_F(SigninPromoViewMediatorTest,
 // Tests that promos aren't shown if browser sign-in is disabled by policy
 TEST_F(SigninPromoViewMediatorTest,
        ShouldNotDisplaySigninPromoViewIfDisabledByPolicy) {
-  base::test::ScopedCommandLine scoped_command_line;
-  scoped_command_line.GetProcessCommandLine()->AppendSwitch(
-      switches::kInstallBrowserSigninHandler);
   CreateMediator(signin_metrics::AccessPoint::ACCESS_POINT_RECENT_TABS);
   TestChromeBrowserState::Builder builder;
   builder.SetPrefService(CreatePrefService());

ios/chrome/browser/ui/recent_tabs/BUILD.gn

@@ -76,7 +76,6 @@ source_set("recent_tabs_ui") {
     "//base",
     "//components/prefs",
     "//components/sessions",
-    "//components/signin/public/base",
     "//components/strings",
     "//components/sync",
     "//ios/chrome/app:tests_hook",
@@ -85,7 +84,6 @@ source_set("recent_tabs_ui") {
     "//ios/chrome/browser/drag_and_drop",
     "//ios/chrome/browser/main:public",
     "//ios/chrome/browser/metrics:metrics_internal",
-    "//ios/chrome/browser/policy:feature_flags",
     "//ios/chrome/browser/sessions",
     "//ios/chrome/browser/sessions:serialisation",
     "//ios/chrome/browser/sync",
@@ -93,6 +91,7 @@ source_set("recent_tabs_ui") {
     "//ios/chrome/browser/ui/alert_coordinator",
     "//ios/chrome/browser/ui/authentication",
     "//ios/chrome/browser/ui/authentication/cells",
+    "//ios/chrome/browser/ui/authentication/signin:signin_headers",
     "//ios/chrome/browser/ui/commands",
     "//ios/chrome/browser/ui/settings/sync/utils",
     "//ios/chrome/browser/ui/table_view",

ios/chrome/browser/ui/recent_tabs/recent_tabs_table_view_controller.mm

@@ -14,7 +14,6 @@
 #include "base/strings/sys_string_conversions.h"
 #include "components/prefs/pref_service.h"
 #include "components/sessions/core/tab_restore_service.h"
-#include "components/signin/public/base/signin_pref_names.h"
 #include "components/strings/grit/components_strings.h"
 #include "components/sync_sessions/open_tabs_ui_delegate.h"
 #include "components/sync_sessions/session_sync_service.h"
@@ -24,7 +23,6 @@
 #import "ios/chrome/browser/drag_and_drop/table_view_url_drag_drop_handler.h"
 #import "ios/chrome/browser/main/browser.h"
 #import "ios/chrome/browser/metrics/new_tab_page_uma.h"
-#include "ios/chrome/browser/policy/policy_features.h"
 #include "ios/chrome/browser/sessions/live_tab_context_browser_agent.h"
 #include "ios/chrome/browser/sessions/session_util.h"
 #include "ios/chrome/browser/sync/session_sync_service_factory.h"
@@ -32,6 +30,7 @@
 #import "ios/chrome/browser/ui/authentication/cells/signin_promo_view_configurator.h"
 #import "ios/chrome/browser/ui/authentication/cells/signin_promo_view_consumer.h"
 #import "ios/chrome/browser/ui/authentication/cells/table_view_signin_promo_item.h"
+#import "ios/chrome/browser/ui/authentication/signin/signin_utils.h"
 #import "ios/chrome/browser/ui/authentication/signin_promo_view_mediator.h"
 #include "ios/chrome/browser/ui/commands/application_commands.h"
 #import "ios/chrome/browser/ui/commands/open_new_tab_command.h"
@@ -498,8 +497,7 @@ API_AVAILABLE(ios(13.0))
         [self.tableViewModel sectionIsCollapsed:SectionIdentifierOtherDevices];
   }
 
-  if (ShouldInstallBrowserSigninPolicyHandler() &&
-      !self.browserState->GetPrefs()->GetBoolean(prefs::kSigninAllowed)) {
+  if (!signin::IsSigninAllowed(self.browserState->GetPrefs())) {
     // If sign-in is disabled, don't show an illustration or a sign-in promo.
     TableViewTextItem* disabledByOrganizationText =
         [[TableViewTextItem alloc] initWithType:ItemTypeSigninDisabled];

ios/chrome/browser/ui/settings/google_services/BUILD.gn

@@ -147,7 +147,6 @@ source_set("eg2_tests") {
     "//components/safe_browsing/core/common:safe_browsing_prefs",
     "//components/signin/public/base",
     "//ios/chrome/app/strings",
-    "//ios/chrome/browser:utils",
     "//ios/chrome/browser/ui:feature_flags",
     "//ios/chrome/browser/ui/authentication:eg_test_support+eg2",
     "//ios/chrome/browser/ui/bookmarks:eg_test_support+eg2",

ios/chrome/browser/ui/settings/google_services/google_services_settings_egtest.mm

@@ -5,7 +5,6 @@
 #include "components/password_manager/core/common/password_manager_pref_names.h"
 #include "components/safe_browsing/core/common/safe_browsing_prefs.h"
 #include "components/signin/public/base/signin_pref_names.h"
-#import "ios/chrome/browser/chrome_switches.h"
 #import "ios/chrome/browser/ui/authentication/signin_earl_grey.h"
 #import "ios/chrome/browser/ui/authentication/signin_earl_grey_ui.h"
 #import "ios/chrome/browser/ui/settings/google_services/google_services_settings_app_interface.h"
@@ -48,16 +47,6 @@ using chrome_test_util::SyncSettingsConfirmButton;
 
 @implementation GoogleServicesSettingsTestCase
 
-- (AppLaunchConfiguration)appConfigurationForTestCase {
-  // Adds the command-line switch to enable support for the BrowserSignin
-  // policy.
-  AppLaunchConfiguration config;
-  config.additional_args.push_back(std::string("--") +
-                                   switches::kInstallBrowserSigninHandler);
-  config.relaunch_policy = NoForceRelaunchAndResetState;
-  return config;
-}
-
 // Opens the Google services settings view, and closes it.
 - (void)testOpenGoogleServicesSettings {
   [self openGoogleServicesSettings];

ios/chrome/browser/ui/settings/settings_table_view_controller_unittest.mm

@@ -4,7 +4,6 @@
 
 #import "ios/chrome/browser/ui/settings/settings_table_view_controller.h"
 
-#import "base/test/scoped_command_line.h"
 #import "base/test/task_environment.h"
 #import "components/pref_registry/pref_registry_syncable.h"
 #import "components/prefs/pref_service.h"
@@ -13,7 +12,6 @@
 #import "components/sync_preferences/pref_service_mock_factory.h"
 #import "components/sync_preferences/pref_service_syncable.h"
 #import "ios/chrome/browser/browser_state/test_chrome_browser_state.h"
-#import "ios/chrome/browser/chrome_switches.h"
 #import "ios/chrome/browser/main/test_browser.h"
 #import "ios/chrome/browser/prefs/browser_prefs.h"
 #import "ios/chrome/browser/search_engines/template_url_service_factory.h"
@@ -179,9 +177,6 @@ TEST_F(SettingsTableViewControllerTest, SyncOn) {
 // Verifies that the sign-in setting item is replaced by the managed sign-in
 // item if sign-in is disabled by policy.
 TEST_F(SettingsTableViewControllerTest, SigninDisabled) {
-  base::test::ScopedCommandLine scoped_command_line;
-  scoped_command_line.GetProcessCommandLine()->AppendSwitch(
-      switches::kInstallBrowserSigninHandler);
   chrome_browser_state_->GetPrefs()->SetBoolean(prefs::kSigninAllowed, false);
   CreateController();
   CheckController();