Skip to content

Commit

Permalink
Always enable send-tab-to-self receiving if user has sync consent
Browse files Browse the repository at this point in the history
This change concerns devices that consented to sync but:
a) disabled "Open tabs" in chrome://settings/syncSetup/advanced; or
b) have SyncRequested set to false (mainly one legacy state on mobile).
Those devices will now be valid targets for the send-tab-to-self feature
(STTS) instead of being filtered. The rationale can be found in the
linked bug.

The change is landed behind a kill switch because it will cause
DEVICE_INFO commits when the send_tab_to_receiving_enabled field [1] is
set to true.

I will note that even though all modern clients now set this field to
true, there's no plan to remove it, because:
a) it allows unlaunching STTS if ever needed; and
b) versions predating STTS must be filtered anyway (the field defaults
to false).

[1] https://source.chromium.org/chromium/chromium/src/+/main:components/sync/protocol/device_info_specifics.proto;l=103;drc=166e3c6b0b6418cf5a08713861fc15dbcd20c009

Bug: 1299833
Change-Id: Ifcfec1fc8dc224b5a358f177a7086dcb90541a52
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3483939
Commit-Queue: Victor Vianna <victorvianna@google.com>
Auto-Submit: Victor Vianna <victorvianna@google.com>
Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Commit-Queue: Mikel Astiz <mastiz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#974170}
  • Loading branch information
Victor Hugo Vianna Silva authored and Chromium LUCI CQ committed Feb 23, 2022
1 parent 0bbeb9d commit 41153d8
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 31 deletions.
8 changes: 6 additions & 2 deletions chrome/browser/sync/device_info_sync_client_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <string>
#include <utility>

#include "base/feature_list.h"
#include "build/build_config.h"
#include "build/chromeos_buildflags.h"
#include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
Expand All @@ -16,6 +17,7 @@
#include "chrome/browser/signin/chrome_device_id_helper.h"
#include "chrome/browser/sync/sync_invalidations_service_factory.h"
#include "components/send_tab_to_self/features.h"
#include "components/sync/base/features.h"
#include "components/sync/base/sync_prefs.h"
#include "components/sync/invalidations/sync_invalidations_service.h"

