Skip to content

Commit

Permalink
[GCM] Add flags for controlling account tokens w/CheckIn requests
Browse files Browse the repository at this point in the history
Bug: 1354621
Change-Id: Ibe3ddbf660f83189760b28ad44f13440fcb78139
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3842568
Commit-Queue: Rayan Kanso <rayankans@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1037143}
  • Loading branch information
rayankans authored and Chromium LUCI CQ committed Aug 19, 2022
1 parent 4538ca3 commit c62b328
Show file tree
Hide file tree
Showing 6 changed files with 83 additions and 5 deletions.
8 changes: 8 additions & 0 deletions components/gcm_driver/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ namespace features {

const base::Feature kInvalidateTokenFeature{"GCMTokenInvalidAfterDays",
base::FEATURE_ENABLED_BY_DEFAULT};

const base::Feature kGCMIncludeAccountTokensInCheckinRequest{
"GCMIncludeAccountTokensInCheckinRequest",
base::FEATURE_ENABLED_BY_DEFAULT};

const base::Feature kGCMReportAccountTokenChanges{
"GCMReportAccountTokenChanges", base::FEATURE_ENABLED_BY_DEFAULT};

const char kParamNameTokenInvalidationPeriodDays[] =
"token_invalidation_period";
// A token invalidation period of 0 means the feature is disabled, and the
Expand Down
2 changes: 2 additions & 0 deletions components/gcm_driver/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ namespace gcm {
namespace features {

extern const base::Feature kInvalidateTokenFeature;
extern const base::Feature kGCMIncludeAccountTokensInCheckinRequest;
extern const base::Feature kGCMReportAccountTokenChanges;
extern const char kParamNameTokenInvalidationPeriodDays[];

// The period after which the GCM token becomes stale.
Expand Down
5 changes: 5 additions & 0 deletions components/gcm_driver/gcm_account_mapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/metrics/histogram_functions.h"
#include "base/time/clock.h"
#include "base/time/default_clock.h"
#include "components/gcm_driver/features.h"
#include "components/gcm_driver/gcm_driver_desktop.h"
#include "google_apis/gcm/engine/gcm_store.h"

Expand Down Expand Up @@ -107,6 +108,10 @@ void GCMAccountMapper::SetAccountTokens(
}
}

if (!base::FeatureList::IsEnabled(features::kGCMReportAccountTokenChanges)) {
return;
}

// Decide what to do with each account (either start mapping, or start
// removing).
for (auto mappings_iter = accounts_.begin(); mappings_iter != accounts_.end();
Expand Down
15 changes: 10 additions & 5 deletions components/gcm_driver/gcm_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -677,11 +677,16 @@ void GCMClientImpl::StartCheckin() {

checkin_proto::ChromeBuildProto chrome_build_proto;
ToCheckinProtoVersion(chrome_build_info_, &chrome_build_proto);
CheckinRequest::RequestInfo request_info(device_checkin_info_.android_id,
device_checkin_info_.secret,
device_checkin_info_.account_tokens,
gservices_settings_.digest(),
chrome_build_proto);

std::map<std::string, std::string> empty_account_tokens;
bool include_account_tokens = base::FeatureList::IsEnabled(
features::kGCMIncludeAccountTokensInCheckinRequest);

CheckinRequest::RequestInfo request_info(
device_checkin_info_.android_id, device_checkin_info_.secret,
include_account_tokens ? device_checkin_info_.account_tokens
: empty_account_tokens,
gservices_settings_.digest(), chrome_build_proto);
checkin_request_ = std::make_unique<CheckinRequest>(
gservices_settings_.GetCheckinURL(), request_info, GetGCMBackoffPolicy(),
base::BindOnce(&GCMClientImpl::OnCheckinCompleted,
Expand Down
54 changes: 54 additions & 0 deletions components/gcm_driver/gcm_client_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "google_apis/gcm/base/fake_encryptor.h"
#include "google_apis/gcm/base/mcs_message.h"
#include "google_apis/gcm/base/mcs_util.h"
#include "google_apis/gcm/engine/checkin_request.h"
#include "google_apis/gcm/engine/fake_connection_factory.h"
#include "google_apis/gcm/engine/fake_connection_handler.h"
#include "google_apis/gcm/engine/gservices_settings.h"
Expand Down Expand Up @@ -364,6 +365,14 @@ class GCMClientImplTest : public testing::Test,
return gcm_client_->device_checkin_info_;
}

const CheckinRequest& checkin_request() const {
return *gcm_client_->checkin_request_;
}

base::test::ScopedFeatureList& scoped_feature_list() {
return scoped_feature_list_;
}

void reset_last_event() {
last_event_ = NONE;
last_app_id_.clear();
Expand Down Expand Up @@ -1280,6 +1289,51 @@ TEST_F(GCMClientImplCheckinTest, CheckinWithAccounts) {
EXPECT_TRUE(device_checkin_info().accounts_set);
EXPECT_EQ(MakeEmailToTokenMap(account_tokens),
device_checkin_info().account_tokens);

// Make sure the checkin request has the account info.
EXPECT_EQ(checkin_request().request_info_.account_tokens.size(), 2u);
}

// This test only checks that periodic checkin happens.
TEST_F(GCMClientImplCheckinTest, CheckinWithAccountsEmptyWithFeature) {
scoped_feature_list().InitAndDisableFeature(
features::kGCMIncludeAccountTokensInCheckinRequest);

std::map<std::string, std::string> settings;
settings["checkin_interval"] = base::NumberToString(kSettingsCheckinInterval);
settings["checkin_url"] = "http://alternative.url/checkin";
settings["gcm_hostname"] = "alternative.gcm.host";
settings["gcm_secure_port"] = "7777";
settings["gcm_registration_url"] = "http://alternative.url/registration";
ASSERT_NO_FATAL_FAILURE(
CompleteCheckin(kDeviceAndroidId, kDeviceSecurityToken,
GServicesSettings::CalculateDigest(settings), settings));

std::vector<GCMClient::AccountTokenInfo> account_tokens;
account_tokens.push_back(MakeAccountToken("test_user1@gmail.com", "token1"));
account_tokens.push_back(MakeAccountToken("test_user2@gmail.com", "token2"));
gcm_client()->SetAccountTokens(account_tokens);

EXPECT_TRUE(device_checkin_info().last_checkin_accounts.empty());
EXPECT_TRUE(device_checkin_info().accounts_set);
EXPECT_EQ(MakeEmailToTokenMap(account_tokens),
device_checkin_info().account_tokens);

PumpLoopUntilIdle();
ASSERT_NO_FATAL_FAILURE(
CompleteCheckin(kDeviceAndroidId, kDeviceSecurityToken,
GServicesSettings::CalculateDigest(settings), settings));

std::set<std::string> accounts;
accounts.insert("test_user1@gmail.com");
accounts.insert("test_user2@gmail.com");
EXPECT_EQ(accounts, device_checkin_info().last_checkin_accounts);
EXPECT_TRUE(device_checkin_info().accounts_set);
EXPECT_EQ(MakeEmailToTokenMap(account_tokens),
device_checkin_info().account_tokens);

// Make sure the checkin request does not have the account info.
EXPECT_TRUE(checkin_request().request_info_.account_tokens.empty());
}

// This test only checks that periodic checkin happens.
Expand Down
4 changes: 4 additions & 0 deletions google_apis/gcm/engine/checkin_request.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <string>

#include "base/callback.h"
#include "base/gtest_prod_util.h"
#include "base/memory/raw_ptr.h"
#include "base/memory/weak_ptr.h"
#include "base/time/time.h"
Expand Down Expand Up @@ -84,6 +85,9 @@ class GCM_EXPORT CheckinRequest {
std::unique_ptr<std::string> body);

private:
FRIEND_TEST_ALL_PREFIXES(GCMClientImplCheckinTest, CheckinWithAccounts);
FRIEND_TEST_ALL_PREFIXES(GCMClientImplCheckinTest,
CheckinWithAccountsEmptyWithFeature);
// Schedules a retry attempt with a backoff.
void RetryWithBackoff();

Expand Down

0 comments on commit c62b328

Please sign in to comment.