Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
[GCM] Fetching OAuth2 tokens periodically in account tracker
Browse files Browse the repository at this point in the history 
BUG=374969
[email protected]

Review URL: https://codereview.chromium.org/631343002

Cr-Commit-Position: refs/heads/master@{#303340}
  • Loading branch information
@fgorski
fgorski authored and Commit bot committed Nov 8, 2014
1 parent b44562c commit ede4117
Show file tree
Hide file tree
Showing 6 changed files with 200 additions and 25 deletions.
90 changes: 72 additions & 18 deletions chrome/browser/services/gcm/gcm_account_tracker.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,18 @@
namespace gcm {

namespace {

// Scopes needed by the OAuth2 access tokens.
const char kGCMGroupServerScope[] = "https://www.googleapis.com/auth/gcm";
const char kGCMCheckinServerScope[] =
"https://www.googleapis.com/auth/android_checkin";
// Name of the GCM account tracker for the OAuth2TokenService.
const char kGCMAccountTrackerName[] = "gcm_account_tracker";
// Minimum token validity when sending to GCM groups server.
const int64 kMinimumTokenValidityMs = 500;
// Token reporting interval, when no account changes are detected.
const int64 kTokenReportingIntervalMs = 12 * 60 * 60 * 1000; // 12 hours in ms.

} // namespace

GCMAccountTracker::AccountInfo::AccountInfo(const std::string& email,
Expand All @@ -36,7 +43,8 @@ GCMAccountTracker::GCMAccountTracker(
: OAuth2TokenService::Consumer(kGCMAccountTrackerName),
account_tracker_(account_tracker.release()),
driver_(driver),
shutdown_called_(false) {
shutdown_called_(false),
reporting_weak_ptr_factory_(this) {
}

GCMAccountTracker::~GCMAccountTracker() {
Expand All @@ -56,11 +64,6 @@ void GCMAccountTracker::Start() {
driver_->AddConnectionObserver(this);

std::vector<gaia::AccountIds> accounts = account_tracker_->GetAccounts();
if (accounts.empty()) {
CompleteCollectingTokens();
return;
}

for (std::vector<gaia::AccountIds>::const_iterator iter = accounts.begin();
iter != accounts.end();
++iter) {
Expand All @@ -70,7 +73,22 @@ void GCMAccountTracker::Start() {
}
}

GetAllNeededTokens();
if (IsTokenReportingRequired())
ReportTokens();
else
ScheduleReportTokens();
}

void GCMAccountTracker::ScheduleReportTokens() {
DVLOG(1) << "Deferring the token reporting for: "
<< GetTimeToNextTokenReporting().InSeconds() << " seconds.";

reporting_weak_ptr_factory_.InvalidateWeakPtrs();
base::MessageLoop::current()->PostDelayedTask(
FROM_HERE,
base::Bind(&GCMAccountTracker::ReportTokens,
reporting_weak_ptr_factory_.GetWeakPtr()),
GetTimeToNextTokenReporting());
}

void GCMAccountTracker::OnAccountAdded(const gaia::AccountIds& ids) {
Expand Down Expand Up @@ -115,7 +133,7 @@ void GCMAccountTracker::OnGetTokenSuccess(
}

DeleteTokenRequest(request);
CompleteCollectingTokens();
ReportTokens();
}

void GCMAccountTracker::OnGetTokenFailure(
Expand All @@ -137,21 +155,22 @@ void GCMAccountTracker::OnGetTokenFailure(
}

DeleteTokenRequest(request);
CompleteCollectingTokens();
ReportTokens();
}

void GCMAccountTracker::OnConnected(const net::IPEndPoint& ip_endpoint) {
if (SanitizeTokens())
GetAllNeededTokens();
if (IsTokenReportingRequired())
ReportTokens();
}

void GCMAccountTracker::OnDisconnected() {
// We are disconnected, so no point in trying to work with tokens.
}

void GCMAccountTracker::CompleteCollectingTokens() {
void GCMAccountTracker::ReportTokens() {
SanitizeTokens();
// Make sure all tokens are valid.
if (SanitizeTokens()) {
if (IsTokenFetchingRequired()) {
GetAllNeededTokens();
return;
}
Expand Down Expand Up @@ -198,13 +217,14 @@ void GCMAccountTracker::CompleteCollectingTokens() {
if (!account_tokens.empty() || account_removed) {
DVLOG(1) << "Reporting the tokens to driver: " << account_tokens.size();
driver_->SetAccountTokens(account_tokens);
driver_->SetLastTokenFetchTime(base::Time::Now());
ScheduleReportTokens();
} else {
DVLOG(1) << "No tokens and nothing removed. Skipping callback.";
}
}

bool GCMAccountTracker::SanitizeTokens() {
bool tokens_needed = false;
void GCMAccountTracker::SanitizeTokens() {
for (AccountInfos::iterator iter = account_infos_.begin();
iter != account_infos_.end();
++iter) {
Expand All @@ -216,12 +236,43 @@ bool GCMAccountTracker::SanitizeTokens() {
iter->second.state = TOKEN_NEEDED;
iter->second.expiration_time = base::Time();
}
}
}

bool GCMAccountTracker::IsTokenReportingRequired() const {
if (GetTimeToNextTokenReporting() == base::TimeDelta())
return true;

bool reporting_required = false;
for (AccountInfos::const_iterator iter = account_infos_.begin();
iter != account_infos_.end();
++iter) {
if (iter->second.state == ACCOUNT_REMOVED)
reporting_required = true;
}

return reporting_required;
}

bool GCMAccountTracker::IsTokenFetchingRequired() const {
bool token_needed = false;
for (AccountInfos::const_iterator iter = account_infos_.begin();
iter != account_infos_.end();
++iter) {
if (iter->second.state == TOKEN_NEEDED)
tokens_needed = true;
token_needed = true;
}

return tokens_needed;
return token_needed;
}

base::TimeDelta GCMAccountTracker::GetTimeToNextTokenReporting() const {
base::TimeDelta time_till_next_reporting =
driver_->GetLastTokenFetchTime() +
base::TimeDelta::FromMilliseconds(kTokenReportingIntervalMs) -
base::Time::Now();
return time_till_next_reporting < base::TimeDelta() ?
base::TimeDelta() : time_till_next_reporting;
}

void GCMAccountTracker::DeleteTokenRequest(
Expand All @@ -235,6 +286,9 @@ void GCMAccountTracker::DeleteTokenRequest(
void GCMAccountTracker::GetAllNeededTokens() {
// Only start fetching tokens if driver is running, they have a limited
// validity time and GCM connection is a good indication of network running.
// If the GetAllNeededTokens was called as part of periodic schedule, it may
// not have network. In that case the next network change will trigger token
// fetching.
if (!driver_->IsConnected())
return;

Expand Down Expand Up @@ -282,7 +336,7 @@ void GCMAccountTracker::OnAccountSignedOut(const gaia::AccountIds& ids) {

iter->second.access_token.clear();
iter->second.state = ACCOUNT_REMOVED;
CompleteCollectingTokens();
ReportTokens();
}

OAuth2TokenService* GCMAccountTracker::GetTokenService() {
Expand Down
28 changes: 26 additions & 2 deletions chrome/browser/services/gcm/gcm_account_tracker.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,21 @@
#include "google_apis/gaia/account_tracker.h"
#include "google_apis/gaia/oauth2_token_service.h"

namespace base {
class Time;
}

namespace gcm {

class GCMDriver;

// Class for reporting back which accounts are signed into. It is only meant to
// be used when the user is signed into sync.
//
// This class makes a check for tokens periodically, to make sure the user is
// still logged into the profile, so that in the case that the user is not, we
// can immediately report that to the GCM and stop messages addressed to that
// user from ever reaching Chrome.
class GCMAccountTracker : public gaia::AccountTracker::Observer,
public OAuth2TokenService::Consumer,
public GCMConnectionObserver {
Expand Down Expand Up @@ -76,6 +85,8 @@ class GCMAccountTracker : public gaia::AccountTracker::Observer,
}

private:
friend class GCMAccountTrackerTest;

// Maps account keys to account states. Keyed by account_ids as used by
// OAuth2TokenService.
typedef std::map<std::string, AccountInfo> AccountInfos;
Expand All @@ -97,13 +108,22 @@ class GCMAccountTracker : public gaia::AccountTracker::Observer,
void OnConnected(const net::IPEndPoint& ip_endpoint) override;
void OnDisconnected() override;

// Schedules token reporting.
void ScheduleReportTokens();
// Report the list of accounts with OAuth2 tokens back using the |callback_|
// function. If there are token requests in progress, do nothing.
void CompleteCollectingTokens();
void ReportTokens();
// Verify that all of the tokens are ready to be passed down to the GCM
// Driver, e.g. none of them has expired or is missing. Returns true if not
// all tokens are valid and a fetching yet more tokens is required.
bool SanitizeTokens();
void SanitizeTokens();
// Indicates whether token reporting is required, either because it is due, or
// some of the accounts were removed.
bool IsTokenReportingRequired() const;
// Indicates whether there are tokens that still need fetching.
bool IsTokenFetchingRequired() const;
// Gets the time until next token reporting.
base::TimeDelta GetTimeToNextTokenReporting() const;
// Deletes a token request. Should be called from OnGetTokenSuccess(..) or
// OnGetTokenFailure(..).
void DeleteTokenRequest(const OAuth2TokenService::Request* request);
Expand Down Expand Up @@ -133,6 +153,10 @@ class GCMAccountTracker : public gaia::AccountTracker::Observer,

ScopedVector<OAuth2TokenService::Request> pending_token_requests_;

// Creates weak pointers used to postpone reporting tokens. See
// ScheduleReportTokens.
base::WeakPtrFactory<GCMAccountTracker> reporting_weak_ptr_factory_;

DISALLOW_COPY_AND_ASSIGN(GCMAccountTracker);
};

Expand Down
90 changes: 87 additions & 3 deletions chrome/browser/services/gcm/gcm_account_tracker_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
#include <string>

#include "base/memory/scoped_ptr.h"
#include "base/message_loop/message_loop.h"
#include "base/run_loop.h"
#include "components/gcm_driver/fake_gcm_driver.h"
#include "google_apis/gaia/fake_identity_provider.h"
#include "google_apis/gaia/fake_oauth2_token_service.h"
Expand Down Expand Up @@ -76,6 +74,8 @@ class CustomFakeGCMDriver : public FakeGCMDriver {
void AddConnectionObserver(GCMConnectionObserver* observer) override;
void RemoveConnectionObserver(GCMConnectionObserver* observer) override;
bool IsConnected() const override { return connected_; }
base::Time GetLastTokenFetchTime() override;
void SetLastTokenFetchTime(const base::Time& time) override;

// Test results and helpers.
void SetConnected(bool connected);
Expand All @@ -98,6 +98,7 @@ class CustomFakeGCMDriver : public FakeGCMDriver {
GCMConnectionObserver* last_connection_observer_;
GCMConnectionObserver* removed_connection_observer_;
net::IPEndPoint ip_endpoint_;
base::Time last_token_fetch_time_;

DISALLOW_COPY_AND_ASSIGN(CustomFakeGCMDriver);
};
Expand Down Expand Up @@ -141,6 +142,15 @@ void CustomFakeGCMDriver::ResetResults() {
removed_connection_observer_ = NULL;
}


base::Time CustomFakeGCMDriver::GetLastTokenFetchTime() {
return last_token_fetch_time_;
}

void CustomFakeGCMDriver::SetLastTokenFetchTime(const base::Time& time) {
last_token_fetch_time_ = time;
}

} // namespace

class GCMAccountTrackerTest : public testing::Test {
Expand All @@ -165,6 +175,11 @@ class GCMAccountTrackerTest : public testing::Test {
GCMAccountTracker* tracker() { return tracker_.get(); }
CustomFakeGCMDriver* driver() { return &driver_; }

// Accessors to private methods of account tracker.
bool IsFetchingRequired() const;
bool IsTokenReportingRequired() const;
base::TimeDelta GetTimeToNextTokenReporting() const;

private:
CustomFakeGCMDriver driver_;

Expand Down Expand Up @@ -237,6 +252,18 @@ void GCMAccountTrackerTest::IssueError(const std::string& account_key) {
GoogleServiceAuthError(GoogleServiceAuthError::SERVICE_UNAVAILABLE));
}

bool GCMAccountTrackerTest::IsFetchingRequired() const {
return tracker_->IsTokenFetchingRequired();
}

bool GCMAccountTrackerTest::IsTokenReportingRequired() const {
return tracker_->IsTokenReportingRequired();
}

base::TimeDelta GCMAccountTrackerTest::GetTimeToNextTokenReporting() const {
return tracker_->GetTimeToNextTokenReporting();
}

TEST_F(GCMAccountTrackerTest, NoAccounts) {
EXPECT_FALSE(driver()->update_accounts_called());
tracker()->Start();
Expand Down Expand Up @@ -403,7 +430,7 @@ TEST_F(GCMAccountTrackerTest, PostponeTokenFetchingUntilConnected) {
EXPECT_EQ(1UL, tracker()->get_pending_token_request_count());
}

TEST_F(GCMAccountTrackerTest, IvalidateExpiredTokens) {
TEST_F(GCMAccountTrackerTest, InvalidateExpiredTokens) {
StartAccountSignIn(kAccountId1);
StartAccountSignIn(kAccountId2);
tracker()->Start();
Expand All @@ -421,6 +448,63 @@ TEST_F(GCMAccountTrackerTest, IvalidateExpiredTokens) {
EXPECT_EQ(1UL, tracker()->get_pending_token_request_count());
}

// Testing for whether there are still more tokens to be fetched. Typically the
// need for token fetching triggers immediate request, unless there is no
// connection, that is why connection is set on and off in this test.
TEST_F(GCMAccountTrackerTest, IsTokenFetchingRequired) {
tracker()->Start();
driver()->SetConnected(false);
EXPECT_FALSE(IsFetchingRequired());
StartAccountSignIn(kAccountId1);
FinishAccountSignIn(kAccountId1);
EXPECT_TRUE(IsFetchingRequired());

driver()->SetConnected(true);
EXPECT_FALSE(IsFetchingRequired()); // Indicates that fetching has started.
IssueAccessToken(kAccountId1);
EXPECT_FALSE(IsFetchingRequired());

driver()->SetConnected(false);
StartAccountSignIn(kAccountId2);
FinishAccountSignIn(kAccountId2);
EXPECT_TRUE(IsFetchingRequired());

IssueExpiredAccessToken(kAccountId2);
// Make sure that if the token was expired it is still needed.
EXPECT_TRUE(IsFetchingRequired());
}

// Tests what is the expected time to the next token fetching.
TEST_F(GCMAccountTrackerTest, GetTimeToNextTokenReporting) {
tracker()->Start();
// At this point the last token fetch time is never.
EXPECT_EQ(base::TimeDelta(), GetTimeToNextTokenReporting());

driver()->SetLastTokenFetchTime(base::Time::Now());
EXPECT_TRUE(GetTimeToNextTokenReporting() <=
base::TimeDelta::FromSeconds(12 * 60 * 60));
}

// Tests conditions when token reporting is required.
TEST_F(GCMAccountTrackerTest, IsTokenReportingRequired) {
tracker()->Start();
// Required because it is overdue.
EXPECT_TRUE(IsTokenReportingRequired());

// Not required because it just happened.
driver()->SetLastTokenFetchTime(base::Time::Now());
EXPECT_FALSE(IsTokenReportingRequired());

SignInAccount(kAccountId1);
IssueAccessToken(kAccountId1);
driver()->ResetResults();
// Reporting was triggered, which means testing for required will give false,
// but we have the update call.
SignOutAccount(kAccountId1);
EXPECT_TRUE(driver()->update_accounts_called());
EXPECT_FALSE(IsTokenReportingRequired());
}

// TODO(fgorski): Add test for adding account after removal >> make sure it does
// not mark removal.

Expand Down
Loading

0 comments on commit ede4117

Please sign in to comment.