Commit f636d94d authored by Daniel Murphy's avatar Daniel Murphy Committed by Commit Bot

Revert "[signin] Don't clear unconsented primary account on cancelling Sync dialog"

This reverts commit f21f4c91.

Reason for revert: Seems to be causing SingleClientStandaloneTransportSyncTest.HandlesResetFromDashboardWhenSyncActive to fail

Bug: 1029869

Original change's description:
> [signin] Don't clear unconsented primary account on cancelling Sync dialog
> 
> The unconsented primary account doesn't necessarily change when the Sync
> confirmation dialog is cancelled.
> In particular this is the case when PrimaryAccountMutator::ClearPrimaryAccount
> is called with RemoveAccountOptions::kKeepAll.
> 
> This CL stops resetting the unconsented primary account as part of the signout
> process.
> The unconsented primary account is recomputed anyway when GoogleSignedOut is
> called.
> 
> 
> Fixed: 1029701
> Change-Id: Id46bb468952efe63170a13872519fcaded076cb1
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1944862
> Commit-Queue: David Roger <droger@chromium.org>
> Reviewed-by: Monica Basta <msalama@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#720502}

TBR=droger@chromium.org,msalama@chromium.org

Change-Id: I0632406070243902295db9f9e482f161df6e65ce
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1946194Reviewed-by: default avatarDaniel Murphy <dmurph@chromium.org>
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720570}
parent 31ef8e69
...@@ -327,7 +327,7 @@ void PrimaryAccountManager::OnSignoutDecisionReached( ...@@ -327,7 +327,7 @@ void PrimaryAccountManager::OnSignoutDecisionReached(
const CoreAccountInfo account_info = GetAuthenticatedAccountInfo(); const CoreAccountInfo account_info = GetAuthenticatedAccountInfo();
client_->GetPrefs()->ClearPref(prefs::kGoogleServicesHostedDomain); client_->GetPrefs()->ClearPref(prefs::kGoogleServicesHostedDomain);
SetPrimaryAccountInternal(account_info, /*consented_to_sync=*/false); SetPrimaryAccountInternal(CoreAccountInfo(), /*consented_to_sync=*/false);
// Revoke all tokens before sending signed_out notification, because there // Revoke all tokens before sending signed_out notification, because there
// may be components that don't listen for token service events when the // may be components that don't listen for token service events when the
...@@ -351,8 +351,10 @@ void PrimaryAccountManager::OnSignoutDecisionReached( ...@@ -351,8 +351,10 @@ void PrimaryAccountManager::OnSignoutDecisionReached(
break; break;
} }
for (Observer& observer : observers_) for (Observer& observer : observers_) {
observer.GoogleSignedOut(account_info); observer.GoogleSignedOut(account_info);
observer.UnconsentedPrimaryAccountChanged(primary_account_info());
}
} }
void PrimaryAccountManager::OnRefreshTokensLoaded() { void PrimaryAccountManager::OnRefreshTokensLoaded() {
......
...@@ -154,16 +154,13 @@ TEST_F(PrimaryAccountManagerTest, SignOut) { ...@@ -154,16 +154,13 @@ TEST_F(PrimaryAccountManagerTest, SignOut) {
EXPECT_FALSE(manager_->IsAuthenticated()); EXPECT_FALSE(manager_->IsAuthenticated());
EXPECT_TRUE(manager_->GetAuthenticatedAccountInfo().email.empty()); EXPECT_TRUE(manager_->GetAuthenticatedAccountInfo().email.empty());
EXPECT_TRUE(manager_->GetAuthenticatedAccountId().empty()); EXPECT_TRUE(manager_->GetAuthenticatedAccountId().empty());
EXPECT_EQ(main_account_id,
manager_->GetUnconsentedPrimaryAccountInfo().account_id);
// Should not be persisted anymore // Should not be persisted anymore
ShutDownManager(); ShutDownManager();
CreatePrimaryAccountManager(); CreatePrimaryAccountManager();
EXPECT_FALSE(manager_->IsAuthenticated()); EXPECT_FALSE(manager_->IsAuthenticated());
EXPECT_TRUE(manager_->GetAuthenticatedAccountInfo().email.empty()); EXPECT_TRUE(manager_->GetAuthenticatedAccountInfo().email.empty());
EXPECT_TRUE(manager_->GetAuthenticatedAccountId().empty()); EXPECT_TRUE(manager_->GetAuthenticatedAccountId().empty());
EXPECT_EQ(main_account_id, EXPECT_EQ(CoreAccountInfo(), manager_->GetUnconsentedPrimaryAccountInfo());
manager_->GetUnconsentedPrimaryAccountInfo().account_id);
} }
TEST_F(PrimaryAccountManagerTest, SignOutRevoke) { TEST_F(PrimaryAccountManagerTest, SignOutRevoke) {
...@@ -207,9 +204,8 @@ TEST_F(PrimaryAccountManagerTest, SignOutDiceNoRevoke) { ...@@ -207,9 +204,8 @@ TEST_F(PrimaryAccountManagerTest, SignOutDiceNoRevoke) {
std::vector<CoreAccountId> expected_tokens = {main_account_id, std::vector<CoreAccountId> expected_tokens = {main_account_id,
other_account_id}; other_account_id};
EXPECT_EQ(expected_tokens, token_service_.GetAccounts()); EXPECT_EQ(expected_tokens, token_service_.GetAccounts());
// Unconsented primary account is not reset. EXPECT_EQ(manager_->GetUnconsentedPrimaryAccountInfo(),
EXPECT_EQ(main_account_id, manager_->GetAuthenticatedAccountInfo());
manager_->GetUnconsentedPrimaryAccountInfo().account_id);
} }
TEST_F(PrimaryAccountManagerTest, SignOutDiceWithError) { TEST_F(PrimaryAccountManagerTest, SignOutDiceWithError) {
......
...@@ -505,7 +505,7 @@ IdentityManager::ComputeUnconsentedPrimaryAccountInfo() const { ...@@ -505,7 +505,7 @@ IdentityManager::ComputeUnconsentedPrimaryAccountInfo() const {
// On ChromeOS and on mobile platforms, we support only the primary account as // On ChromeOS and on mobile platforms, we support only the primary account as
// the unconsented primary account. By this early return, we avoid an extra // the unconsented primary account. By this early return, we avoid an extra
// request to GAIA that lists cookie accounts. // request to GAIA that lists cookie accounts.
return CoreAccountInfo(); return base::nullopt;
#else #else
AccountsInCookieJarInfo cookie_info = GetAccountsInCookieJar(); AccountsInCookieJarInfo cookie_info = GetAccountsInCookieJar();
......
...@@ -12,7 +12,6 @@ ...@@ -12,7 +12,6 @@
#include "base/command_line.h" #include "base/command_line.h"
#include "base/containers/flat_set.h" #include "base/containers/flat_set.h"
#include "base/run_loop.h" #include "base/run_loop.h"
#include "base/scoped_observer.h"
#include "base/stl_util.h" #include "base/stl_util.h"
#include "base/test/bind_test_util.h" #include "base/test/bind_test_util.h"
#include "base/test/task_environment.h" #include "base/test/task_environment.h"
...@@ -1559,47 +1558,6 @@ TEST_F( ...@@ -1559,47 +1558,6 @@ TEST_F(
EXPECT_EQ(identity_manager()->GetUnconsentedPrimaryAccountInfo(), empty_info); EXPECT_EQ(identity_manager()->GetUnconsentedPrimaryAccountInfo(), empty_info);
} }
TEST_F(IdentityManagerTest, UnconsentedPrimaryAccountNotChangedOnSignout) {
// Setup cookies and token for the main account.
CoreAccountInfo account_info;
account_info.email = kTestEmail;
account_info.gaia = kTestGaiaId;
account_info.account_id =
identity_manager()->PickAccountIdForAccount(kTestGaiaId, kTestEmail);
SetCookieAccounts(identity_manager(), test_url_loader_factory(),
{{account_info.email, account_info.gaia}});
SetRefreshTokenForAccount(identity_manager(), account_info.account_id,
"refresh-token");
EXPECT_EQ(account_info,
identity_manager()->GetUnconsentedPrimaryAccountInfo());
EXPECT_EQ(account_info, identity_manager()->GetPrimaryAccountInfo());
EXPECT_TRUE(identity_manager()->HasPrimaryAccountWithRefreshToken());
// Tests that OnUnconsentedPrimaryAccountChanged is never called.
class UnconsentedPrimaryAccountObserver : public IdentityManager::Observer {
public:
void OnUnconsentedPrimaryAccountChanged(
const CoreAccountInfo& unconsented_primary_account_info) override {
NOTREACHED();
called_ = true;
}
bool called_ = false;
};
UnconsentedPrimaryAccountObserver observer;
ScopedObserver<IdentityManager, IdentityManager::Observer> scoped_observer(
&observer);
scoped_observer.Add(identity_manager());
// Clear primary account but do not delete the account.
ClearPrimaryAccount(identity_manager(),
ClearPrimaryAccountPolicy::KEEP_ALL_ACCOUNTS);
// Primary account is cleared, but unconsented account is not.
EXPECT_FALSE(identity_manager()->HasPrimaryAccount());
EXPECT_EQ(account_info,
identity_manager()->GetUnconsentedPrimaryAccountInfo());
// OnUnconsentedPrimaryAccountChanged was not fired.
EXPECT_FALSE(observer.called_);
}
TEST_F(IdentityManagerTest, TEST_F(IdentityManagerTest,
UnconsentedPrimaryAccountTokenRevokedWithStaleCookies) { UnconsentedPrimaryAccountTokenRevokedWithStaleCookies) {
ClearPrimaryAccount(identity_manager(), ClearPrimaryAccountPolicy::DEFAULT); ClearPrimaryAccount(identity_manager(), ClearPrimaryAccountPolicy::DEFAULT);
......
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