Commit 50c4372d authored by Kyle Horimoto's avatar Kyle Horimoto Committed by Chromium LUCI CQ

[CrOS PhoneHub] Add support for prohibited notification access

Notification access is not allowed for users using a Work Profile. This
CL updates NotificationAccessManager to support this case, and it
updates PhoneStatusProcessor to mark access as prohibited if a Work
Profile is active.

This CL also propagates this value to settings so that the notifications
settings UI can be disabled in this case.

Bug: 1155151, 1106937
Change-Id: I76cc99e17eca333f695c4786b387e5e419a7350a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2573681
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: default avatarJimmy Gong <jimmyxgong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#834085}
parent 0e7b21d0
...@@ -156,8 +156,13 @@ void NotificationOptInView::InitLayout() { ...@@ -156,8 +156,13 @@ void NotificationOptInView::InitLayout() {
void NotificationOptInView::UpdateVisibility() { void NotificationOptInView::UpdateVisibility() {
DCHECK(notification_access_manager_); DCHECK(notification_access_manager_);
// Can only request access if it is available but has not yet been granted.
bool can_request_access = notification_access_manager_->GetAccessStatus() ==
chromeos::phonehub::NotificationAccessManager::
AccessStatus::kAvailableButNotGranted;
const bool should_show = const bool should_show =
!notification_access_manager_->HasAccessBeenGranted() && can_request_access &&
!notification_access_manager_->HasNotificationSetupUiBeenDismissed(); !notification_access_manager_->HasNotificationSetupUiBeenDismissed();
SetVisible(should_show); SetVisible(should_show);
} }
......
...@@ -205,7 +205,9 @@ TEST_F(PhoneHubTrayTest, FocusBubbleWhenOpenedByKeyboard) { ...@@ -205,7 +205,9 @@ TEST_F(PhoneHubTrayTest, FocusBubbleWhenOpenedByKeyboard) {
} }
TEST_F(PhoneHubTrayTest, ShowNotificationOptInViewWhenAccessNotGranted) { TEST_F(PhoneHubTrayTest, ShowNotificationOptInViewWhenAccessNotGranted) {
GetNotificationAccessManager()->SetHasAccessBeenGrantedInternal(false); GetNotificationAccessManager()->SetAccessStatusInternal(
chromeos::phonehub::NotificationAccessManager::AccessStatus::
kAvailableButNotGranted);
ClickTrayButton(); ClickTrayButton();
...@@ -223,7 +225,19 @@ TEST_F(PhoneHubTrayTest, ShowNotificationOptInViewWhenAccessNotGranted) { ...@@ -223,7 +225,19 @@ TEST_F(PhoneHubTrayTest, ShowNotificationOptInViewWhenAccessNotGranted) {
} }
TEST_F(PhoneHubTrayTest, HideNotificationOptInViewWhenAccessHasBeenGranted) { TEST_F(PhoneHubTrayTest, HideNotificationOptInViewWhenAccessHasBeenGranted) {
GetNotificationAccessManager()->SetHasAccessBeenGrantedInternal(true); GetNotificationAccessManager()->SetAccessStatusInternal(
chromeos::phonehub::NotificationAccessManager::AccessStatus::
kAccessGranted);
ClickTrayButton();
EXPECT_TRUE(notification_opt_in_view());
EXPECT_FALSE(notification_opt_in_view()->GetVisible());
}
TEST_F(PhoneHubTrayTest, HideNotificationOptInViewWhenAccessIsProhibited) {
GetNotificationAccessManager()->SetAccessStatusInternal(
chromeos::phonehub::NotificationAccessManager::AccessStatus::kProhibited);
ClickTrayButton(); ClickTrayButton();
...@@ -232,7 +246,9 @@ TEST_F(PhoneHubTrayTest, HideNotificationOptInViewWhenAccessHasBeenGranted) { ...@@ -232,7 +246,9 @@ TEST_F(PhoneHubTrayTest, HideNotificationOptInViewWhenAccessHasBeenGranted) {
} }
TEST_F(PhoneHubTrayTest, StartNotificationSetUpFlow) { TEST_F(PhoneHubTrayTest, StartNotificationSetUpFlow) {
GetNotificationAccessManager()->SetHasAccessBeenGrantedInternal(false); GetNotificationAccessManager()->SetAccessStatusInternal(
chromeos::phonehub::NotificationAccessManager::AccessStatus::
kAvailableButNotGranted);
ClickTrayButton(); ClickTrayButton();
EXPECT_TRUE(notification_opt_in_view()); EXPECT_TRUE(notification_opt_in_view());
...@@ -251,13 +267,17 @@ TEST_F(PhoneHubTrayTest, StartNotificationSetUpFlow) { ...@@ -251,13 +267,17 @@ TEST_F(PhoneHubTrayTest, StartNotificationSetUpFlow) {
ClickOnAndWait(notification_opt_in_set_up_button()); ClickOnAndWait(notification_opt_in_set_up_button());
// Simulate that notification access has been granted. // Simulate that notification access has been granted.
GetNotificationAccessManager()->SetHasAccessBeenGrantedInternal(true); GetNotificationAccessManager()->SetAccessStatusInternal(
chromeos::phonehub::NotificationAccessManager::AccessStatus::
kAccessGranted);
// This view should be dismissed. // This view should be dismissed.
EXPECT_FALSE(notification_opt_in_view()->GetVisible()); EXPECT_FALSE(notification_opt_in_view()->GetVisible());
// Simulate that notification access has been revoked by the phone. // Simulate that notification access has been revoked by the phone.
GetNotificationAccessManager()->SetHasAccessBeenGrantedInternal(false); GetNotificationAccessManager()->SetAccessStatusInternal(
chromeos::phonehub::NotificationAccessManager::AccessStatus::
kAvailableButNotGranted);
// This view should show up again. // This view should show up again.
EXPECT_TRUE(notification_opt_in_view()->GetVisible()); EXPECT_TRUE(notification_opt_in_view()->GetVisible());
......
...@@ -379,9 +379,12 @@ void MultideviceHandler::HandleAttemptNotificationSetup( ...@@ -379,9 +379,12 @@ void MultideviceHandler::HandleAttemptNotificationSetup(
DCHECK(features::IsPhoneHubEnabled()); DCHECK(features::IsPhoneHubEnabled());
DCHECK(!notification_access_operation_); DCHECK(!notification_access_operation_);
if (notification_access_manager_->HasAccessBeenGranted()) { phonehub::NotificationAccessManager::AccessStatus access_status =
PA_LOG(WARNING) << "Phonehub notification access has already been granted, " notification_access_manager_->GetAccessStatus();
"returning early."; if (access_status != phonehub::NotificationAccessManager::AccessStatus::
kAvailableButNotGranted) {
PA_LOG(WARNING) << "Cannot request notification access setup flow; current "
<< "status: " << access_status;
return; return;
} }
...@@ -473,16 +476,12 @@ MultideviceHandler::GeneratePageContentDataDictionary() { ...@@ -473,16 +476,12 @@ MultideviceHandler::GeneratePageContentDataDictionary() {
? android_sms_pairing_state_tracker_->IsAndroidSmsPairingComplete() ? android_sms_pairing_state_tracker_->IsAndroidSmsPairingComplete()
: false); : false);
// TODO(khorimoto): Send prohibited value if notification access is phonehub::NotificationAccessManager::AccessStatus access_status = phonehub::
// prohibited. NotificationAccessManager::AccessStatus::kAvailableButNotGranted;
static const int kAccessNotGranted = 1; if (notification_access_manager_)
static const int kAccessGranted = 2; access_status = notification_access_manager_->GetAccessStatus();
int access_value = kAccessNotGranted; page_content_dictionary->SetInteger(kNotificationAccessStatus,
if (notification_access_manager_ && static_cast<int32_t>(access_status));
notification_access_manager_->HasAccessBeenGranted()) {
access_value = kAccessGranted;
}
page_content_dictionary->SetInteger(kNotificationAccessStatus, access_value);
return page_content_dictionary; return page_content_dictionary;
} }
......
...@@ -158,7 +158,8 @@ class MultideviceHandlerTest : public testing::Test { ...@@ -158,7 +158,8 @@ class MultideviceHandlerTest : public testing::Test {
std::make_unique<multidevice_setup::FakeMultiDeviceSetupClient>(); std::make_unique<multidevice_setup::FakeMultiDeviceSetupClient>();
fake_notification_access_manager_ = fake_notification_access_manager_ =
std::make_unique<phonehub::FakeNotificationAccessManager>( std::make_unique<phonehub::FakeNotificationAccessManager>(
/*has_access_been_granted=*/false); phonehub::NotificationAccessManager::AccessStatus::
kAvailableButNotGranted);
fake_android_sms_pairing_state_tracker_ = std::make_unique< fake_android_sms_pairing_state_tracker_ = std::make_unique<
multidevice_setup::FakeAndroidSmsPairingStateTracker>(); multidevice_setup::FakeAndroidSmsPairingStateTracker>();
fake_android_sms_app_manager_ = fake_android_sms_app_manager_ =
...@@ -227,8 +228,11 @@ class MultideviceHandlerTest : public testing::Test { ...@@ -227,8 +228,11 @@ class MultideviceHandlerTest : public testing::Test {
} }
void CallAttemptNotificationSetup(bool has_access_been_granted) { void CallAttemptNotificationSetup(bool has_access_been_granted) {
fake_notification_access_manager()->SetHasAccessBeenGrantedInternal( fake_notification_access_manager()->SetAccessStatusInternal(
has_access_been_granted); has_access_been_granted
? phonehub::NotificationAccessManager::AccessStatus::kAccessGranted
: phonehub::NotificationAccessManager::AccessStatus::
kAvailableButNotGranted);
base::ListValue empty_args; base::ListValue empty_args;
test_web_ui()->HandleReceivedMessage("attemptNotificationSetup", test_web_ui()->HandleReceivedMessage("attemptNotificationSetup",
&empty_args); &empty_args);
......
...@@ -8,22 +8,23 @@ namespace chromeos { ...@@ -8,22 +8,23 @@ namespace chromeos {
namespace phonehub { namespace phonehub {
FakeNotificationAccessManager::FakeNotificationAccessManager( FakeNotificationAccessManager::FakeNotificationAccessManager(
bool has_access_been_granted) AccessStatus access_status)
: has_access_been_granted_(has_access_been_granted) {} : access_status_(access_status) {}
FakeNotificationAccessManager::~FakeNotificationAccessManager() = default; FakeNotificationAccessManager::~FakeNotificationAccessManager() = default;
void FakeNotificationAccessManager::SetHasAccessBeenGrantedInternal( void FakeNotificationAccessManager::SetAccessStatusInternal(
bool has_access_been_granted) { AccessStatus access_status) {
if (has_access_been_granted_ == has_access_been_granted) if (access_status_ == access_status)
return; return;
has_access_been_granted_ = has_access_been_granted; access_status_ = access_status;
NotifyNotificationAccessChanged(); NotifyNotificationAccessChanged();
} }
bool FakeNotificationAccessManager::HasAccessBeenGranted() const { NotificationAccessManager::AccessStatus
return has_access_been_granted_; FakeNotificationAccessManager::GetAccessStatus() const {
return access_status_;
} }
bool FakeNotificationAccessManager::HasNotificationSetupUiBeenDismissed() bool FakeNotificationAccessManager::HasNotificationSetupUiBeenDismissed()
...@@ -41,9 +42,17 @@ void FakeNotificationAccessManager::ResetHasNotificationSetupUiBeenDismissed() { ...@@ -41,9 +42,17 @@ void FakeNotificationAccessManager::ResetHasNotificationSetupUiBeenDismissed() {
void FakeNotificationAccessManager::SetNotificationSetupOperationStatus( void FakeNotificationAccessManager::SetNotificationSetupOperationStatus(
NotificationAccessSetupOperation::Status new_status) { NotificationAccessSetupOperation::Status new_status) {
if (new_status == switch (new_status) {
NotificationAccessSetupOperation::Status::kCompletedSuccessfully) { case NotificationAccessSetupOperation::Status::kCompletedSuccessfully:
SetHasAccessBeenGrantedInternal(true); SetAccessStatusInternal(AccessStatus::kAccessGranted);
break;
case NotificationAccessSetupOperation::Status::
kProhibitedFromProvidingAccess:
SetAccessStatusInternal(AccessStatus::kProhibited);
break;
default:
// Do not update access status based on other operation status values.
break;
} }
NotificationAccessManager::SetNotificationSetupOperationStatus(new_status); NotificationAccessManager::SetNotificationSetupOperationStatus(new_status);
......
...@@ -12,24 +12,25 @@ namespace phonehub { ...@@ -12,24 +12,25 @@ namespace phonehub {
class FakeNotificationAccessManager : public NotificationAccessManager { class FakeNotificationAccessManager : public NotificationAccessManager {
public: public:
explicit FakeNotificationAccessManager(bool has_access_been_granted = false); explicit FakeNotificationAccessManager(
AccessStatus access_status = AccessStatus::kAvailableButNotGranted);
~FakeNotificationAccessManager() override; ~FakeNotificationAccessManager() override;
using NotificationAccessManager::IsSetupOperationInProgress; using NotificationAccessManager::IsSetupOperationInProgress;
void SetHasAccessBeenGrantedInternal(bool has_access_been_granted) override; void SetAccessStatusInternal(AccessStatus access_status) override;
void SetNotificationSetupOperationStatus( void SetNotificationSetupOperationStatus(
NotificationAccessSetupOperation::Status new_status); NotificationAccessSetupOperation::Status new_status);
// NotificationAccessManager: // NotificationAccessManager:
bool HasAccessBeenGranted() const override; AccessStatus GetAccessStatus() const override;
bool HasNotificationSetupUiBeenDismissed() const override; bool HasNotificationSetupUiBeenDismissed() const override;
void DismissSetupRequiredUi() override; void DismissSetupRequiredUi() override;
void ResetHasNotificationSetupUiBeenDismissed(); void ResetHasNotificationSetupUiBeenDismissed();
private: private:
bool has_access_been_granted_; AccessStatus access_status_;
bool has_notification_setup_ui_been_dismissed_ = false; bool has_notification_setup_ui_been_dismissed_ = false;
}; };
......
...@@ -43,8 +43,10 @@ MultideviceSetupStateUpdater::~MultideviceSetupStateUpdater() { ...@@ -43,8 +43,10 @@ MultideviceSetupStateUpdater::~MultideviceSetupStateUpdater() {
} }
void MultideviceSetupStateUpdater::OnNotificationAccessChanged() { void MultideviceSetupStateUpdater::OnNotificationAccessChanged() {
if (notification_access_manager_->HasAccessBeenGranted()) if (notification_access_manager_->GetAccessStatus() ==
NotificationAccessManager::AccessStatus::kAccessGranted) {
return; return;
}
PA_LOG(INFO) << "Disabling PhoneHubNotifications feature."; PA_LOG(INFO) << "Disabling PhoneHubNotifications feature.";
......
...@@ -36,7 +36,10 @@ class MultideviceSetupStateUpdaterTest : public testing::Test { ...@@ -36,7 +36,10 @@ class MultideviceSetupStateUpdaterTest : public testing::Test {
} }
void SetNotififcationAccess(bool enabled) { void SetNotififcationAccess(bool enabled) {
fake_notification_access_manager_.SetHasAccessBeenGrantedInternal(enabled); fake_notification_access_manager_.SetAccessStatusInternal(
enabled
? NotificationAccessManager::AccessStatus::kAccessGranted
: NotificationAccessManager::AccessStatus::kAvailableButNotGranted);
} }
void SetFeatureState(Feature feature, FeatureState feature_state) { void SetFeatureState(Feature feature, FeatureState feature_state) {
......
...@@ -18,7 +18,9 @@ NotificationAccessManager::~NotificationAccessManager() = default; ...@@ -18,7 +18,9 @@ NotificationAccessManager::~NotificationAccessManager() = default;
std::unique_ptr<NotificationAccessSetupOperation> std::unique_ptr<NotificationAccessSetupOperation>
NotificationAccessManager::AttemptNotificationSetup( NotificationAccessManager::AttemptNotificationSetup(
NotificationAccessSetupOperation::Delegate* delegate) { NotificationAccessSetupOperation::Delegate* delegate) {
if (HasAccessBeenGranted()) // Should only be able to start the setup process if notification access is
// available but not yet granted.
if (GetAccessStatus() != AccessStatus::kAvailableButNotGranted)
return nullptr; return nullptr;
int operation_id = next_operation_id_; int operation_id = next_operation_id_;
...@@ -75,5 +77,21 @@ void NotificationAccessManager::OnSetupOperationDeleted(int operation_id) { ...@@ -75,5 +77,21 @@ void NotificationAccessManager::OnSetupOperationDeleted(int operation_id) {
PA_LOG(INFO) << "Notification access setup operation has ended."; PA_LOG(INFO) << "Notification access setup operation has ended.";
} }
std::ostream& operator<<(std::ostream& stream,
NotificationAccessManager::AccessStatus status) {
switch (status) {
case NotificationAccessManager::AccessStatus::kProhibited:
stream << "[Access prohibited]";
break;
case NotificationAccessManager::AccessStatus::kAvailableButNotGranted:
stream << "[Access available but not granted]";
break;
case NotificationAccessManager::AccessStatus::kAccessGranted:
stream << "[Access granted]";
break;
}
return stream;
}
} // namespace phonehub } // namespace phonehub
} // namespace chromeos } // namespace chromeos
...@@ -5,6 +5,8 @@ ...@@ -5,6 +5,8 @@
#ifndef CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_ACCESS_MANAGER_H_ #ifndef CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_ACCESS_MANAGER_H_
#define CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_ACCESS_MANAGER_H_ #define CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_ACCESS_MANAGER_H_
#include <ostream>
#include "base/containers/flat_map.h" #include "base/containers/flat_map.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/observer_list.h" #include "base/observer_list.h"
...@@ -25,11 +27,25 @@ namespace phonehub { ...@@ -25,11 +27,25 @@ namespace phonehub {
// access setup flow via AttemptNotificationSetup(). // access setup flow via AttemptNotificationSetup().
class NotificationAccessManager { class NotificationAccessManager {
public: public:
// Status of notification access. Numerical values are stored in prefs and
// should not be changed or reused.
enum class AccessStatus {
// Access has not been granted and is prohibited from being granted (e.g.,
// if the phone is using a Work Profile).
kProhibited = 0,
// Access has not been granted, but the user is free to grant access.
kAvailableButNotGranted = 1,
// Access has been granted by the user.
kAccessGranted = 2
};
class Observer : public base::CheckedObserver { class Observer : public base::CheckedObserver {
public: public:
~Observer() override = default; ~Observer() override = default;
// Called when notification access has changed; use HasAccessBeenGranted() // Called when notification access has changed; use GetAccessStatus()
// for the new status. // for the new status.
virtual void OnNotificationAccessChanged() = 0; virtual void OnNotificationAccessChanged() = 0;
}; };
...@@ -39,7 +55,7 @@ class NotificationAccessManager { ...@@ -39,7 +55,7 @@ class NotificationAccessManager {
delete; delete;
virtual ~NotificationAccessManager(); virtual ~NotificationAccessManager();
virtual bool HasAccessBeenGranted() const = 0; virtual AccessStatus GetAccessStatus() const = 0;
virtual bool HasNotificationSetupUiBeenDismissed() const = 0; virtual bool HasNotificationSetupUiBeenDismissed() const = 0;
...@@ -77,11 +93,9 @@ class NotificationAccessManager { ...@@ -77,11 +93,9 @@ class NotificationAccessManager {
friend class NotificationAccessManagerImplTest; friend class NotificationAccessManagerImplTest;
friend class PhoneStatusProcessor; friend class PhoneStatusProcessor;
// This only sets the internal state of the whether notification access has // Sets the internal AccessStatus but does not send a request for a new
// mode been enabled and does not send a request to set the state of the // status to the remote phone device.
// remote phone device. virtual void SetAccessStatusInternal(AccessStatus access_status) = 0;
virtual void SetHasAccessBeenGrantedInternal(
bool has_access_been_granted) = 0;
void OnSetupOperationDeleted(int operation_id); void OnSetupOperationDeleted(int operation_id);
...@@ -91,6 +105,9 @@ class NotificationAccessManager { ...@@ -91,6 +105,9 @@ class NotificationAccessManager {
base::WeakPtrFactory<NotificationAccessManager> weak_ptr_factory_{this}; base::WeakPtrFactory<NotificationAccessManager> weak_ptr_factory_{this};
}; };
std::ostream& operator<<(std::ostream& stream,
NotificationAccessManager::AccessStatus status);
} // namespace phonehub } // namespace phonehub
} // namespace chromeos } // namespace chromeos
......
...@@ -17,7 +17,9 @@ namespace phonehub { ...@@ -17,7 +17,9 @@ namespace phonehub {
// static // static
void NotificationAccessManagerImpl::RegisterPrefs( void NotificationAccessManagerImpl::RegisterPrefs(
PrefRegistrySimple* registry) { PrefRegistrySimple* registry) {
registry->RegisterBooleanPref(prefs::kNotificationAccessGranted, false); registry->RegisterIntegerPref(
prefs::kNotificationAccessStatus,
static_cast<int>(AccessStatus::kAvailableButNotGranted));
registry->RegisterBooleanPref(prefs::kHasDismissedSetupRequiredUi, false); registry->RegisterBooleanPref(prefs::kHasDismissedSetupRequiredUi, false);
} }
...@@ -50,25 +52,40 @@ void NotificationAccessManagerImpl::DismissSetupRequiredUi() { ...@@ -50,25 +52,40 @@ void NotificationAccessManagerImpl::DismissSetupRequiredUi() {
pref_service_->SetBoolean(prefs::kHasDismissedSetupRequiredUi, true); pref_service_->SetBoolean(prefs::kHasDismissedSetupRequiredUi, true);
} }
bool NotificationAccessManagerImpl::HasAccessBeenGranted() const { NotificationAccessManagerImpl::AccessStatus
return pref_service_->GetBoolean(prefs::kNotificationAccessGranted); NotificationAccessManagerImpl::GetAccessStatus() const {
int status = pref_service_->GetInteger(prefs::kNotificationAccessStatus);
return static_cast<AccessStatus>(status);
} }
void NotificationAccessManagerImpl::SetHasAccessBeenGrantedInternal( void NotificationAccessManagerImpl::SetAccessStatusInternal(
bool has_access_been_granted) { AccessStatus access_status) {
if (has_access_been_granted == HasAccessBeenGranted()) if (access_status == GetAccessStatus())
return; return;
PA_LOG(INFO) << "Notification access state has been set to: " PA_LOG(INFO) << "Notification access: " << GetAccessStatus() << " => "
<< has_access_been_granted; << access_status;
pref_service_->SetBoolean(prefs::kNotificationAccessGranted, pref_service_->SetInteger(prefs::kNotificationAccessStatus,
has_access_been_granted); static_cast<int>(access_status));
NotifyNotificationAccessChanged(); NotifyNotificationAccessChanged();
if (IsSetupOperationInProgress() && has_access_been_granted) { if (!IsSetupOperationInProgress())
SetNotificationSetupOperationStatus( return;
NotificationAccessSetupOperation::Status::kCompletedSuccessfully);
switch (access_status) {
case AccessStatus::kProhibited:
SetNotificationSetupOperationStatus(
NotificationAccessSetupOperation::Status::
kProhibitedFromProvidingAccess);
break;
case AccessStatus::kAccessGranted:
SetNotificationSetupOperationStatus(
NotificationAccessSetupOperation::Status::kCompletedSuccessfully);
break;
case AccessStatus::kAvailableButNotGranted:
// Intentionally blank; the operation status should not change.
break;
} }
} }
......
...@@ -20,9 +20,6 @@ class ConnectionScheduler; ...@@ -20,9 +20,6 @@ class ConnectionScheduler;
// Implements NotificationAccessManager by persisting the last-known // Implements NotificationAccessManager by persisting the last-known
// notification access value to user prefs. // notification access value to user prefs.
// TODO(khorimoto): Currently HasAccessBeenGranted() always returns false. Have
// it return true once the phone has sent a message indicating that it has
// granted access.
class NotificationAccessManagerImpl : public NotificationAccessManager, class NotificationAccessManagerImpl : public NotificationAccessManager,
public FeatureStatusProvider::Observer { public FeatureStatusProvider::Observer {
public: public:
...@@ -39,8 +36,8 @@ class NotificationAccessManagerImpl : public NotificationAccessManager, ...@@ -39,8 +36,8 @@ class NotificationAccessManagerImpl : public NotificationAccessManager,
friend class NotificationAccessManagerImplTest; friend class NotificationAccessManagerImplTest;
// NotificationAccessManager: // NotificationAccessManager:
bool HasAccessBeenGranted() const override; AccessStatus GetAccessStatus() const override;
void SetHasAccessBeenGrantedInternal(bool has_access_been_granted) override; void SetAccessStatusInternal(AccessStatus access_status) override;
void OnSetupRequested() override; void OnSetupRequested() override;
bool HasNotificationSetupUiBeenDismissed() const override; bool HasNotificationSetupUiBeenDismissed() const override;
......
...@@ -16,11 +16,13 @@ namespace { ...@@ -16,11 +16,13 @@ namespace {
// Status values which are considered "final" - i.e., once the status of an // Status values which are considered "final" - i.e., once the status of an
// operation changes to one of these values, the operation has completed. These // operation changes to one of these values, the operation has completed. These
// status values indicate either a success or a fatal error. // status values indicate either a success or a fatal error.
constexpr std::array<NotificationAccessSetupOperation::Status, 3> constexpr std::array<NotificationAccessSetupOperation::Status, 4>
kOperationFinishedStatus{ kOperationFinishedStatus{
NotificationAccessSetupOperation::Status::kTimedOutConnecting, NotificationAccessSetupOperation::Status::kTimedOutConnecting,
NotificationAccessSetupOperation::Status::kConnectionDisconnected, NotificationAccessSetupOperation::Status::kConnectionDisconnected,
NotificationAccessSetupOperation::Status::kCompletedSuccessfully, NotificationAccessSetupOperation::Status::kCompletedSuccessfully,
NotificationAccessSetupOperation::Status::
kProhibitedFromProvidingAccess,
}; };
} // namespace } // namespace
...@@ -66,6 +68,10 @@ std::ostream& operator<<(std::ostream& stream, ...@@ -66,6 +68,10 @@ std::ostream& operator<<(std::ostream& stream,
case NotificationAccessSetupOperation::Status::kCompletedSuccessfully: case NotificationAccessSetupOperation::Status::kCompletedSuccessfully:
stream << "[Completed successfully]"; stream << "[Completed successfully]";
break; break;
case NotificationAccessSetupOperation::Status::
kProhibitedFromProvidingAccess:
stream << "[Prohibited from providing access]";
break;
} }
return stream; return stream;
......
...@@ -48,6 +48,10 @@ class NotificationAccessSetupOperation { ...@@ -48,6 +48,10 @@ class NotificationAccessSetupOperation {
// The user has completed the phone-side opt-in flow. // The user has completed the phone-side opt-in flow.
kCompletedSuccessfully = 5, kCompletedSuccessfully = 5,
// The user's phone is prohibited from granting notification access (e.g.,
// the user could be using a Work Profile).
kProhibitedFromProvidingAccess = 6
}; };
// Returns true if the provided status is the final one for this operation, // Returns true if the provided status is the final one for this operation,
......
...@@ -108,6 +108,21 @@ PhoneStatusModel::BatterySaverState GetBatterySaverStateFromProto( ...@@ -108,6 +108,21 @@ PhoneStatusModel::BatterySaverState GetBatterySaverStateFromProto(
} }
} }
NotificationAccessManager::AccessStatus ComputeNotificationAccessState(
const proto::PhoneProperties& phone_properties) {
// If the user has a Work Profile active, notification access is not allowed
// by Android. See https://crbug.com/1155151.
if (phone_properties.profile_type() == proto::ProfileType::WORK_PROFILE)
return NotificationAccessManager::AccessStatus::kProhibited;
if (phone_properties.notification_access_state() ==
proto::NotificationAccessState::ACCESS_GRANTED) {
return NotificationAccessManager::AccessStatus::kAccessGranted;
}
return NotificationAccessManager::AccessStatus::kAvailableButNotGranted;
}
base::Optional<Notification> ProcessNotificationProto( base::Optional<Notification> ProcessNotificationProto(
const proto::Notification& proto) { const proto::Notification& proto) {
// Only process notifications that are messaging apps with inline-replies. // Only process notifications that are messaging apps with inline-replies.
...@@ -219,9 +234,8 @@ void PhoneStatusProcessor::SetReceivedPhoneStatusModelStates( ...@@ -219,9 +234,8 @@ void PhoneStatusProcessor::SetReceivedPhoneStatusModelStates(
proto::NotificationMode::DO_NOT_DISTURB_ON, proto::NotificationMode::DO_NOT_DISTURB_ON,
phone_properties.profile_type() != proto::ProfileType::WORK_PROFILE); phone_properties.profile_type() != proto::ProfileType::WORK_PROFILE);
notification_access_manager_->SetHasAccessBeenGrantedInternal( notification_access_manager_->SetAccessStatusInternal(
phone_properties.notification_access_state() == ComputeNotificationAccessState(phone_properties));
proto::NotificationAccessState::ACCESS_GRANTED);
find_my_device_controller_->SetIsPhoneRingingInternal( find_my_device_controller_->SetIsPhoneRingingInternal(
phone_properties.ring_status() == proto::FindMyDeviceRingStatus::RINGING); phone_properties.ring_status() == proto::FindMyDeviceRingStatus::RINGING);
......
...@@ -111,7 +111,7 @@ TEST_F(PhoneStatusProcessorTest, PhoneStatusSnapshotUpdate) { ...@@ -111,7 +111,7 @@ TEST_F(PhoneStatusProcessorTest, PhoneStatusSnapshotUpdate) {
expected_phone_properties->set_profile_type( expected_phone_properties->set_profile_type(
proto::ProfileType::DEFAULT_PROFILE); proto::ProfileType::DEFAULT_PROFILE);
expected_phone_properties->set_notification_access_state( expected_phone_properties->set_notification_access_state(
proto::NotificationAccessState::ACCESS_GRANTED); proto::NotificationAccessState::ACCESS_NOT_GRANTED);
expected_phone_properties->set_ring_status( expected_phone_properties->set_ring_status(
proto::FindMyDeviceRingStatus::RINGING); proto::FindMyDeviceRingStatus::RINGING);
expected_phone_properties->set_battery_percentage(24u); expected_phone_properties->set_battery_percentage(24u);
...@@ -142,7 +142,8 @@ TEST_F(PhoneStatusProcessorTest, PhoneStatusSnapshotUpdate) { ...@@ -142,7 +142,8 @@ TEST_F(PhoneStatusProcessorTest, PhoneStatusSnapshotUpdate) {
EXPECT_TRUE(fake_do_not_disturb_controller_->CanRequestNewDndState()); EXPECT_TRUE(fake_do_not_disturb_controller_->CanRequestNewDndState());
EXPECT_EQ(FindMyDeviceController::Status::kRingingOn, EXPECT_EQ(FindMyDeviceController::Status::kRingingOn,
fake_find_my_device_controller_->GetPhoneRingingStatus()); fake_find_my_device_controller_->GetPhoneRingingStatus());
EXPECT_TRUE(fake_notification_access_manager_->HasAccessBeenGranted()); EXPECT_EQ(NotificationAccessManager::AccessStatus::kAvailableButNotGranted,
fake_notification_access_manager_->GetAccessStatus());
base::Optional<PhoneStatusModel> phone_status_model = base::Optional<PhoneStatusModel> phone_status_model =
mutable_phone_model_->phone_status_model(); mutable_phone_model_->phone_status_model();
...@@ -206,7 +207,8 @@ TEST_F(PhoneStatusProcessorTest, PhoneStatusUpdate) { ...@@ -206,7 +207,8 @@ TEST_F(PhoneStatusProcessorTest, PhoneStatusUpdate) {
EXPECT_FALSE(fake_do_not_disturb_controller_->CanRequestNewDndState()); EXPECT_FALSE(fake_do_not_disturb_controller_->CanRequestNewDndState());
EXPECT_EQ(FindMyDeviceController::Status::kRingingOn, EXPECT_EQ(FindMyDeviceController::Status::kRingingOn,
fake_find_my_device_controller_->GetPhoneRingingStatus()); fake_find_my_device_controller_->GetPhoneRingingStatus());
EXPECT_TRUE(fake_notification_access_manager_->HasAccessBeenGranted()); EXPECT_EQ(NotificationAccessManager::AccessStatus::kProhibited,
fake_notification_access_manager_->GetAccessStatus());
base::Optional<PhoneStatusModel> phone_status_model = base::Optional<PhoneStatusModel> phone_status_model =
mutable_phone_model_->phone_status_model(); mutable_phone_model_->phone_status_model();
...@@ -220,18 +222,21 @@ TEST_F(PhoneStatusProcessorTest, PhoneStatusUpdate) { ...@@ -220,18 +222,21 @@ TEST_F(PhoneStatusProcessorTest, PhoneStatusUpdate) {
EXPECT_EQ(PhoneStatusModel::MobileStatus::kSimWithReception, EXPECT_EQ(PhoneStatusModel::MobileStatus::kSimWithReception,
phone_status_model->mobile_status()); phone_status_model->mobile_status());
// Update with one removed notification. // Update with one removed notification and a default profile.
expected_update.add_removed_notification_ids(0u); expected_update.add_removed_notification_ids(0u);
expected_update.mutable_properties()->set_profile_type(
proto::ProfileType::DEFAULT_PROFILE);
fake_message_receiver_->NotifyPhoneStatusUpdateReceived(expected_update); fake_message_receiver_->NotifyPhoneStatusUpdateReceived(expected_update);
EXPECT_EQ(0u, fake_notification_manager_->num_notifications()); EXPECT_EQ(0u, fake_notification_manager_->num_notifications());
EXPECT_EQ(base::UTF8ToUTF16(test_remote_device_.name()), EXPECT_EQ(base::UTF8ToUTF16(test_remote_device_.name()),
*mutable_phone_model_->phone_name()); *mutable_phone_model_->phone_name());
EXPECT_TRUE(fake_do_not_disturb_controller_->IsDndEnabled()); EXPECT_TRUE(fake_do_not_disturb_controller_->IsDndEnabled());
EXPECT_FALSE(fake_do_not_disturb_controller_->CanRequestNewDndState()); EXPECT_TRUE(fake_do_not_disturb_controller_->CanRequestNewDndState());
EXPECT_EQ(FindMyDeviceController::Status::kRingingOn, EXPECT_EQ(FindMyDeviceController::Status::kRingingOn,
fake_find_my_device_controller_->GetPhoneRingingStatus()); fake_find_my_device_controller_->GetPhoneRingingStatus());
EXPECT_TRUE(fake_notification_access_manager_->HasAccessBeenGranted()); EXPECT_EQ(NotificationAccessManager::AccessStatus::kAccessGranted,
fake_notification_access_manager_->GetAccessStatus());
phone_status_model = mutable_phone_model_->phone_status_model(); phone_status_model = mutable_phone_model_->phone_status_model();
EXPECT_EQ(PhoneStatusModel::ChargingState::kChargingAc, EXPECT_EQ(PhoneStatusModel::ChargingState::kChargingAc,
......
...@@ -8,9 +8,11 @@ namespace chromeos { ...@@ -8,9 +8,11 @@ namespace chromeos {
namespace phonehub { namespace phonehub {
namespace prefs { namespace prefs {
// Whether notification access had been granted by the user on their phone. // The last provided notification access status provided by the phone. This pref
const char kNotificationAccessGranted[] = // stores the numerical value associated with the
"cros.phonehub.notification_access_granted"; // NotificationAccessManager::AccessStatus enum.
const char kNotificationAccessStatus[] =
"cros.phonehub.notification_access_status";
// Whether user has completed onboarding and dismissed the UI before or if // Whether user has completed onboarding and dismissed the UI before or if
// the user has already gone through the onboarding process and has enabled the // the user has already gone through the onboarding process and has enabled the
......
...@@ -9,7 +9,7 @@ namespace chromeos { ...@@ -9,7 +9,7 @@ namespace chromeos {
namespace phonehub { namespace phonehub {
namespace prefs { namespace prefs {
extern const char kNotificationAccessGranted[]; extern const char kNotificationAccessStatus[];
extern const char kHideOnboardingUi[]; extern const char kHideOnboardingUi[];
extern const char kIsAwaitingVerifiedHost[]; extern const char kIsAwaitingVerifiedHost[];
extern const char kHasDismissedSetupRequiredUi[]; extern const char kHasDismissedSetupRequiredUi[];
......
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