Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed device duplication when Google Account cookies are deleted (uplift to 1.71.x) #26087

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions components/sync/service/brave_sync_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -417,4 +417,13 @@ void BraveSyncServiceImpl::StopAndClearWithResetLocalDataReason() {
StopAndClear(ResetEngineReason::kResetLocalData);
}

void BraveSyncServiceImpl::OnAccountsCookieDeletedByUserAction() {}

void BraveSyncServiceImpl::OnAccountsInCookieUpdated(
const signin::AccountsInCookieJarInfo& accounts_in_cookie_jar_info,
const GoogleServiceAuthError& error) {}

void BraveSyncServiceImpl::OnPrimaryAccountChanged(
const signin::PrimaryAccountChangeEvent& event_details) {}

} // namespace syncer
12 changes: 12 additions & 0 deletions components/sync/service/brave_sync_service_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ class BraveSyncServiceImpl : public SyncServiceImpl {
void StopAndClearWithResetLocalDataReason();

private:
friend class BraveSyncServiceImplGACookiesTest;
friend class BraveSyncServiceImplTest;
FRIEND_TEST_ALL_PREFIXES(BraveSyncServiceImplTest,
ForcedSetDecryptionPassphrase);
Expand Down Expand Up @@ -115,6 +116,17 @@ class BraveSyncServiceImpl : public SyncServiceImpl {
void UpdateP3AObjectsNumber();
void OnGetTypeEntitiesCount(const TypeEntitiesCount& count);

// IdentityManager::Observer implementation.
// Override with an empty implementation.
// We need this to avoid device cache guid regeneration once any Google
// Account cookie gets deleted, for example when user signs out from GMail
void OnAccountsCookieDeletedByUserAction() override;
void OnAccountsInCookieUpdated(
const signin::AccountsInCookieJarInfo& accounts_in_cookie_jar_info,
const GoogleServiceAuthError& error) override;
void OnPrimaryAccountChanged(
const signin::PrimaryAccountChangeEvent& event_details) override;

brave_sync::Prefs brave_sync_prefs_;

PrefChangeRegistrar brave_sync_prefs_change_registrar_;
Expand Down
98 changes: 96 additions & 2 deletions components/sync/service/brave_sync_service_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,20 @@
#include "components/os_crypt/sync/os_crypt.h"
#include "components/os_crypt/sync/os_crypt_mocker.h"
#include "components/prefs/pref_change_registrar.h"
#include "components/signin/public/identity_manager/accounts_in_cookie_jar_info.h"
#include "components/sync/engine/nigori/key_derivation_params.h"
#include "components/sync/engine/nigori/nigori.h"
#include "components/sync/model/type_entities_count.h"
#include "components/sync/service/data_type_manager_impl.h"
#include "components/sync/service/glue/sync_transport_data_prefs.h"
#include "components/sync/test/data_type_manager_mock.h"
#include "components/sync/test/fake_data_type_controller.h"
#include "components/sync/test/fake_sync_engine.h"
#include "components/sync/test/fake_sync_engine_factory.h"
#include "components/sync/test/fake_sync_manager.h"
#include "components/sync/test/sync_service_impl_bundle.h"
#include "components/sync/test/test_data_type_store_service.h"
#include "components/sync_preferences/testing_pref_service_syncable.h"
#include "content/public/test/browser_task_environment.h"
#include "testing/gtest/include/gtest/gtest.h"

Expand All @@ -45,7 +48,9 @@ namespace syncer {

namespace {

const char kValidSyncCode[] =
constexpr char kCacheGuid[] = "cache_guid";

constexpr char kValidSyncCode[] =
"fringe digital begin feed equal output proof cheap "
"exotic ill sure question trial squirrel glove celery "
"awkward push jelly logic broccoli almost grocery drift";
Expand Down Expand Up @@ -126,7 +131,7 @@ class BraveSyncServiceImplTest : public testing::Test {

SyncPrefs* sync_prefs() { return &sync_prefs_; }

PrefService* pref_service() {
sync_preferences::TestingPrefServiceSyncable* pref_service() {
return sync_service_impl_bundle_.pref_service();
}

Expand Down Expand Up @@ -661,4 +666,93 @@ TEST_F(BraveSyncServiceImplTest, NoLeaveDetailsWhenInitializeIOS) {
EXPECT_FALSE(brave_sync_prefs()->GetLeaveChainDetails().empty());
}

namespace {
enum class GACookiesMethodType {
kOnAccountsCookieDeletedByUserAction,
kOnAccountsInCookieUpdated,
kOnPrimaryAccountChanged
};
}

class BraveSyncServiceImplGACookiesTest
: public BraveSyncServiceImplTest,
public testing::WithParamInterface<GACookiesMethodType> {
public:
base::OnceCallback<void(void)> GetGACookiesMethod() {
switch (GetParam()) {
case GACookiesMethodType::kOnAccountsCookieDeletedByUserAction:
return base::BindOnce(
[](BraveSyncServiceImplGACookiesTest* p_this) {
p_this->brave_sync_service_impl()
->OnAccountsCookieDeletedByUserAction();
},
this);
case GACookiesMethodType::kOnAccountsInCookieUpdated:
return base::BindOnce(
[](BraveSyncServiceImplGACookiesTest* p_this) {
p_this->brave_sync_service_impl()->OnAccountsInCookieUpdated(
signin::AccountsInCookieJarInfo(), GoogleServiceAuthError());
},
this);
case GACookiesMethodType::kOnPrimaryAccountChanged:
return base::BindOnce(
[](BraveSyncServiceImplGACookiesTest* p_this) {
CoreAccountInfo prev_account_info;
prev_account_info.email = "usertest@gmail.com";
prev_account_info.gaia = "gaia";
prev_account_info.account_id =
CoreAccountId::FromGaiaId(prev_account_info.gaia);

signin::PrimaryAccountChangeEvent primary_accountchange_event(
signin::PrimaryAccountChangeEvent::State(
prev_account_info, signin::ConsentLevel::kSignin),
signin::PrimaryAccountChangeEvent::State(),
signin_metrics::ProfileSignout::kTest);

p_this->brave_sync_service_impl()->OnPrimaryAccountChanged(
primary_accountchange_event);
},
this);
default:
NOTREACHED_NORETURN();
}
}
};

TEST_P(BraveSyncServiceImplGACookiesTest, CacheGuidIsNotWiped) {
SyncTransportDataPrefs::RegisterProfilePrefs(pref_service()->registry());

SyncTransportDataPrefs sync_transport_data_prefs(
pref_service(), signin::GaiaIdHash::FromGaiaId("user_gaia_id"));

sync_transport_data_prefs.SetCacheGuid(kCacheGuid);

CreateSyncService();

std::move(GetGACookiesMethod()).Run();

EXPECT_EQ(sync_transport_data_prefs.GetCacheGuid(), kCacheGuid);
}

INSTANTIATE_TEST_SUITE_P(
All,
BraveSyncServiceImplGACookiesTest,
testing::ValuesIn(
{GACookiesMethodType::kOnAccountsCookieDeletedByUserAction,
GACookiesMethodType::kOnAccountsInCookieUpdated,
GACookiesMethodType::kOnPrimaryAccountChanged}),
[](const testing::TestParamInfo<
BraveSyncServiceImplGACookiesTest::ParamType>& info) {
switch (info.param) {
case GACookiesMethodType::kOnAccountsCookieDeletedByUserAction:
return "OnAccountsCookieDeletedByUserAction";
case GACookiesMethodType::kOnAccountsInCookieUpdated:
return "OnAccountsInCookieUpdated";
case GACookiesMethodType::kOnPrimaryAccountChanged:
return "OnPrimaryAccountChanged";
default:
NOTREACHED_NORETURN();
}
});

} // namespace syncer
Loading