Move SigninManager::IsSignoutProhibited to SigninClient

The old SigninManager::IsSignoutProhibited() had a few smells
1. callers of SM::SignOut() were expected to always call SignOut()
   even when prohibited. This allowed the *chrome* client to do special
   cleanup.
2. ProhibitSignout() was really meant to narrowly prohibit user-triggered
   sign-out for scenarios like cloud-managed accounts. But the prohibition
   was applied broadly for all SignOut() calls, leading to weird states
   when sign-out was triggered for internal reasons (e.g. cloud policy
   changes).

These issues came up in planning an analogous API for the new
IdentityManager. This CL implements the proposed fixes
https://docs.google.com/document/d/14esaufZT0fr258zL5I69y7YBBc24XsxtDU0B7wpWxBo/edit#bookmark=id.t69fnqpy377k

Namely:
- Prohibiting sign-out is now up to the SigninClient. SignOut() signals
  the source of the sign-out (user-driven vs internal reasons) and the client
  runs a callback indicating whether the sign-out may continue.
- signin_util::SetUserSignoutAllowedForProfile() chrome/ code
  allow/block user-driven sign-outs.
- Other implementers of SigninClient never block sign-out and are unchanged.

NOTE: For now, signin_util::SetUserSignoutAllowedForProfile() actually
prohibits most non-user-driven signouts to avoid changing sign-out behavior in this
large plumbing CL. This will change in a follow up CL where non-user-driven
sign-outs are always permitted.
https://chromium-review.googlesource.com/c/chromium/src/+/1175797/


Bug: 866518
Cq-Include-Trybots: luci.chromium.try:ios-simulator-cronet;luci.chromium.try:ios-simulator-full-configs
Change-Id: I4758706d796f953cbb68ff1b4a0c8d2985d12fb3
Reviewed-on: https://chromium-review.googlesource.com/1162173
Commit-Queue: Chrome Cunningham <chcunningham@chromium.org>
Reviewed-by: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Colin Blundell <blundell@chromium.org>
Reviewed-by: Boris Sazonov <bsazonov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595214}

chrome/android/java/src/org/chromium/chrome/browser/signin/SigninHelper.java