Expand Down Expand Up @@ -51,8 +53,10 @@ std::string DeviceInfoSyncClientImpl::GetSigninScopedDeviceId() const {

// syncer::DeviceInfoSyncClient:
bool DeviceInfoSyncClientImpl::GetSendTabToSelfReceivingEnabled() const {
return send_tab_to_self::IsReceivingEnabledByUserOnThisDevice(
profile_->GetPrefs());
return base::FeatureList::IsEnabled(syncer::kAlwaysReceiveSendTabToSelf)
? true
: send_tab_to_self::IsReceivingEnabledByUserOnThisDevice(
profile_->GetPrefs());
}

// syncer::DeviceInfoSyncClient:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "base/callback_list.h"
#include "base/run_loop.h"
#include "base/test/scoped_feature_list.h"
#include "build/build_config.h"
#include "build/chromeos_buildflags.h"
#include "chrome/browser/history/history_service_factory.h"
Expand All @@ -20,6 +21,7 @@
#include "components/send_tab_to_self/send_tab_to_self_model.h"
#include "components/send_tab_to_self/send_tab_to_self_sync_service.h"
#include "components/send_tab_to_self/target_device_info.h"
#include "components/sync/base/features.h"
#include "components/sync_device_info/device_info.h"
#include "components/sync_device_info/device_info_sync_service.h"
#include "components/sync_device_info/local_device_info_provider.h"
Expand Down Expand Up @@ -146,34 +148,6 @@ IN_PROC_BROWSER_TEST_F(TwoClientSendTabToSelfSyncTest,
EXPECT_TRUE(device_infos[1]->send_tab_to_self_receiving_enabled());
}

IN_PROC_BROWSER_TEST_F(TwoClientSendTabToSelfSyncTest,
SendTabToSelfReceivingDisabled) {
ASSERT_TRUE(SetupSync());
GetClient(0)->DisableSyncForType(syncer::UserSelectableType::kTabs);

DeviceInfoSyncServiceFactory::GetForProfile(GetProfile(0))
->GetDeviceInfoTracker()
->ForcePulseForTest();

ASSERT_TRUE(send_tab_to_self_helper::SendTabToSelfDeviceDisabledChecker(
DeviceInfoSyncServiceFactory::GetForProfile(GetProfile(1))
->GetDeviceInfoTracker(),
DeviceInfoSyncServiceFactory::GetForProfile(GetProfile(0))
->GetLocalDeviceInfoProvider()
->GetLocalDeviceInfo()
->guid())
.Wait());

std::vector<std::unique_ptr<syncer::DeviceInfo>> device_infos =
DeviceInfoSyncServiceFactory::GetForProfile(GetProfile(1))
->GetDeviceInfoTracker()
->GetAllDeviceInfo();
EXPECT_EQ(2u, device_infos.size());

EXPECT_NE(device_infos[0]->send_tab_to_self_receiving_enabled(),
device_infos[1]->send_tab_to_self_receiving_enabled());
}

IN_PROC_BROWSER_TEST_F(TwoClientSendTabToSelfSyncTest,
SendTabToSelfTargetDeviceInfoList) {
ASSERT_TRUE(SetupSync());
Expand Down Expand Up @@ -335,4 +309,44 @@ IN_PROC_BROWSER_TEST_F(TwoClientSendTabToSelfWithTransportModeSyncTest,
EXPECT_TRUE(device_infos[0]->send_tab_to_self_receiving_enabled());
EXPECT_TRUE(device_infos[1]->send_tab_to_self_receiving_enabled());
}

class TwoClientSendTabToSelfSyncTestWithAlwaysReceiveDisabled
: public TwoClientSendTabToSelfSyncTest {
public:
TwoClientSendTabToSelfSyncTestWithAlwaysReceiveDisabled() {
feature_list_.InitAndDisableFeature(syncer::kAlwaysReceiveSendTabToSelf);
}

private:
base::test::ScopedFeatureList feature_list_;
};

IN_PROC_BROWSER_TEST_F(TwoClientSendTabToSelfSyncTestWithAlwaysReceiveDisabled,
SendTabToSelfReceivingDisabled) {
ASSERT_TRUE(SetupSync());
GetClient(0)->DisableSyncForType(syncer::UserSelectableType::kTabs);

DeviceInfoSyncServiceFactory::GetForProfile(GetProfile(0))
->GetDeviceInfoTracker()
->ForcePulseForTest();

ASSERT_TRUE(send_tab_to_self_helper::SendTabToSelfDeviceDisabledChecker(
DeviceInfoSyncServiceFactory::GetForProfile(GetProfile(1))
->GetDeviceInfoTracker(),
DeviceInfoSyncServiceFactory::GetForProfile(GetProfile(0))
->GetLocalDeviceInfoProvider()
->GetLocalDeviceInfo()
->guid())
.Wait());

std::vector<std::unique_ptr<syncer::DeviceInfo>> device_infos =
DeviceInfoSyncServiceFactory::GetForProfile(GetProfile(1))
->GetDeviceInfoTracker()
->GetAllDeviceInfo();
EXPECT_EQ(2u, device_infos.size());

EXPECT_NE(device_infos[0]->send_tab_to_self_receiving_enabled(),
device_infos[1]->send_tab_to_self_receiving_enabled());
}

#endif // !BUILDFLAG(IS_CHROMEOS_ASH)
5 changes: 5 additions & 0 deletions components/sync/base/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ inline constexpr base::Feature kAllowSilentTrustedVaultDeviceRegistration{
"AllowSilentTrustedVaultDeviceRegistration",
base::FEATURE_ENABLED_BY_DEFAULT};

// Causes the device to advertise itself as a send-tab-to-self target regardless
// of UserSelectableType::kTabs and SyncRequested.
inline constexpr base::Feature kAlwaysReceiveSendTabToSelf{
"AlwaysReceiveSendTabToSelf", base::FEATURE_ENABLED_BY_DEFAULT};

// Causes Sync to ignore updates encrypted with keys that have been missing for
// too long from this client; Sync will proceed normally as if those updates
// didn't exist.
Expand Down
6 changes: 5 additions & 1 deletion ios/chrome/browser/sync/device_info_sync_service_factory.mm
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@
#include <utility>

#include "base/bind.h"
#include "base/feature_list.h"
#include "base/memory/singleton.h"
#include "base/time/default_clock.h"
#include "components/keyed_service/core/service_access_type.h"
#include "components/keyed_service/ios/browser_state_dependency_manager.h"
#include "components/send_tab_to_self/features.h"
#include "components/signin/public/base/device_id_helper.h"
#include "components/sync/base/features.h"
#include "components/sync/invalidations/sync_invalidations_service.h"
#include "components/sync/model/model_type_store_service.h"
#include "components/sync_device_info/device_info_prefs.h"
Expand Down Expand Up @@ -48,7 +50,9 @@

// syncer::DeviceInfoSyncClient:
bool GetSendTabToSelfReceivingEnabled() const override {
return send_tab_to_self::IsReceivingEnabledByUserOnThisDevice(prefs_);
return base::FeatureList::IsEnabled(syncer::kAlwaysReceiveSendTabToSelf)
? true
: send_tab_to_self::IsReceivingEnabledByUserOnThisDevice(prefs_);
}

// syncer::DeviceInfoSyncClient:
Expand Down

0 comments on commit 41153d8

Please sign in to comment.