[Signin][Android] Revise sign-out flow for unconsented account

Changes sign-out flow logic in PrimaryAccountManager:
1. Allows sign-out flow even if there's only an unconsented account and
   adds a test for this.
2. Adds "if !defined(OS_ANDROID)" to DICE-specific
   PrimaryAccountManager & PrimaryAccountMutator tests.

Bug: 1095128
Change-Id: I0d5b5e804b360ff2ea5c4bf7c6e3add07a21d835
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2391237
Commit-Queue: Boris Sazonov <bsazonov@chromium.org>
Reviewed-by: David Roger <droger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#807015}

components/signin/internal/identity_manager/primary_account_manager.cc

@@ -12,6 +12,7 @@
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
+#include "build/build_config.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "components/signin/internal/identity_manager/account_tracker_service.h"
@@ -310,12 +311,20 @@ void PrimaryAccountManager::StartSignOut(
   VLOG(1) << "StartSignOut: " << static_cast<int>(signout_source_metric) << ", "
           << static_cast<int>(signout_delete_metric) << ", "
           << static_cast<int>(remove_option);
-  client_->PreSignOut(
-      base::BindOnce(&PrimaryAccountManager::OnSignoutDecisionReached,
-                     base::Unretained(this), signout_source_metric,
-                     signout_delete_metric, remove_option,
-                     assert_signout_allowed),
-      signout_source_metric);
+  if (IsAuthenticated()) {
+    client_->PreSignOut(
+        base::BindOnce(&PrimaryAccountManager::OnSignoutDecisionReached,
+                       base::Unretained(this), signout_source_metric,
+                       signout_delete_metric, remove_option,
+                       assert_signout_allowed),
+        signout_source_metric);
+  } else {
+    // Sign-out is always allowed if there's only unconsented primary account
+    // without sync consent, so skip calling PreSignOut.
+    OnSignoutDecisionReached(signout_source_metric, signout_delete_metric,
+                             remove_option, assert_signout_allowed,
+                             SigninClient::SignoutDecision::ALLOW_SIGNOUT);
+  }
 }
 
 void PrimaryAccountManager::OnSignoutDecisionReached(
@@ -331,7 +340,7 @@ void PrimaryAccountManager::OnSignoutDecisionReached(
   VLOG(1) << "OnSignoutDecisionReached: "
           << (signout_decision == SigninClient::SignoutDecision::ALLOW_SIGNOUT);
   signin_metrics::LogSignout(signout_source_metric, signout_delete_metric);
-  if (!IsAuthenticated()) {
+  if (primary_account_info().IsEmpty()) {
     return;
   }
 

components/signin/internal/identity_manager/primary_account_manager_unittest.cc

@@ -12,6 +12,7 @@
 #include "base/bind.h"
 #include "base/run_loop.h"
 #include "base/test/task_environment.h"
+#include "build/build_config.h"
 #include "components/image_fetcher/core/fake_image_decoder.h"
 #include "components/prefs/pref_service.h"
 #include "components/prefs/scoped_user_pref_update.h"
@@ -184,6 +185,8 @@ TEST_F(PrimaryAccountManagerTest, SignOutRevoke) {
   EXPECT_TRUE(token_service_.GetAccounts().empty());
 }
 
+// kRemoveAuthenticatedAccountIfInError isn't supported on Android.
+#if !defined(OS_ANDROID)
 TEST_F(PrimaryAccountManagerTest, SignOutDiceNoRevoke) {
   account_consistency_ = signin::AccountConsistencyMethod::kDice;
   CreatePrimaryAccountManager();
@@ -239,6 +242,7 @@ TEST_F(PrimaryAccountManagerTest, SignOutDiceWithError) {
   std::vector<CoreAccountId> expected_tokens = {other_account_id};
   EXPECT_EQ(expected_tokens, token_service_.GetAccounts());
 }
+#endif
 
 TEST_F(PrimaryAccountManagerTest, SignOutWhileProhibited) {
   CreatePrimaryAccountManager();
@@ -258,6 +262,23 @@ TEST_F(PrimaryAccountManagerTest, SignOutWhileProhibited) {
   EXPECT_FALSE(manager_->IsAuthenticated());
 }
 
+TEST_F(PrimaryAccountManagerTest, UnconsentedSignOutWhileProhibited) {
+  CreatePrimaryAccountManager();
+  EXPECT_FALSE(manager_->IsAuthenticated());
+  EXPECT_TRUE(manager_->GetAuthenticatedAccountInfo().email.empty());
+  EXPECT_TRUE(manager_->GetAuthenticatedAccountId().empty());
+
+  CoreAccountId account_id = AddToAccountTracker("gaia_id", "user@gmail.com");
+  CoreAccountInfo account_info = account_tracker()->GetAccountInfo(account_id);
+  manager_->SetUnconsentedPrimaryAccountInfo(account_info);
+  EXPECT_TRUE(manager_->HasUnconsentedPrimaryAccount());
+  EXPECT_FALSE(manager_->IsAuthenticated());
+  signin_client()->set_is_signout_allowed(false);
+  manager_->SignOut(signin_metrics::SIGNOUT_TEST,
+                    signin_metrics::SignoutDelete::IGNORE_METRIC);
+  EXPECT_FALSE(manager_->HasUnconsentedPrimaryAccount());
+}
+
 TEST_F(PrimaryAccountManagerTest, ProhibitedAtStartup) {
   CoreAccountId account_id = AddToAccountTracker("gaia_id", "user@gmail.com");
   user_prefs_.SetString(prefs::kGoogleServicesAccountId, account_id.ToString());

components/signin/public/identity_manager/primary_account_mutator_unittest.cc

@@ -9,6 +9,7 @@
 #include "base/run_loop.h"
 #include "base/scoped_observer.h"
 #include "base/test/task_environment.h"
+#include "build/build_config.h"
 #include "components/signin/public/base/signin_metrics.h"
 #include "components/signin/public/base/signin_pref_names.h"
 #include "components/signin/public/identity_manager/consent_level.h"
@@ -480,6 +481,9 @@ TEST_F(PrimaryAccountMutatorTest,
       RemoveAccountExpectation::kRemoveAll);
 }
 
+// kRemoveAuthenticatedAccountIfInError isn't supported on Android.
+#if !defined(OS_ANDROID)
+
 // Test that ClearPrimaryAccount(...) with ClearAccountTokensAction::kDefault
 // and AccountConsistencyMethod::kDice keeps all accounts when the the primary
 // account does not have an authentication error (see *_AuthError test).
@@ -500,6 +504,7 @@ TEST_F(PrimaryAccountMutatorTest,
       signin::PrimaryAccountMutator::ClearAccountsAction::kDefault,
       RemoveAccountExpectation::kRemovePrimary, AuthExpectation::kAuthError);
 }
+#endif  // !defined(OS_ANDROID)
 #endif  // !defined(OS_CHROMEOS)
 
 #if defined(OS_CHROMEOS)