@@ -153,11 +153,6 @@ public class SigninHelper {
                 protected void onPostExecute(Void result) {
                     String renamedAccount = getNewSignedInAccountName();
                     if (renamedAccount == null) {
-                        // SigninManager.signOut() uses the same code path as a user-triggered
-                        // signout, which can be prohibited in some cases (e.g. child accounts).
-                        // Here we have to sign out though to ensure account consistency,
-                        // so override the flag.
-                        mSigninManager.prohibitSignout(false);
                         mSigninManager.signOut(SignoutReason.ACCOUNT_REMOVED_FROM_DEVICE);
                     } else {
                         validateAccountSettings(true);

chrome/android/java/src/org/chromium/chrome/browser/signin/SigninManager.java

@@ -568,10 +568,6 @@ public class SigninManager implements AccountTrackerService.OnSystemAccountsSeed
         nativeClearLastSignedInUser(mNativeSigninManagerAndroid);
     }
 
-    public void prohibitSignout(boolean prohibitSignout) {
-        nativeProhibitSignout(mNativeSigninManagerAndroid, prohibitSignout);
-    }
-
     /**
      * Aborts the current sign in.
      *
@@ -751,6 +747,4 @@ public class SigninManager implements AccountTrackerService.OnSystemAccountsSeed
     native void nativeLogInSignedInUser(long nativeSigninManagerAndroid);
     @VisibleForTesting
     native boolean nativeIsSignedInOnNative(long nativeSigninManagerAndroid);
-    @VisibleForTesting
-    native void nativeProhibitSignout(long nativeSigninManagerAndroid, boolean prohibitSignout);
 }

chrome/browser/android/signin/signin_manager_android.cc

@@ -326,13 +326,6 @@ jboolean SigninManagerAndroid::IsSignedInOnNative(
   return SigninManagerFactory::GetForProfile(profile_)->IsAuthenticated();
 }
 
-void SigninManagerAndroid::ProhibitSignout(JNIEnv* env,
-                                           const JavaParamRef<jobject>& obj,
-                                           jboolean prohibit_signout) {
-  SigninManagerFactory::GetForProfile(profile_)->ProhibitSignout(
-      prohibit_signout);
-}
-
 void SigninManagerAndroid::GoogleSigninFailed(
     const GoogleServiceAuthError& error) {}
 

chrome/browser/android/signin/signin_manager_android.h

@@ -80,10 +80,6 @@ class SigninManagerAndroid : public SigninManagerBase::Observer {
   jboolean IsSignedInOnNative(JNIEnv* env,
                               const base::android::JavaParamRef<jobject>& obj);
 
-  void ProhibitSignout(JNIEnv* env,
-                       const base::android::JavaParamRef<jobject>& obj,
-                       jboolean prohibit_signout);
-
   // SigninManagerBase::Observer implementation.
   void GoogleSigninFailed(const GoogleServiceAuthError& error) override;
   void GoogleSigninSucceeded(const std::string& account_id,

chrome/browser/policy/cloud/user_policy_signin_service.cc

@@ -14,6 +14,7 @@
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
+#include "chrome/browser/signin/signin_util.h"
 #include "components/account_id/account_id.h"
 #include "components/policy/core/common/cloud/cloud_policy_client_registration_helper.h"
 #include "components/policy/core/common/cloud/user_cloud_policy_manager.h"
@@ -190,7 +191,8 @@ void UserPolicySigninService::ShutdownUserCloudPolicyManager() {
   UserCloudPolicyManager* manager = policy_manager();
   // Allow the user to signout again.
   if (manager)
-    signin_manager()->ProhibitSignout(false);
+    signin_util::SetUserSignoutAllowedForProfile(profile_, true);
+
   UserPolicySigninServiceBase::ShutdownUserCloudPolicyManager();
 }
 
@@ -245,7 +247,7 @@ void UserPolicySigninService::OnRegistrationComplete() {
 void UserPolicySigninService::ProhibitSignoutIfNeeded() {
   if (policy_manager()->IsClientRegistered()) {
     DVLOG(1) << "User is registered for policy - prohibiting signout";
-    signin_manager()->ProhibitSignout(true);
+    signin_util::SetUserSignoutAllowedForProfile(profile_, false);
   }
 }
 

chrome/browser/policy/cloud/user_policy_signin_service_unittest.cc

@@ -25,6 +25,7 @@
 #include "chrome/browser/signin/fake_signin_manager_builder.h"
 #include "chrome/browser/signin/profile_oauth2_token_service_factory.h"
 #include "chrome/browser/signin/signin_manager_factory.h"
+#include "chrome/browser/signin/signin_util.h"
 #include "chrome/browser/signin/test_signin_client_builder.h"
 #include "chrome/test/base/testing_browser_process.h"
 #include "chrome/test/base/testing_profile.h"
@@ -846,7 +847,7 @@ TEST_F(UserPolicySigninServiceTest, PolicyFetchFailureDisableManagement) {
 
   EXPECT_TRUE(manager_->IsClientRegistered());
 #if !defined(OS_ANDROID)
-  EXPECT_TRUE(signin_manager_->IsSignoutProhibited());
+  EXPECT_FALSE(signin_util::IsUserSignoutAllowedForProfile(profile_.get()));
 #endif
 
   // Kick off another policy fetch.
@@ -868,7 +869,7 @@ TEST_F(UserPolicySigninServiceTest, PolicyFetchFailureDisableManagement) {
   base::RunLoop().RunUntilIdle();
   EXPECT_FALSE(manager_->IsClientRegistered());
 #if !defined(OS_ANDROID)
-  EXPECT_FALSE(signin_manager_->IsSignoutProhibited());
+  EXPECT_TRUE(signin_util::IsUserSignoutAllowedForProfile(profile_.get()));
 #endif
 }
 

chrome/browser/signin/chrome_signin_client.cc

@@ -65,6 +65,25 @@
 #include "chrome/browser/ui/user_manager.h"
 #endif
 
+namespace {
+SigninClient::SignoutDecision IsSignoutAllowed(
+    Profile* profile,
+    const signin_metrics::ProfileSignout signout_source_metric) {
+  // TODO(chcunningham): This logic should be reworked to only prohibit user-
+  // initiated sign-out. For now signin_util::IsUserSignoutAllowedForProfile()
+  // prohibits ALL sign-outs with the exception of ACCOUNT_REMOVED_FROM_DEVICE
+  // because this preserves the original behavior. A follow-up CL will make the
+  // slightly riskier change described above.
+  if (signin_util::IsUserSignoutAllowedForProfile(profile) ||
+      signout_source_metric ==
+          signin_metrics::ProfileSignout::ACCOUNT_REMOVED_FROM_DEVICE) {
+    return SigninClient::SignoutDecision::ALLOW_SIGNOUT;
+  }
+
+  return SigninClient::SignoutDecision::DISALLOW_SIGNOUT;
+}
+}  // namespace
+
 ChromeSigninClient::ChromeSigninClient(
     Profile* profile,
     SigninErrorController* signin_error_controller)
@@ -186,8 +205,12 @@ void ChromeSigninClient::PostSignedIn(const std::string& account_id,
 }
 
 void ChromeSigninClient::PreSignOut(
-    const base::Callback<void()>& sign_out,
+    base::OnceCallback<void(SignoutDecision)> on_signout_decision_reached,
     signin_metrics::ProfileSignout signout_source_metric) {
+  DCHECK(on_signout_decision_reached);
+  DCHECK(!on_signout_decision_reached_) << "SignOut already in-progress!";
+  on_signout_decision_reached_ = std::move(on_signout_decision_reached);
+
 #if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
 
   // These sign out won't remove the policy cache, keep the window opened.
@@ -206,13 +229,12 @@ void ChromeSigninClient::PreSignOut(
       // Call OnCloseBrowsersSuccess to continue sign out and show UserManager
       // afterwards.
       should_display_user_manager_ = false;  // Don't show UserManager twice.
-      OnCloseBrowsersSuccess(sign_out, signout_source_metric,
-                             profile_->GetPath());
+      OnCloseBrowsersSuccess(signout_source_metric, profile_->GetPath());
     } else {
       BrowserList::CloseAllBrowsersWithProfile(
           profile_,
           base::Bind(&ChromeSigninClient::OnCloseBrowsersSuccess,
-                     base::Unretained(this), sign_out, signout_source_metric),
+                     base::Unretained(this), signout_source_metric),
           base::Bind(&ChromeSigninClient::OnCloseBrowsersAborted,
                      base::Unretained(this)),
           signout_source_metric == signin_metrics::ABORT_SIGNIN ||
@@ -224,7 +246,8 @@ void ChromeSigninClient::PreSignOut(
 #else
   {
 #endif
-    SigninClient::PreSignOut(sign_out, signout_source_metric);
+    std::move(on_signout_decision_reached_)
+        .Run(IsSignoutAllowed(profile_, signout_source_metric));
   }
 }
 
@@ -390,7 +413,6 @@ void ChromeSigninClient::SetURLLoaderFactoryForTest(
 }
 
 void ChromeSigninClient::OnCloseBrowsersSuccess(
-    const base::Callback<void()>& sign_out,
     const signin_metrics::ProfileSignout signout_source_metric,
     const base::FilePath& profile_path) {
 #if !defined(OS_ANDROID) && !defined(OS_CHROMEOS)
@@ -398,7 +420,9 @@ void ChromeSigninClient::OnCloseBrowsersSuccess(
     force_signin_verifier_->Cancel();
   }
 #endif
-  SigninClient::PreSignOut(sign_out, signout_source_metric);
+
+  std::move(on_signout_decision_reached_)
+      .Run(IsSignoutAllowed(profile_, signout_source_metric));
 
   LockForceSigninProfile(profile_path);
   // After sign out, lock the profile and show UserManager if necessary.
@@ -412,6 +436,10 @@ void ChromeSigninClient::OnCloseBrowsersSuccess(
 void ChromeSigninClient::OnCloseBrowsersAborted(
     const base::FilePath& profile_path) {
   should_display_user_manager_ = true;
+
+  // Disallow sign-out (aborted).
+  std::move(on_signout_decision_reached_)
+      .Run(SignoutDecision::DISALLOW_SIGNOUT);
 }
 
 void ChromeSigninClient::LockForceSigninProfile(

chrome/browser/signin/chrome_signin_client.h

@@ -46,6 +46,9 @@ class ChromeSigninClient
 
   // SigninClient implementation.
   PrefService* GetPrefs() override;
+  void PreSignOut(
+      base::OnceCallback<void(SignoutDecision)> on_signout_decision_reached,
+      signin_metrics::ProfileSignout signout_source_metric) override;
   void OnSignedOut() override;
   scoped_refptr<network::SharedURLLoaderFactory> GetURLLoaderFactory() override;
   network::mojom::CookieManager* GetCookieManager() override;
@@ -74,9 +77,6 @@ class ChromeSigninClient
   void PostSignedIn(const std::string& account_id,
                     const std::string& username,
                     const std::string& password) override;
-  void PreSignOut(
-      const base::Callback<void()>& sign_out,
-      signin_metrics::ProfileSignout signout_source_metric) override;
 
   // SigninErrorController::Observer implementation.
   void OnErrorChanged() override;
@@ -116,7 +116,6 @@ class ChromeSigninClient
   void MaybeFetchSigninTokenHandle();
   void VerifySyncToken();
   void OnCloseBrowsersSuccess(
-      const base::Callback<void()>& sign_out,
       const signin_metrics::ProfileSignout signout_source_metric,
       const base::FilePath& profile_path);
   void OnCloseBrowsersAborted(const base::FilePath& profile_path);
@@ -124,6 +123,10 @@ class ChromeSigninClient
   Profile* profile_;
 
   SigninErrorController* signin_error_controller_;
+
+  // Stored callback from PreSignOut();
+  base::OnceCallback<void(SignoutDecision)> on_signout_decision_reached_;
+
 #if !defined(OS_CHROMEOS)
   std::list<base::Closure> delayed_callbacks_;
 #endif

chrome/browser/signin/chrome_signin_client_unittest.cc

@@ -10,6 +10,7 @@
 #include "base/bind.h"
 #include "base/logging.h"
 #include "base/run_loop.h"
+#include "base/stl_util.h"
 #include "build/build_config.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/signin/chrome_signin_client_factory.h"
@@ -150,10 +151,11 @@ class MockSigninManager : public SigninManager {
     DCHECK(signin_error_controller);
   }
 
-  MOCK_METHOD3(DoSignOut,
+  MOCK_METHOD4(OnSignoutDecisionReached,
                void(signin_metrics::ProfileSignout,
                     signin_metrics::SignoutDelete,
-                    RemoveAccountsOption remove_option));
+                    RemoveAccountsOption remove_option,
+                    SigninClient::SignoutDecision signout_decision));
 
   AccountTrackerService fake_service_;
 };
@@ -196,10 +198,11 @@ TEST_F(ChromeSigninClientSignoutTest, SignOut) {
       .Times(1);
   EXPECT_CALL(*client_, LockForceSigninProfile(browser()->profile()->GetPath()))
       .Times(1);
-  EXPECT_CALL(
-      *manager_,
-      DoSignOut(source_metric, delete_metric,
-                SigninManager::RemoveAccountsOption::kRemoveAllAccounts))
+  EXPECT_CALL(*manager_,
+              OnSignoutDecisionReached(
+                  source_metric, delete_metric,
+                  SigninManager::RemoveAccountsOption::kRemoveAllAccounts,
+                  SigninClient::SignoutDecision::ALLOW_SIGNOUT))
       .Times(1);
 
   manager_->SignOut(source_metric, delete_metric);
@@ -218,10 +221,11 @@ TEST_F(ChromeSigninClientSignoutTest, SignOutWithoutManager) {
       .Times(0);
   EXPECT_CALL(*client_, LockForceSigninProfile(browser()->profile()->GetPath()))
       .Times(1);
-  EXPECT_CALL(
-      *manager_,
-      DoSignOut(source_metric, delete_metric,
-                SigninManager::RemoveAccountsOption::kRemoveAllAccounts))
+  EXPECT_CALL(*manager_,
+              OnSignoutDecisionReached(
+                  source_metric, delete_metric,
+                  SigninManager::RemoveAccountsOption::kRemoveAllAccounts,
+                  SigninClient::SignoutDecision::ALLOW_SIGNOUT))
       .Times(1);
   manager_->SignOut(source_metric, delete_metric);
 
@@ -231,10 +235,11 @@ TEST_F(ChromeSigninClientSignoutTest, SignOutWithoutManager) {
       .Times(1);
   EXPECT_CALL(*client_, LockForceSigninProfile(browser()->profile()->GetPath()))
       .Times(1);
-  EXPECT_CALL(
-      *manager_,
-      DoSignOut(source_metric, delete_metric,
-                SigninManager::RemoveAccountsOption::kRemoveAllAccounts))
+  EXPECT_CALL(*manager_,
+              OnSignoutDecisionReached(
+                  source_metric, delete_metric,
+                  SigninManager::RemoveAccountsOption::kRemoveAllAccounts,
+                  SigninClient::SignoutDecision::ALLOW_SIGNOUT))
       .Times(1);
   manager_->SignOut(source_metric, delete_metric);
 }
@@ -254,10 +259,11 @@ TEST_F(ChromeSigninClientSignoutTest, SignOutWithoutForceSignin) {
       .Times(0);
   EXPECT_CALL(*client_, LockForceSigninProfile(browser()->profile()->GetPath()))
       .Times(0);
-  EXPECT_CALL(
-      *manager_,
-      DoSignOut(source_metric, delete_metric,
-                SigninManager::RemoveAccountsOption::kRemoveAllAccounts))
+  EXPECT_CALL(*manager_,
+              OnSignoutDecisionReached(
+                  source_metric, delete_metric,
+                  SigninManager::RemoveAccountsOption::kRemoveAllAccounts,
+                  SigninClient::SignoutDecision::ALLOW_SIGNOUT))
       .Times(1);
   manager_->SignOut(source_metric, delete_metric);
 }
@@ -280,13 +286,126 @@ TEST_F(ChromeSigninClientSignoutTest, SignOutGuestSession) {
       .Times(0);
   EXPECT_CALL(*client_, LockForceSigninProfile(browser()->profile()->GetPath()))
       .Times(0);
-  EXPECT_CALL(
-      *manager_,
-      DoSignOut(source_metric, delete_metric,
-                SigninManager::RemoveAccountsOption::kRemoveAllAccounts))
+  EXPECT_CALL(*manager_,
+              OnSignoutDecisionReached(
+                  source_metric, delete_metric,
+                  SigninManager::RemoveAccountsOption::kRemoveAllAccounts,
+                  SigninClient::SignoutDecision::ALLOW_SIGNOUT))
       .Times(1);
   manager_->SignOut(source_metric, delete_metric);
 }
 
+class ChromeSigninClientSignoutSourceTest
+    : public ::testing::WithParamInterface<signin_metrics::ProfileSignout>,
+      public ChromeSigninClientSignoutTest {};
+
+bool IsUserDrivenSignout(signin_metrics::ProfileSignout signout_source) {
+  switch (signout_source) {
+    // NOTE: SIGNOUT_TEST == SIGNOUT_PREF_CHANGED.
+    case signin_metrics::ProfileSignout::SIGNOUT_PREF_CHANGED:
+    case signin_metrics::ProfileSignout::GOOGLE_SERVICE_NAME_PATTERN_CHANGED:
+    case signin_metrics::ProfileSignout::SIGNIN_PREF_CHANGED_DURING_SIGNIN:
+    case signin_metrics::ProfileSignout::USER_CLICKED_SIGNOUT_SETTINGS:
+    case signin_metrics::ProfileSignout::ABORT_SIGNIN:
+    case signin_metrics::ProfileSignout::SERVER_FORCED_DISABLE:
+    case signin_metrics::ProfileSignout::TRANSFER_CREDENTIALS:
+    case signin_metrics::ProfileSignout::
+        AUTHENTICATION_FAILED_WITH_FORCE_SIGNIN:
+    case signin_metrics::ProfileSignout::USER_TUNED_OFF_SYNC_FROM_DICE_UI:
+      return true;
+    case signin_metrics::ProfileSignout::ACCOUNT_REMOVED_FROM_DEVICE:
+      // TODO(chcunningham): Add more of the above cases to this "false" branch.
+      // For now only ACCOUNT_REMOVED_FROM_DEVICE is here to preserve the status
+      // quo. Additional internal sources of sign-out will be moved here in a
+      // follow up CL.
+      return false;
+    case signin_metrics::ProfileSignout::NUM_PROFILE_SIGNOUT_METRICS:
+      NOTREACHED();
+      return false;
+  }
+}
+
+TEST_P(ChromeSigninClientSignoutSourceTest, UserSignoutAllowed) {
+  signin_metrics::ProfileSignout signout_source = GetParam();
+
+  TestingProfile::Builder builder;
+  builder.SetGuestSession();
+  std::unique_ptr<TestingProfile> profile = builder.Build();
+
+  CreateClient(profile.get());
+  manager_ = std::make_unique<MockSigninManager>(client_.get(),
+                                                 fake_controller_.get());
+
+  // User sign-out is allowed for this test.
+  ASSERT_TRUE(signin_util::IsUserSignoutAllowedForProfile(profile.get()));
+
+  // Verify SigninManager gets callback indicating sign-out is always allowed.
+  signin_metrics::SignoutDelete delete_metric =
+      signin_metrics::SignoutDelete::IGNORE_METRIC;
+  EXPECT_CALL(*manager_,
+              OnSignoutDecisionReached(
+                  signout_source, delete_metric,
+                  SigninManager::RemoveAccountsOption::kRemoveAllAccounts,
+                  SigninClient::SignoutDecision::ALLOW_SIGNOUT))
+      .Times(1);
+
+  manager_->SignOut(signout_source, delete_metric);
+}
+
+TEST_P(ChromeSigninClientSignoutSourceTest, UserSignoutDisallowed) {
+  signin_metrics::ProfileSignout signout_source = GetParam();
+
+  TestingProfile::Builder builder;
+  builder.SetGuestSession();
+  std::unique_ptr<TestingProfile> profile = builder.Build();
+
+  CreateClient(profile.get());
+  manager_ = std::make_unique<MockSigninManager>(client_.get(),
+                                                 fake_controller_.get());
+
+  // Disallow user sign-out.
+  ASSERT_TRUE(signin_util::IsUserSignoutAllowedForProfile(profile.get()));
+  signin_util::SetUserSignoutAllowedForProfile(profile.get(), false);
+  ASSERT_FALSE(signin_util::IsUserSignoutAllowedForProfile(profile.get()));
+
+  // Verify SigninManager gets callback indicating sign-out is disallowed iff
+  // the source of the sign-out is a user-action.
+  SigninClient::SignoutDecision signout_decision =
+      IsUserDrivenSignout(signout_source)
+          ? SigninClient::SignoutDecision::DISALLOW_SIGNOUT
+          : SigninClient::SignoutDecision::ALLOW_SIGNOUT;
+  signin_metrics::SignoutDelete delete_metric =
+      signin_metrics::SignoutDelete::IGNORE_METRIC;
+  EXPECT_CALL(*manager_,
+              OnSignoutDecisionReached(
+                  signout_source, delete_metric,
+                  SigninManager::RemoveAccountsOption::kRemoveAllAccounts,
+                  signout_decision))
+      .Times(1);
+
+  manager_->SignOut(signout_source, delete_metric);
+}
+
+const signin_metrics::ProfileSignout kSignoutSources[] = {
+    // NOTE: SIGNOUT_TEST == SIGNOUT_PREF_CHANGED.
+    signin_metrics::ProfileSignout::SIGNOUT_PREF_CHANGED,
+    signin_metrics::ProfileSignout::GOOGLE_SERVICE_NAME_PATTERN_CHANGED,
+    signin_metrics::ProfileSignout::SIGNIN_PREF_CHANGED_DURING_SIGNIN,
+    signin_metrics::ProfileSignout::USER_CLICKED_SIGNOUT_SETTINGS,
+    signin_metrics::ProfileSignout::ABORT_SIGNIN,
+    signin_metrics::ProfileSignout::SERVER_FORCED_DISABLE,
+    signin_metrics::ProfileSignout::TRANSFER_CREDENTIALS,
+    signin_metrics::ProfileSignout::AUTHENTICATION_FAILED_WITH_FORCE_SIGNIN,
+    signin_metrics::ProfileSignout::USER_TUNED_OFF_SYNC_FROM_DICE_UI,
+    signin_metrics::ProfileSignout::ACCOUNT_REMOVED_FROM_DEVICE,
+};
+static_assert(base::size(kSignoutSources) ==
+                  signin_metrics::ProfileSignout::NUM_PROFILE_SIGNOUT_METRICS,
+              "kSignoutSources should enumerate all ProfileSignout values");
+
+INSTANTIATE_TEST_CASE_P(AllSignoutSources,
+                        ChromeSigninClientSignoutSourceTest,
+                        testing::ValuesIn(kSignoutSources));
+
 #endif  // !defined(OS_ANDROID)
 #endif  // !defined(OS_CHROMEOS)

chrome/browser/signin/signin_util.cc

@@ -4,13 +4,46 @@
 
 #include "chrome/browser/signin/signin_util.h"
 
+#include <memory>
+
+#include "base/supports_user_data.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "components/prefs/pref_service.h"
 
 namespace signin_util {
 namespace {
 
+constexpr char kSignoutSettingKey[] = "signout_setting";
+
+class UserSignoutSetting : public base::SupportsUserData::Data {
+ public:
+  // Fetch from Profile. Make and store if not already present.
+  static UserSignoutSetting* GetForProfile(Profile* profile) {
+    UserSignoutSetting* signout_setting = static_cast<UserSignoutSetting*>(
+        profile->GetUserData(kSignoutSettingKey));
+
+    if (!signout_setting) {
+      profile->SetUserData(kSignoutSettingKey,
+                           std::make_unique<UserSignoutSetting>());
+      signout_setting = static_cast<UserSignoutSetting*>(
+          profile->GetUserData(kSignoutSettingKey));
+    }
+
+    return signout_setting;
+  }
+
+  bool is_user_signout_allowed() const { return is_user_signout_allowed_; }
+  void set_is_user_signout_allowed(bool is_allowed) {
+    is_user_signout_allowed_ = is_allowed;
+  }
+
+ private:
+  // User sign-out allowed by default.
+  bool is_user_signout_allowed_ = true;
+};
+
 enum ForceSigninPolicyCache {
   NOT_CACHED = 0,
   ENABLE,
@@ -42,4 +75,13 @@ void ResetForceSigninForTesting() {
   g_is_force_signin_enabled_cache = NOT_CACHED;
 }
 
+void SetUserSignoutAllowedForProfile(Profile* profile, bool is_allowed) {
+  UserSignoutSetting::GetForProfile(profile)->set_is_user_signout_allowed(
+      is_allowed);
+}
+
+bool IsUserSignoutAllowedForProfile(Profile* profile) {
+  return UserSignoutSetting::GetForProfile(profile)->is_user_signout_allowed();
+}
+
 }  // namespace signin_util

chrome/browser/signin/signin_util.h

@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_SIGNIN_SIGNIN_UTIL_H_
 #define CHROME_BROWSER_SIGNIN_SIGNIN_UTIL_H_
 
+class Profile;
+
 namespace signin_util {
 
 // Return whether the force sign in policy is enabled or not.
@@ -17,6 +19,15 @@ void SetForceSigninForTesting(bool enable);
 // Reset force sign in to uninitialized state for testing.
 void ResetForceSigninForTesting();
 
+// Sign-out is allowed by default, but some Chrome profiles (e.g. for cloud-
+// managed enterprise accounts) may wish to disallow user-initiated sign-out.
+// Note that this exempts sign-outs that are not user-initiated (e.g. sign-out
+// triggered when cloud policy no longer allows current email pattern). See
+// ChromeSigninClient::PreSignOut().
+void SetUserSignoutAllowedForProfile(Profile* profile, bool is_allowed);
+
+bool IsUserSignoutAllowedForProfile(Profile* profile);
+
 }  // namespace signin_util
 
 #endif  // CHROME_BROWSER_SIGNIN_SIGNIN_UTIL_H_

chrome/browser/supervised_user/child_accounts/child_account_service.cc

@@ -39,6 +39,8 @@
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/profiles/profile_helper.h"
+#else
+#include "chrome/browser/signin/signin_util.h"
 #endif
 
 const char kGaiaCookieManagerSource[] = "child_account_service";
@@ -195,7 +197,7 @@ bool ChildAccountService::SetActive(bool active) {
     // This is also used by user policies (UserPolicySigninService), but since
     // child accounts can not also be Dasher accounts, there shouldn't be any
     // problems.
-    SigninManagerFactory::GetForProfile(profile_)->ProhibitSignout(true);
+    signin_util::SetUserSignoutAllowedForProfile(profile_, false);
 #endif
 
     // TODO(treib): Maybe store the last update time in a pref, so we don't
@@ -227,7 +229,7 @@ bool ChildAccountService::SetActive(bool active) {
 #endif
 
 #if !defined(OS_CHROMEOS)
-    SigninManagerFactory::GetForProfile(profile_)->ProhibitSignout(false);
+    signin_util::SetUserSignoutAllowedForProfile(profile_, true);
 #endif
 
     CancelFetchingFamilyInfo();

chrome/browser/sync/sync_ui_util.cc

@@ -22,8 +22,7 @@
 #include "ui/base/l10n/l10n_util.h"
 
 #if !defined(OS_CHROMEOS)
-#include "chrome/browser/signin/signin_manager_factory.h"
-#include "components/signin/core/browser/signin_manager.h"
+#include "chrome/browser/signin/signin_util.h"
 #endif  // defined(OS_CHROMEOS)
 
 using browser_sync::ProfileSyncService;
@@ -114,7 +113,7 @@ void GetStatusForUnrecoverableError(Profile* profile,
     status_label->assign(l10n_util::GetStringUTF16(
         IDS_SYNC_STATUS_UNRECOVERABLE_ERROR));
     // The message for managed accounts is the same as that of the cros.
-    if (SigninManagerFactory::GetForProfile(profile)->IsSignoutProhibited()) {
+    if (!signin_util::IsUserSignoutAllowedForProfile(profile)) {
       status_label->assign(l10n_util::GetStringUTF16(
           IDS_SYNC_STATUS_UNRECOVERABLE_ERROR_NEEDS_SIGNOUT));
     }
@@ -352,7 +351,7 @@ AvatarSyncErrorType GetMessagesForAvatarSyncError(
     service->QueryDetailedSyncStatus(&status);
     if (status.sync_protocol_error.action != syncer::UPGRADE_CLIENT) {
       // Display different messages and buttons for managed accounts.
-      if (SigninManagerFactory::GetForProfile(profile)->IsSignoutProhibited()) {
+      if (!signin_util::IsUserSignoutAllowedForProfile(profile)) {
         // For a managed user, the user is directed to the signout
         // confirmation dialogue in the settings page.
         *content_string_id = IDS_SYNC_ERROR_USER_MENU_SIGNOUT_MESSAGE;

chrome/browser/ui/webui/settings/people_handler.cc

@@ -65,6 +65,7 @@
 #include "chrome/browser/chromeos/login/quick_unlock/pin_backend.h"
 #include "components/signin/core/browser/signin_manager_base.h"
 #else
+#include "chrome/browser/signin/signin_util.h"
 #include "chrome/browser/ui/user_manager.h"
 #include "chrome/browser/ui/webui/profile_helper.h"
 #include "components/signin/core/browser/signin_manager.h"
@@ -775,11 +776,12 @@ void PeopleHandler::HandleSignout(const base::ListValue* args) {
   bool delete_profile = false;
   args->GetBoolean(0, &delete_profile);
 
-  SigninManager* signin_manager = SigninManagerFactory::GetForProfile(profile_);
-  if (signin_manager->IsSignoutProhibited()) {
+  if (!signin_util::IsUserSignoutAllowedForProfile(profile_)) {
     // If the user cannot signout, the profile must be destroyed.
     DCHECK(delete_profile);
   } else {
+    SigninManager* signin_manager =
+        SigninManagerFactory::GetForProfile(profile_);
     if (signin_manager->IsAuthenticated()) {
       if (GetSyncService())
         ProfileSyncService::SyncEvent(ProfileSyncService::STOP_FROM_OPTIONS);
@@ -945,7 +947,7 @@ PeopleHandler::GetSyncStatusDictionary() {
   DCHECK(signin);
 #if !defined(OS_CHROMEOS)
   // Signout is not allowed if the user has policy (crbug.com/172204).
-  if (SigninManagerFactory::GetForProfile(profile_)->IsSignoutProhibited()) {
+  if (!signin_util::IsUserSignoutAllowedForProfile(profile_)) {
     std::string username = signin->GetAuthenticatedAccountInfo().email;
 
     // If there is no one logged in or if the profile name is empty then the

components/signin/core/browser/fake_signin_manager.cc

@@ -103,7 +103,7 @@ void FakeSigninManager::SignIn(const std::string& gaia_id,
 }
 
 void FakeSigninManager::ForceSignOut() {
-  ProhibitSignout(false);
+  // SigninClients should always allow sign-out for SIGNOUT_TEST.
   SignOut(signin_metrics::SIGNOUT_TEST,
           signin_metrics::SignoutDelete::IGNORE_METRIC);
 }
@@ -113,10 +113,11 @@ void FakeSigninManager::FailSignin(const GoogleServiceAuthError& error) {
     observer.GoogleSigninFailed(error);
 }
 
-void FakeSigninManager::DoSignOut(
+void FakeSigninManager::OnSignoutDecisionReached(
     signin_metrics::ProfileSignout signout_source_metric,
     signin_metrics::SignoutDelete signout_delete_metric,
-    RemoveAccountsOption remove_option) {
+    RemoveAccountsOption remove_option,
+    SigninClient::SignoutDecision signout_decision) {
   if (!IsAuthenticated()) {
     if (AuthInProgress()) {
       // If the user is in the process of signing in, then treat a call to
@@ -133,8 +134,11 @@ void FakeSigninManager::DoSignOut(
     return;
   }
 
-  if (IsSignoutProhibited())
+  // TODO(crbug.com/887756): Consider moving this higher up, or document why
+  // the above blocks are exempt from the |signout_decision| early return.
+  if (signout_decision == SigninClient::SignoutDecision::DISALLOW_SIGNOUT)
     return;
+
   set_auth_in_progress(std::string());
   set_password(std::string());
   AccountInfo account_info = GetAuthenticatedAccountInfo();

components/signin/core/browser/fake_signin_manager.h

@@ -76,9 +76,11 @@ class FakeSigninManager : public SigninManager {
   void CompletePendingSignin() override;
 
  protected:
-  void DoSignOut(signin_metrics::ProfileSignout signout_source_metric,
-                 signin_metrics::SignoutDelete signout_delete_metric,
-                 RemoveAccountsOption remove_option) override;
+  void OnSignoutDecisionReached(
+      signin_metrics::ProfileSignout signout_source_metric,
+      signin_metrics::SignoutDelete signout_delete_metric,
+      RemoveAccountsOption remove_option,
+      SigninClient::SignoutDecision signout_decision) override;
 
   // Username specified in StartSignInWithRefreshToken() call.
   std::string username_;

components/signin/core/browser/signin_client.cc

@@ -5,9 +5,10 @@
 #include "components/signin/core/browser/signin_client.h"
 
 void SigninClient::PreSignOut(
-    const base::Callback<void()>& sign_out,
+    base::OnceCallback<void(SignoutDecision)> on_signout_decision_reached,
     signin_metrics::ProfileSignout signout_source_metric) {
-  sign_out.Run();
+  // Allow sign out to continue.
+  std::move(on_signout_decision_reached).Run(SignoutDecision::ALLOW_SIGNOUT);
 }
 
 void SigninClient::PreGaiaLogout(base::OnceClosure callback) {

components/signin/core/browser/signin_client.h

@@ -36,12 +36,10 @@ class CookieManager;
 // embedder.
 class SigninClient : public KeyedService {
  public:
-  ~SigninClient() override = default;
+  // Argument to PreSignOut() callback, indicating client decision.
+  enum class SignoutDecision { ALLOW_SIGNOUT, DISALLOW_SIGNOUT };
 
-  // Called before Google signout started, call |sign_out| to start the sign out
-  // process.
-  virtual void PreSignOut(const base::Callback<void()>& sign_out,
-                          signin_metrics::ProfileSignout signout_source_metric);
+  ~SigninClient() override = default;
 
   // Perform Chrome-specific sign out. This happens when user signs out.
   virtual void OnSignedOut() = 0;
@@ -77,6 +75,14 @@ class SigninClient : public KeyedService {
                             const std::string& username,
                             const std::string& password) {}
 
+  // Called before Google sign-out started. Implementers must run the
+  // |on_signout_decision_reached|, passing a SignoutDecision to allow/disallow
+  // sign-out to continue. When to disallow sign-out is implementation specific.
+  // Sign-out is always allowed by default.
+  virtual void PreSignOut(
+      base::OnceCallback<void(SignoutDecision)> on_signout_decision_reached,
+      signin_metrics::ProfileSignout signout_source_metric);
+
   // Called before calling the GAIA logout endpoint.
   // For iOS, cookies should be cleaned up.
   virtual void PreGaiaLogout(base::OnceClosure callback);

components/signin/core/browser/signin_manager.cc

@@ -15,7 +15,6 @@
 #include "components/prefs/pref_service.h"
 #include "components/signin/core/browser/account_tracker_service.h"
 #include "components/signin/core/browser/gaia_cookie_manager_service.h"
-#include "components/signin/core/browser/signin_client.h"
 #include "components/signin/core/browser/signin_internals_util.h"
 #include "components/signin/core/browser/signin_metrics.h"
 #include "components/signin/core/browser/signin_pref_names.h"
@@ -37,7 +36,6 @@ SigninManager::SigninManager(
     : SigninManagerBase(client,
                         account_tracker_service,
                         signin_error_controller),
-      prohibit_signout_(false),
       type_(SIGNIN_TYPE_NONE),
       client_(client),
       token_service_(token_service),
@@ -185,15 +183,17 @@ void SigninManager::StartSignOut(
     signin_metrics::SignoutDelete signout_delete_metric,
     RemoveAccountsOption remove_option) {
   client_->PreSignOut(
-      base::Bind(&SigninManager::DoSignOut, base::Unretained(this),
-                 signout_source_metric, signout_delete_metric, remove_option),
+      base::BindOnce(&SigninManager::OnSignoutDecisionReached,
+                     base::Unretained(this), signout_source_metric,
+                     signout_delete_metric, remove_option),
       signout_source_metric);
 }
 
-void SigninManager::DoSignOut(
+void SigninManager::OnSignoutDecisionReached(
     signin_metrics::ProfileSignout signout_source_metric,
     signin_metrics::SignoutDelete signout_delete_metric,
-    RemoveAccountsOption remove_option) {
+    RemoveAccountsOption remove_option,
+    SigninClient::SignoutDecision signout_decision) {
   DCHECK(IsInitialized());
 
   signin_metrics::LogSignout(signout_source_metric, signout_delete_metric);
@@ -213,8 +213,10 @@ void SigninManager::DoSignOut(
     return;
   }
 
-  if (IsSignoutProhibited()) {
-    DVLOG(1) << "Ignoring attempt to sign out while signout is prohibited";
+  // TODO(crbug.com/887756): Consider moving this higher up, or document why
+  // the above blocks are exempt from the |signout_decision| early return.
+  if (signout_decision == SigninClient::SignoutDecision::DISALLOW_SIGNOUT) {
+    DVLOG(1) << "Ignoring attempt to sign out while signout disallowed";
     return;
   }
 
@@ -504,9 +506,3 @@ void SigninManager::OnRefreshTokensLoaded() {
     token_service_->RemoveObserver(this);
   }
 }
-
-void SigninManager::ProhibitSignout(bool prohibit_signout) {
-  prohibit_signout_ = prohibit_signout;
-}
-
-bool SigninManager::IsSignoutProhibited() const { return prohibit_signout_; }

components/signin/core/browser/signin_manager.h

@@ -38,6 +38,7 @@
 #include "components/signin/core/browser/account_tracker_service.h"
 #include "components/signin/core/browser/profile_management_switches.h"
 #include "components/signin/core/browser/profile_oauth2_token_service.h"
+#include "components/signin/core/browser/signin_client.h"
 #include "components/signin/core/browser/signin_internals_util.h"
 #include "components/signin/core/browser/signin_manager_base.h"
 #include "components/signin/core/browser/signin_metrics.h"
@@ -185,22 +186,13 @@ class SigninManager : public SigninManagerBase,
   // a new account).
   static void DisableOneClickSignIn(PrefService* prefs);
 
-  // Tells the SigninManager whether to prohibit signout for this profile.
-  // If |prohibit_signout| is true, then signout will be prohibited.
-  void ProhibitSignout(bool prohibit_signout);
-
-  // If true, signout is prohibited for this profile (calls to SignOut() are
-  // ignored).
-  bool IsSignoutProhibited() const;
-
  protected:
-  // Flag saying whether signing out is allowed.
-  bool prohibit_signout_;
-
   // The sign out process which is started by SigninClient::PreSignOut()
-  virtual void DoSignOut(signin_metrics::ProfileSignout signout_source_metric,
-                         signin_metrics::SignoutDelete signout_delete_metric,
-                         RemoveAccountsOption remove_option);
+  virtual void OnSignoutDecisionReached(
+      signin_metrics::ProfileSignout signout_source_metric,
+      signin_metrics::SignoutDelete signout_delete_metric,
+      RemoveAccountsOption remove_option,
+      SigninClient::SignoutDecision signout_decision);
 
  private:
   enum SigninType {

components/signin/core/browser/signin_manager_unittest.cc

@@ -335,11 +335,11 @@ TEST_F(SigninManagerTest, SignOutWhileProhibited) {
   EXPECT_TRUE(manager_->GetAuthenticatedAccountId().empty());
 
   manager_->SetAuthenticatedAccountInfo("gaia_id", "user@gmail.com");
-  manager_->ProhibitSignout(true);
+  signin_client()->set_is_signout_allowed(false);
   manager_->SignOut(signin_metrics::SIGNOUT_TEST,
                     signin_metrics::SignoutDelete::IGNORE_METRIC);
   EXPECT_TRUE(manager_->IsAuthenticated());
-  manager_->ProhibitSignout(false);
+  signin_client()->set_is_signout_allowed(true);
   manager_->SignOut(signin_metrics::SIGNOUT_TEST,
                     signin_metrics::SignoutDelete::IGNORE_METRIC);
   EXPECT_FALSE(manager_->IsAuthenticated());

components/signin/core/browser/signin_metrics.h

@@ -69,7 +69,7 @@ enum AccessPointAction {
   HISTOGRAM_REJECTED,
   // The user pressed the X button to dismiss the infobar this time.
   HISTOGRAM_DISMISSED,
-  // The user completely ignored the infoar.  Either they navigated away, or
+  // The user completely ignored the infobar.  Either they navigated away, or
   // they used the page as is.
   HISTOGRAM_IGNORED,
   // The user clicked on the learn more link in the infobar.

components/signin/core/browser/test_signin_client.cc

@@ -49,7 +49,8 @@ TestSigninClient::TestSigninClient(PrefService* pref_service)
               &test_url_loader_factory_)),
       pref_service_(pref_service),
       are_signin_cookies_allowed_(true),
-      network_calls_delayed_(false) {}
+      network_calls_delayed_(false),
+      is_signout_allowed_(true) {}
 
 TestSigninClient::~TestSigninClient() {}
 
@@ -75,6 +76,14 @@ void TestSigninClient::PostSignedIn(const std::string& account_id,
   signed_in_password_ = password;
 }
 
+void TestSigninClient::PreSignOut(
+    base::OnceCallback<void(SignoutDecision)> on_signout_decision_reached,
+    signin_metrics::ProfileSignout signout_source_metric) {
+  std::move(on_signout_decision_reached)
+      .Run(is_signout_allowed_ ? SignoutDecision::ALLOW_SIGNOUT
+                               : SignoutDecision::DISALLOW_SIGNOUT);
+}
+
 scoped_refptr<network::SharedURLLoaderFactory>
 TestSigninClient::GetURLLoaderFactory() {
   return shared_factory_;

components/signin/core/browser/test_signin_client.h

@@ -46,6 +46,12 @@ class TestSigninClient : public SigninClient {
                     const std::string& username,
                     const std::string& password) override;
 
+  // Allow or disallow continuation of sign-out depending on value of
+  // |is_signout_allowed_|;
+  void PreSignOut(
+      base::OnceCallback<void(SignoutDecision)> on_signout_decision_reached,
+      signin_metrics::ProfileSignout signout_source_metric) override;
+
   std::string get_signed_in_password() { return signed_in_password_; }
 
   // Returns the empty string.
@@ -65,6 +71,8 @@ class TestSigninClient : public SigninClient {
     are_signin_cookies_allowed_ = value;
   }
 
+  void set_is_signout_allowed(bool value) { is_signout_allowed_ = value; }
+
   // When |value| is true, network calls posted through DelayNetworkCall() are
   // delayed indefinitely.
   // When |value| is false, all pending calls are unblocked, and new calls are
@@ -95,6 +103,7 @@ class TestSigninClient : public SigninClient {
   std::unique_ptr<network::mojom::CookieManager> cookie_manager_;
   bool are_signin_cookies_allowed_;
   bool network_calls_delayed_;
+  bool is_signout_allowed_;
   std::vector<base::OnceClosure> delayed_network_calls_;
 
   // Pointer to be filled by PostSignedIn.

ios/web_view/internal/signin/ios_web_view_signin_client.h

@@ -49,7 +49,7 @@ class IOSWebViewSigninClient : public SigninClient,
   void RemoveContentSettingsObserver(
       content_settings::Observer* observer) override;
   void PreSignOut(
-      const base::RepeatingClosure& sign_out,
+      base::OnceCallback<void(SignoutDecision)> on_signout_decision_reached,
       signin_metrics::ProfileSignout signout_source_metric) override;
   void DelayNetworkCall(const base::Closure& callback) override;
   std::unique_ptr<GaiaAuthFetcher> CreateGaiaAuthFetcher(

ios/web_view/internal/signin/ios_web_view_signin_client.mm

@@ -86,9 +86,9 @@ void IOSWebViewSigninClient::RemoveContentSettingsObserver(
 }
 
 void IOSWebViewSigninClient::PreSignOut(
-    const base::RepeatingClosure& sign_out,
+    base::OnceCallback<void(SignoutDecision)> on_signout_decision_reached,
     signin_metrics::ProfileSignout signout_source_metric) {
-  sign_out.Run();
+  std::move(on_signout_decision_reached).Run(SignoutDecision::ALLOW_SIGNOUT);
   [sync_controller_ didSignoutWithSourceMetric:signout_source_metric];
 }