Commit 4eb2a7f0 authored by Colin Blundell's avatar Colin Blundell Committed by Commit Bot

Make SubscriptionManager test work with async token fetches

In order to move the behavior of PrimaryAccountAccessTokenFetcher closer
to what it will be once it is interacting with the Identity Service, I
will shortly be changing this class to fetch
access tokens strictly asynchronously (by posting a task to the current
message loop to call into ProfileOAuth2TokenService). In particular,
this change is necessary to preserve the expected ordering with other
soon-to-be-introduced IdentityManager APIs that will interact
asynchronously with ProfileOAuth2TokenService (e.g., to invalidate
access tokens).

The SubscriptionManager unittests currently assume that
AccessTokenFetcher interacts synchronously with PO2TS. This CL relaxes
these assumptions so that the tests will continue to work as-is when the
above change is made.

Bug: 654990
Change-Id: Ib0a863c0b00133379819319129e1a49a46647d88
Reviewed-on: https://chromium-review.googlesource.com/844575
Commit-Queue: Colin Blundell <blundell@chromium.org>
Reviewed-by: default avatarJan Krcal <jkrcal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526664}
parent 0defaba6
...@@ -38,7 +38,9 @@ const char kUnsubscriptionUrlSignedIn[] = "http://valid-url.test/unsubscribe"; ...@@ -38,7 +38,9 @@ const char kUnsubscriptionUrlSignedIn[] = "http://valid-url.test/unsubscribe";
const char kUnsubscriptionUrlSignedOut[] = const char kUnsubscriptionUrlSignedOut[] =
"http://valid-url.test/unsubscribe?key=fakeAPIkey"; "http://valid-url.test/unsubscribe?key=fakeAPIkey";
class SubscriptionManagerImplTest : public testing::Test { class SubscriptionManagerImplTest
: public testing::Test,
public OAuth2TokenService::DiagnosticsObserver {
public: public:
SubscriptionManagerImplTest() SubscriptionManagerImplTest()
: request_context_getter_( : request_context_getter_(
...@@ -51,6 +53,11 @@ class SubscriptionManagerImplTest : public testing::Test { ...@@ -51,6 +53,11 @@ class SubscriptionManagerImplTest : public testing::Test {
signin::RegisterAccountConsistencyProfilePrefs( signin::RegisterAccountConsistencyProfilePrefs(
utils_.pref_service()->registry()); utils_.pref_service()->registry());
signin::SetGaiaOriginIsolatedCallback(base::Bind([] { return true; })); signin::SetGaiaOriginIsolatedCallback(base::Bind([] { return true; }));
utils_.token_service()->AddDiagnosticsObserver(this);
}
void TearDown() override {
utils_.token_service()->RemoveDiagnosticsObserver(this);
} }
scoped_refptr<net::URLRequestContextGetter> GetRequestContext() { scoped_refptr<net::URLRequestContextGetter> GetRequestContext() {
...@@ -133,6 +140,10 @@ class SubscriptionManagerImplTest : public testing::Test { ...@@ -133,6 +140,10 @@ class SubscriptionManagerImplTest : public testing::Test {
base::Time::Max()); base::Time::Max());
} }
void set_on_access_token_request_callback(base::OnceClosure callback) {
on_access_token_request_callback_ = std::move(callback);
}
private: private:
void RespondSuccessfully() { void RespondSuccessfully() {
net::TestURLFetcher* url_fetcher = GetRunningFetcher(); net::TestURLFetcher* url_fetcher = GetRunningFetcher();
...@@ -151,10 +162,20 @@ class SubscriptionManagerImplTest : public testing::Test { ...@@ -151,10 +162,20 @@ class SubscriptionManagerImplTest : public testing::Test {
url_fetcher->delegate()->OnURLFetchComplete(url_fetcher); url_fetcher->delegate()->OnURLFetchComplete(url_fetcher);
} }
// OAuth2TokenService::DiagnosticsObserver:
void OnAccessTokenRequested(
const std::string& account_id,
const std::string& consumer_id,
const OAuth2TokenService::ScopeSet& scopes) override {
if (on_access_token_request_callback_)
std::move(on_access_token_request_callback_).Run();
}
base::MessageLoop message_loop_; base::MessageLoop message_loop_;
test::RemoteSuggestionsTestUtils utils_; test::RemoteSuggestionsTestUtils utils_;
scoped_refptr<net::TestURLRequestContextGetter> request_context_getter_; scoped_refptr<net::TestURLRequestContextGetter> request_context_getter_;
net::TestURLFetcherFactory url_fetcher_factory_; net::TestURLFetcherFactory url_fetcher_factory_;
base::OnceClosure on_access_token_request_callback_;
}; };
TEST_F(SubscriptionManagerImplTest, SubscribeSuccessfully) { TEST_F(SubscriptionManagerImplTest, SubscribeSuccessfully) {
...@@ -179,6 +200,9 @@ TEST_F(SubscriptionManagerImplTest, SubscribeSuccessfully) { ...@@ -179,6 +200,9 @@ TEST_F(SubscriptionManagerImplTest, SubscribeSuccessfully) {
#if !defined(OS_CHROMEOS) #if !defined(OS_CHROMEOS)
TEST_F(SubscriptionManagerImplTest, TEST_F(SubscriptionManagerImplTest,
ShouldSubscribeWithAuthenticationWhenAuthenticated) { ShouldSubscribeWithAuthenticationWhenAuthenticated) {
base::RunLoop run_loop;
set_on_access_token_request_callback(run_loop.QuitClosure());
// Sign in. // Sign in.
FakeProfileOAuth2TokenService* auth_token_service = GetOAuth2TokenService(); FakeProfileOAuth2TokenService* auth_token_service = GetOAuth2TokenService();
SignIn(); SignIn();
...@@ -192,6 +216,8 @@ TEST_F(SubscriptionManagerImplTest, ...@@ -192,6 +216,8 @@ TEST_F(SubscriptionManagerImplTest,
/*locale=*/"", kAPIKey, GURL(kSubscriptionUrl), GURL(kUnsubscriptionUrl)); /*locale=*/"", kAPIKey, GURL(kSubscriptionUrl), GURL(kUnsubscriptionUrl));
manager.Subscribe(subscription_token); manager.Subscribe(subscription_token);
run_loop.Run();
// Make sure that subscription is pending an access token. // Make sure that subscription is pending an access token.
ASSERT_FALSE(manager.IsSubscribed()); ASSERT_FALSE(manager.IsSubscribed());
ASSERT_EQ(1u, auth_token_service->GetPendingRequests().size()); ASSERT_EQ(1u, auth_token_service->GetPendingRequests().size());
...@@ -274,10 +300,15 @@ TEST_F(SubscriptionManagerImplTest, ...@@ -274,10 +300,15 @@ TEST_F(SubscriptionManagerImplTest,
RespondToSubscriptionRequestSuccessfully(/*is_signed_in=*/false); RespondToSubscriptionRequestSuccessfully(/*is_signed_in=*/false);
ASSERT_FALSE(manager.NeedsToResubscribe()); ASSERT_FALSE(manager.NeedsToResubscribe());
base::RunLoop run_loop;
set_on_access_token_request_callback(run_loop.QuitClosure());
// Sign in. This should trigger a resubscribe. // Sign in. This should trigger a resubscribe.
SignIn(); SignIn();
IssueRefreshToken(auth_token_service); IssueRefreshToken(auth_token_service);
ASSERT_TRUE(manager.NeedsToResubscribe()); ASSERT_TRUE(manager.NeedsToResubscribe());
run_loop.Run();
ASSERT_EQ(1u, auth_token_service->GetPendingRequests().size()); ASSERT_EQ(1u, auth_token_service->GetPendingRequests().size());
IssueAccessToken(auth_token_service); IssueAccessToken(auth_token_service);
RespondToSubscriptionRequestSuccessfully(/*is_signed_in=*/true); RespondToSubscriptionRequestSuccessfully(/*is_signed_in=*/true);
...@@ -293,6 +324,9 @@ TEST_F(SubscriptionManagerImplTest, ...@@ -293,6 +324,9 @@ TEST_F(SubscriptionManagerImplTest,
#if !defined(OS_CHROMEOS) #if !defined(OS_CHROMEOS)
TEST_F(SubscriptionManagerImplTest, TEST_F(SubscriptionManagerImplTest,
ShouldResubscribeIfSignOutAfterSubscription) { ShouldResubscribeIfSignOutAfterSubscription) {
base::RunLoop run_loop;
set_on_access_token_request_callback(run_loop.QuitClosure());
// Signin and subscribe. // Signin and subscribe.
FakeProfileOAuth2TokenService* auth_token_service = GetOAuth2TokenService(); FakeProfileOAuth2TokenService* auth_token_service = GetOAuth2TokenService();
SignIn(); SignIn();
...@@ -303,6 +337,7 @@ TEST_F(SubscriptionManagerImplTest, ...@@ -303,6 +337,7 @@ TEST_F(SubscriptionManagerImplTest,
/*variations_service=*/nullptr, GetSigninManager(), auth_token_service, /*variations_service=*/nullptr, GetSigninManager(), auth_token_service,
/*locale=*/"", kAPIKey, GURL(kSubscriptionUrl), GURL(kUnsubscriptionUrl)); /*locale=*/"", kAPIKey, GURL(kSubscriptionUrl), GURL(kUnsubscriptionUrl));
manager.Subscribe(subscription_token); manager.Subscribe(subscription_token);
run_loop.Run();
ASSERT_EQ(1u, auth_token_service->GetPendingRequests().size()); ASSERT_EQ(1u, auth_token_service->GetPendingRequests().size());
IssueAccessToken(auth_token_service); IssueAccessToken(auth_token_service);
RespondToSubscriptionRequestSuccessfully(/*is_signed_in=*/true); RespondToSubscriptionRequestSuccessfully(/*is_signed_in=*/true);
......
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