Commit e3ac7414 authored by Mihai Sardarescu's avatar Mihai Sardarescu Committed by Commit Bot

Avoid monitoring the browser active state in account consistency service.

AccountConsistencyService now uses the cookie manager directly to add
or delete the CHROME_CONNECTED cookies for google web domains (this
change landed in CL
https://chromium-review.googlesource.com/c/chromium/src/+/2323901 ).

This means that it can update the CHROME_CONNECTED cookies even when
the incognito mode is active as it no longer relies on using a WKWebView
to inject the CHROME_CONNECTED cookies.

Bug: 1102794

Change-Id: I2064ca62f801f46c0d2dae9b87276f1f6dc9bceb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2339656
Commit-Queue: Mihai Sardarescu <msarda@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Reviewed-by: default avatarNohemi Fernandez <fernandex@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796322}
parent c111c001
...@@ -15,7 +15,6 @@ ...@@ -15,7 +15,6 @@
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "components/keyed_service/core/keyed_service.h" #include "components/keyed_service/core/keyed_service.h"
#include "components/signin/ios/browser/active_state_manager.h"
#import "components/signin/ios/browser/manage_accounts_delegate.h" #import "components/signin/ios/browser/manage_accounts_delegate.h"
#import "components/signin/public/identity_manager/identity_manager.h" #import "components/signin/public/identity_manager/identity_manager.h"
#include "net/cookies/canonical_cookie.h" #include "net/cookies/canonical_cookie.h"
...@@ -40,8 +39,7 @@ class PrefService; ...@@ -40,8 +39,7 @@ class PrefService;
// CHROME_CONNECTED, which informs Gaia that the user is signed in to Chrome // CHROME_CONNECTED, which informs Gaia that the user is signed in to Chrome
// with Account Consistency on. // with Account Consistency on.
class AccountConsistencyService : public KeyedService, class AccountConsistencyService : public KeyedService,
public signin::IdentityManager::Observer, public signin::IdentityManager::Observer {
public ActiveStateManager::Observer {
public: public:
// Name of the cookie that is managed by AccountConsistencyService and is used // Name of the cookie that is managed by AccountConsistencyService and is used
// to inform Google web properties that the browser is connected and that // to inform Google web properties that the browser is connected and that
...@@ -177,9 +175,6 @@ class AccountConsistencyService : public KeyedService, ...@@ -177,9 +175,6 @@ class AccountConsistencyService : public KeyedService,
const signin::AccountsInCookieJarInfo& accounts_in_cookie_jar_info, const signin::AccountsInCookieJarInfo& accounts_in_cookie_jar_info,
const GoogleServiceAuthError& error) override; const GoogleServiceAuthError& error) override;
// ActiveStateManager::Observer implementation.
void OnActive() override;
// Browser state associated with the service. // Browser state associated with the service.
web::BrowserState* browser_state_; web::BrowserState* browser_state_;
// Used to update kDomainsWithCookiePref. // Used to update kDomainsWithCookiePref.
......
...@@ -209,7 +209,6 @@ AccountConsistencyService::AccountConsistencyService( ...@@ -209,7 +209,6 @@ AccountConsistencyService::AccountConsistencyService(
identity_manager_(identity_manager), identity_manager_(identity_manager),
applying_cookie_requests_(false) { applying_cookie_requests_(false) {
identity_manager_->AddObserver(this); identity_manager_->AddObserver(this);
ActiveStateManager::FromBrowserState(browser_state_)->AddObserver(this);
LoadFromPrefs(); LoadFromPrefs();
if (identity_manager_->HasPrimaryAccount()) { if (identity_manager_->HasPrimaryAccount()) {
AddChromeConnectedCookies(); AddChromeConnectedCookies();
...@@ -348,7 +347,6 @@ void AccountConsistencyService::LoadFromPrefs() { ...@@ -348,7 +347,6 @@ void AccountConsistencyService::LoadFromPrefs() {
void AccountConsistencyService::Shutdown() { void AccountConsistencyService::Shutdown() {
identity_manager_->RemoveObserver(this); identity_manager_->RemoveObserver(this);
ActiveStateManager::FromBrowserState(browser_state_)->RemoveObserver(this);
web_state_handlers_.clear(); web_state_handlers_.clear();
} }
...@@ -361,11 +359,6 @@ void AccountConsistencyService::ApplyCookieRequests() { ...@@ -361,11 +359,6 @@ void AccountConsistencyService::ApplyCookieRequests() {
if (cookie_requests_.empty()) { if (cookie_requests_.empty()) {
return; return;
} }
if (!ActiveStateManager::FromBrowserState(browser_state_)->IsActive()) {
// Web view usage isn't active for now, ignore cookie requests for now and
// wait to be notified that it became active again.
return;
}
applying_cookie_requests_ = true; applying_cookie_requests_ = true;
const GURL url("https://" + cookie_requests_.front().domain); const GURL url("https://" + cookie_requests_.front().domain);
...@@ -498,9 +491,3 @@ void AccountConsistencyService::OnAccountsInCookieUpdated( ...@@ -498,9 +491,3 @@ void AccountConsistencyService::OnAccountsInCookieUpdated(
const GoogleServiceAuthError& error) { const GoogleServiceAuthError& error) {
AddChromeConnectedCookies(); AddChromeConnectedCookies();
} }
void AccountConsistencyService::OnActive() {
// |browser_state_| is now active. There might be some pending cookie requests
// to apply.
ApplyCookieRequests();
}
...@@ -162,7 +162,6 @@ class AccountConsistencyServiceTest : public PlatformTest { ...@@ -162,7 +162,6 @@ class AccountConsistencyServiceTest : public PlatformTest {
protected: protected:
void SetUp() override { void SetUp() override {
PlatformTest::SetUp(); PlatformTest::SetUp();
ActiveStateManager::FromBrowserState(&browser_state_)->SetActive(true);
AccountConsistencyService::RegisterPrefs(prefs_.registry()); AccountConsistencyService::RegisterPrefs(prefs_.registry());
content_settings::CookieSettings::RegisterProfilePrefs(prefs_.registry()); content_settings::CookieSettings::RegisterProfilePrefs(prefs_.registry());
HostContentSettingsMap::RegisterProfilePrefs(prefs_.registry()); HostContentSettingsMap::RegisterProfilePrefs(prefs_.registry());
...@@ -186,7 +185,6 @@ class AccountConsistencyServiceTest : public PlatformTest { ...@@ -186,7 +185,6 @@ class AccountConsistencyServiceTest : public PlatformTest {
void TearDown() override { void TearDown() override {
account_consistency_service_->Shutdown(); account_consistency_service_->Shutdown();
settings_map_->ShutdownOnUIThread(); settings_map_->ShutdownOnUIThread();
ActiveStateManager::FromBrowserState(&browser_state_)->SetActive(false);
identity_test_env_.reset(); identity_test_env_.reset();
PlatformTest::TearDown(); PlatformTest::TearDown();
} }
...@@ -204,9 +202,7 @@ class AccountConsistencyServiceTest : public PlatformTest { ...@@ -204,9 +202,7 @@ class AccountConsistencyServiceTest : public PlatformTest {
// Spinning the runloop is needed to ensure that the cookie manager requests // Spinning the runloop is needed to ensure that the cookie manager requests
// are executed. // are executed.
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_TRUE(account_consistency_service_->cookie_requests_.empty());
if (ActiveStateManager::FromBrowserState(&browser_state_)->IsActive())
EXPECT_TRUE(account_consistency_service_->cookie_requests_.empty());
} }
void SignIn() { void SignIn() {
...@@ -369,20 +365,6 @@ TEST_F(AccountConsistencyServiceTest, SignOutWithoutDomains) { ...@@ -369,20 +365,6 @@ TEST_F(AccountConsistencyServiceTest, SignOutWithoutDomains) {
CheckNoChromeConnectedCookies(); CheckNoChromeConnectedCookies();
} }
// Tests that pending cookie requests are correctly applied when the browser
// state becomes active.
TEST_F(AccountConsistencyServiceTest, ApplyOnActive) {
// No request is made until the browser state is active, then a WKWebView and
// its navigation delegate are created, and the requests are processed.
ActiveStateManager::FromBrowserState(&browser_state_)->SetActive(false);
SignIn();
CheckNoChromeConnectedCookies();
ActiveStateManager::FromBrowserState(&browser_state_)->SetActive(true);
CheckDomainHasChromeConnectedCookie(kGoogleDomain);
CheckDomainHasChromeConnectedCookie(kYoutubeDomain);
}
// Tests that the X-Chrome-Manage-Accounts header is ignored unless it comes // Tests that the X-Chrome-Manage-Accounts header is ignored unless it comes
// from Gaia signon realm. // from Gaia signon realm.
TEST_F(AccountConsistencyServiceTest, ChromeManageAccountsNotOnGaia) { TEST_F(AccountConsistencyServiceTest, ChromeManageAccountsNotOnGaia) {
......
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