Skip to content

Commit

Permalink
Set allow_stale_read flag for speculative GetMyDevices CryptAuth API …
Browse files Browse the repository at this point in the history
…calls.

BUG=512230
TEST=unit test

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

Cr-Commit-Position: refs/heads/master@{#342159}
  • Loading branch information
tengs authored and Commit bot committed Aug 6, 2015
1 parent e672df8 commit 344af25
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,12 @@ void CryptAuthDeviceManager::OnSyncRequested(
int reason_stored_in_prefs =
pref_service_->GetInteger(prefs::kCryptAuthDeviceSyncReason);

// If the sync attempt is not forced, it is acceptable for CryptAuth to return
// a cached copy of the user's devices, rather taking a database hit for the
// freshest data.
bool is_sync_speculative =
reason_stored_in_prefs != cryptauth::INVOCATION_REASON_UNKNOWN;

if (cryptauth::InvocationReason_IsValid(reason_stored_in_prefs) &&
reason_stored_in_prefs != cryptauth::INVOCATION_REASON_UNKNOWN) {
invocation_reason =
Expand All @@ -275,6 +281,7 @@ void CryptAuthDeviceManager::OnSyncRequested(

cryptauth::GetMyDevicesRequest request;
request.set_invocation_reason(invocation_reason);
request.set_allow_stale_read(is_sync_speculative);
cryptauth_client_->GetMyDevices(
request, base::Bind(&CryptAuthDeviceManager::OnGetMyDevicesSuccess,
weak_ptr_factory_.GetWeakPtr()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,12 @@ class ProximityAuthCryptAuthDeviceManagerTest

EXPECT_EQ(expected_invocation_reason,
get_my_devices_request_.invocation_reason());

// The allow_stale_read flag is set if the sync was not forced.
bool allow_stale_read =
pref_service_.GetInteger(prefs::kCryptAuthDeviceSyncReason) !=
cryptauth::INVOCATION_REASON_UNKNOWN;
EXPECT_EQ(allow_stale_read, get_my_devices_request_.allow_stale_read());
}

// MockCryptAuthClientFactory::Observer:
Expand Down Expand Up @@ -444,6 +450,44 @@ TEST_F(ProximityAuthCryptAuthDeviceManagerTest, ForceSyncFailsThenSucceeds) {
prefs::kCryptAuthDeviceSyncIsRecoveringFromFailure));
}

TEST_F(ProximityAuthCryptAuthDeviceManagerTest, PeriodicSyncFailsThenSucceeds) {
device_manager_.Start();
base::Time old_sync_time = device_manager_.GetLastSyncTime();

// The first periodic sync fails.
FireSchedulerForSync(cryptauth::INVOCATION_REASON_PERIODIC);
clock_->SetNow(base::Time::FromDoubleT(kLaterTimeNowSeconds));
EXPECT_CALL(*this,
OnSyncFinishedProxy(
CryptAuthDeviceManager::SyncResult::FAILURE,
CryptAuthDeviceManager::DeviceChangeResult::UNCHANGED));
error_callback_.Run("401");
EXPECT_EQ(old_sync_time, device_manager_.GetLastSyncTime());
EXPECT_TRUE(pref_service_.GetBoolean(
prefs::kCryptAuthDeviceSyncIsRecoveringFromFailure));

// The second recovery sync succeeds.
ON_CALL(*sync_scheduler(), GetStrategy())
.WillByDefault(Return(SyncScheduler::Strategy::AGGRESSIVE_RECOVERY));
FireSchedulerForSync(cryptauth::INVOCATION_REASON_FAILURE_RECOVERY);
clock_->SetNow(base::Time::FromDoubleT(kLaterTimeNowSeconds + 30));
EXPECT_CALL(*this, OnSyncFinishedProxy(
CryptAuthDeviceManager::SyncResult::SUCCESS,
CryptAuthDeviceManager::DeviceChangeResult::CHANGED));
success_callback_.Run(get_my_devices_response_);
EXPECT_EQ(clock_->Now(), device_manager_.GetLastSyncTime());

ExpectUnlockKeysAndPrefAreEqual(std::vector<cryptauth::ExternalDeviceInfo>(
1, get_my_devices_response_.devices(0)),
device_manager_.unlock_keys(), pref_service_);

EXPECT_FLOAT_EQ(
clock_->Now().ToDoubleT(),
pref_service_.GetDouble(prefs::kCryptAuthDeviceSyncLastSyncTimeSeconds));
EXPECT_FALSE(pref_service_.GetBoolean(
prefs::kCryptAuthDeviceSyncIsRecoveringFromFailure));
}

TEST_F(ProximityAuthCryptAuthDeviceManagerTest, SyncSameDevice) {
// Set the same unlock key in the user prefs as the one that would be synced.
{
Expand Down

0 comments on commit 344af25

Please sign in to comment.