Commit b09b9b4d authored by James Cook's avatar James Cook Committed by Commit Bot

cros: Migrate PrivetNotificationService to consent aware identity API

PrivetNotificationService uses IdentityManager::HasPrimaryAccount
to detect if a Profile is for a signed-in user. However, that method
returns false if the user has not consented to browser sync.

Historically all Chrome OS users were consented to browser sync, but
the SplitSettingsSync project is making it possible for users to
opt-out.

Change the call site to the new API, indicating that browser sync
consent is not required. The updated call returns true for any
signed-in user.

See go/cros-primary-account and go/consent-aware-api-dd

Also fix the startup delay timing, which was trying to generate
a TimeDelta between 0 and 1.25 seconds, but always chose exactly
0 or exactly 1.

Bug: 349098, 1042400, 1046746
Test: added to unit_tests
Change-Id: Ic7484428cd718937173d5097e57672d74c8db614
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2055963Reviewed-by: default avatarLei Zhang <thestig@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#741299}
parent 906ab607
......@@ -33,6 +33,7 @@
#include "chrome/grit/generated_resources.h"
#include "chrome/grit/theme_resources.h"
#include "components/prefs/pref_service.h"
#include "components/signin/public/identity_manager/consent_level.h"
#include "components/signin/public/identity_manager/identity_manager.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/navigation_controller.h"
......@@ -57,7 +58,6 @@ const int kTenMinutesInSeconds = 600;
const char kPrivetInfoKeyUptime[] = "uptime";
const char kPrivetNotificationID[] = "privet_notification";
const char kPrivetNotificationOriginUrl[] = "chrome://devices";
const int kStartDelaySeconds = 5;
} // namespace
......@@ -179,13 +179,15 @@ PrivetNotificationsListener::DeviceContext::DeviceContext() {
PrivetNotificationsListener::DeviceContext::~DeviceContext() {
}
// static
constexpr base::TimeDelta PrivetNotificationService::kStartDelay;
PrivetNotificationService::PrivetNotificationService(
content::BrowserContext* profile)
: profile_(profile) {
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
FROM_HERE, base::BindOnce(&PrivetNotificationService::Start, AsWeakPtr()),
base::TimeDelta::FromSeconds(kStartDelaySeconds +
base::RandInt(0, kStartDelaySeconds / 4)));
kStartDelay + base::TimeDelta::FromMilliseconds(base::RandInt(0, 1000)));
}
PrivetNotificationService::~PrivetNotificationService() {
......@@ -287,8 +289,11 @@ void PrivetNotificationService::Start() {
auto* identity_manager = IdentityManagerFactory::GetForProfileIfExists(
Profile::FromBrowserContext(profile_));
if (!identity_manager || !identity_manager->HasPrimaryAccount())
// Only show notifications for signed-in accounts. https://crbug.com/349098
if (!identity_manager || !identity_manager->HasPrimaryAccount(
signin::ConsentLevel::kNotRequired)) {
return;
}
#endif // defined(OS_CHROMEOS)
enable_privet_notification_member_.Init(
......
......@@ -10,6 +10,7 @@
#include <set>
#include <string>
#include "base/time/time.h"
#include "chrome/browser/printing/cloud_print/privet_device_lister.h"
#include "chrome/browser/printing/cloud_print/privet_http.h"
#include "components/keyed_service/core/keyed_service.h"
......@@ -101,6 +102,10 @@ class PrivetNotificationService
public PrivetNotificationsListener::Delegate,
public base::SupportsWeakPtr<PrivetNotificationService> {
public:
// Visible for testing.
static constexpr base::TimeDelta kStartDelay =
base::TimeDelta::FromSeconds(5);
explicit PrivetNotificationService(content::BrowserContext* profile);
~PrivetNotificationService() override;
......@@ -117,6 +122,11 @@ class PrivetNotificationService
static bool IsEnabled();
static bool IsForced();
PrivetDeviceLister* device_lister_for_test() { return device_lister_.get(); }
PrivetTrafficDetector* traffic_detector_for_test() {
return traffic_detector_.get();
}
private:
void Start();
void OnNotificationsEnabledChanged();
......
......@@ -320,6 +320,24 @@ TEST_F(PrivetNotificationsNotificationTest, DontShowAgain) {
.size());
}
#if defined(OS_CHROMEOS)
TEST(PrivetNotificationServiceTest, NoNotificationsInGuestMode) {
content::BrowserTaskEnvironment task_environment{
base::test::TaskEnvironment::TimeSource::MOCK_TIME};
TestingProfileManager profile_manager(TestingBrowserProcess::GetGlobal());
ASSERT_TRUE(profile_manager.SetUp());
Profile* profile = profile_manager.CreateGuestProfile();
TestPrivetNotificationService service(profile);
// Wait for delayed initialization.
task_environment.FastForwardBy(PrivetNotificationService::kStartDelay * 2);
// We're not watching for printers.
EXPECT_FALSE(service.device_lister_for_test());
EXPECT_FALSE(service.traffic_detector_for_test());
}
#endif // defined(OS_CHROMEOS)
} // namespace
} // namespace cloud_print
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment