Commit 51e15805 authored by Jimmy Gong's avatar Jimmy Gong Committed by Chromium LUCI CQ

Phone Hub: Add CanRequestNewDndState to DoNotDisturbController

CanRequestNewDndState indicates whether or not the DoNotDisturb feature
should be enabled for the Phone Hub UI.

Bug: 1155244
Test: chromeos_components_unittest
Change-Id: Ief42e2f3cefdcb2aa4a74b5e41a5f7329ae330c3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2572958
Commit-Queue: Jimmy Gong <jimmyxgong@chromium.org>
Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833804}
parent a0eb5aca
......@@ -296,7 +296,8 @@ void MultidevicePhoneHubHandler::HandleEnableDnd(const base::ListValue* args) {
CHECK(args->GetBoolean(0, &enabled));
PA_LOG(VERBOSE) << "Setting Do Not Disturb state to " << enabled;
fake_phone_hub_manager_->fake_do_not_disturb_controller()
->SetDoNotDisturbStateInternal(enabled);
->SetDoNotDisturbStateInternal(enabled,
/*can_request_new_dnd_state=*/true);
}
void MultidevicePhoneHubHandler::HandleSetFindMyDeviceStatus(
......
......@@ -29,6 +29,10 @@ class DoNotDisturbController {
virtual bool IsDndEnabled() const = 0;
// Returns whether a new DoNotDisturb state can be set. If this returns false,
// then we are disabling the DoNotDisturb mode feature on the CrOS device.
virtual bool CanRequestNewDndState() const = 0;
// Note: Setting DND state is not a synchronous operation, since it requires
// sending a message to the connected phone. Use the observer interface to be
// notified of when the state changes.
......@@ -42,9 +46,11 @@ class DoNotDisturbController {
DoNotDisturbController();
// This only sets the internal state of the DoNotDisturb mode and does not
// send a request to set the state of the remote phone device.
virtual void SetDoNotDisturbStateInternal(bool is_dnd_enabled) = 0;
// This sets the internal state of the DoNotDisturb mode and and whether the
// DoNotDisturb state can be changed. This does not send a request to set the
// state of the remote phone device.
virtual void SetDoNotDisturbStateInternal(bool is_dnd_enabled,
bool can_request_new_dnd_state) = 0;
void NotifyDndStateChanged();
......
......@@ -23,13 +23,25 @@ bool DoNotDisturbControllerImpl::IsDndEnabled() const {
}
void DoNotDisturbControllerImpl::SetDoNotDisturbStateInternal(
bool is_dnd_enabled) {
if (is_dnd_enabled == is_dnd_enabled_)
bool is_dnd_enabled,
bool can_request_new_dnd_state) {
if (is_dnd_enabled_ == is_dnd_enabled &&
can_request_new_dnd_state_ == can_request_new_dnd_state) {
return;
}
PA_LOG(INFO) << "Do Not Disturb state updated: " << is_dnd_enabled_ << " => "
<< is_dnd_enabled;
is_dnd_enabled_ = is_dnd_enabled;
if (is_dnd_enabled != is_dnd_enabled_) {
PA_LOG(INFO) << "Do Not Disturb state updated: " << is_dnd_enabled_
<< " => " << is_dnd_enabled;
is_dnd_enabled_ = is_dnd_enabled;
}
if (can_request_new_dnd_state != can_request_new_dnd_state_) {
PA_LOG(INFO) << "Can request new Do Not Disturb state updated: "
<< can_request_new_dnd_state_ << " => "
<< can_request_new_dnd_state;
can_request_new_dnd_state_ = can_request_new_dnd_state;
}
NotifyDndStateChanged();
}
......@@ -42,5 +54,9 @@ void DoNotDisturbControllerImpl::RequestNewDoNotDisturbState(bool enabled) {
message_sender_->SendUpdateNotificationModeRequest(enabled);
}
bool DoNotDisturbControllerImpl::CanRequestNewDndState() const {
return can_request_new_dnd_state_;
}
} // namespace phonehub
} // namespace chromeos
......@@ -16,7 +16,7 @@ class MessageSender;
// feature of the user's remote phone.
class DoNotDisturbControllerImpl : public DoNotDisturbController {
public:
DoNotDisturbControllerImpl(MessageSender* message_sender);
explicit DoNotDisturbControllerImpl(MessageSender* message_sender);
~DoNotDisturbControllerImpl() override;
private:
......@@ -24,10 +24,13 @@ class DoNotDisturbControllerImpl : public DoNotDisturbController {
// DoNotDisturbController:
bool IsDndEnabled() const override;
void SetDoNotDisturbStateInternal(bool is_dnd_enabled) override;
void SetDoNotDisturbStateInternal(bool is_dnd_enabled,
bool can_request_new_dnd_state) override;
void RequestNewDoNotDisturbState(bool enabled) override;
bool CanRequestNewDndState() const override;
bool is_dnd_enabled_ = false;
bool can_request_new_dnd_state_ = false;
MessageSender* message_sender_;
};
......
......@@ -50,8 +50,14 @@ class DoNotDisturbControllerImplTest : public testing::Test {
bool IsDndEnabled() const { return controller_->IsDndEnabled(); }
void SetDoNotDisturbInternal(bool is_dnd_enabled) {
controller_->SetDoNotDisturbStateInternal(is_dnd_enabled);
bool CanRequestNewDndState() const {
return controller_->CanRequestNewDndState();
}
void SetDoNotDisturbInternal(bool is_dnd_enabled,
bool can_request_new_dnd_state) {
controller_->SetDoNotDisturbStateInternal(is_dnd_enabled,
can_request_new_dnd_state);
}
void RequestNewDoNotDisturbState(bool enabled) {
......@@ -77,19 +83,46 @@ class DoNotDisturbControllerImplTest : public testing::Test {
TEST_F(DoNotDisturbControllerImplTest, SetInternalStatesWithObservers) {
EXPECT_FALSE(IsDndEnabled());
SetDoNotDisturbInternal(/*is_dnd_enabled=*/true);
SetDoNotDisturbInternal(/*is_dnd_enabled=*/true,
/*can_request_new_dnd_state=*/true);
EXPECT_TRUE(IsDndEnabled());
EXPECT_TRUE(CanRequestNewDndState());
EXPECT_EQ(1u, GetNumObserverCalls());
SetDoNotDisturbInternal(/*is_dnd_enabled=*/false);
SetDoNotDisturbInternal(/*is_dnd_enabled=*/false,
/*can_request_new_dnd_state=*/true);
EXPECT_FALSE(IsDndEnabled());
EXPECT_TRUE(CanRequestNewDndState());
EXPECT_EQ(2u, GetNumObserverCalls());
// Setting internal state with the same previous state will not trigger an
// Setting internal dnd state with the same previous state will not trigger an
// observer event.
SetDoNotDisturbInternal(/*is_dnd_enabled=*/false);
SetDoNotDisturbInternal(/*is_dnd_enabled=*/false,
/*can_request_new_dnd_state=*/true);
EXPECT_FALSE(IsDndEnabled());
EXPECT_TRUE(CanRequestNewDndState());
EXPECT_EQ(2u, GetNumObserverCalls());
// Changing |can_request_new_dnd_state| should also trigger an observer event.
SetDoNotDisturbInternal(/*is_dnd_enabled=*/false,
/*can_request_new_dnd_state=*/false);
EXPECT_FALSE(IsDndEnabled());
EXPECT_FALSE(CanRequestNewDndState());
EXPECT_EQ(3u, GetNumObserverCalls());
// No new changes, expect no observer call.
SetDoNotDisturbInternal(/*is_dnd_enabled=*/false,
/*can_request_new_dnd_state=*/false);
EXPECT_FALSE(IsDndEnabled());
EXPECT_FALSE(CanRequestNewDndState());
EXPECT_EQ(3u, GetNumObserverCalls());
// Both states are changed, expect an observer call.
SetDoNotDisturbInternal(/*is_dnd_enabled=*/true,
/*can_request_new_dnd_state=*/true);
EXPECT_TRUE(IsDndEnabled());
EXPECT_TRUE(CanRequestNewDndState());
EXPECT_EQ(4u, GetNumObserverCalls());
}
TEST_F(DoNotDisturbControllerImplTest, RequestNewDoNotDisturbState) {
......@@ -97,13 +130,15 @@ TEST_F(DoNotDisturbControllerImplTest, RequestNewDoNotDisturbState) {
EXPECT_TRUE(GetRecentUpdateNotificationModeRequest());
EXPECT_EQ(1u, GetUpdateNotificationModeRequestCallCount());
// Simulate receiving a response and setting the internal value.
SetDoNotDisturbInternal(/*is_dnd_enabled=*/true);
SetDoNotDisturbInternal(/*is_dnd_enabled=*/true,
/*can_request_new_dnd_state=*/true);
RequestNewDoNotDisturbState(/*enabled=*/false);
EXPECT_FALSE(GetRecentUpdateNotificationModeRequest());
EXPECT_EQ(2u, GetUpdateNotificationModeRequestCallCount());
// Simulate receiving a response and setting the internal value.
SetDoNotDisturbInternal(/*is_dnd_enabled=*/false);
SetDoNotDisturbInternal(/*is_dnd_enabled=*/false,
/*can_request_new_dnd_state=*/true);
// Requesting for a the same state as the currently set state is a no-op.
RequestNewDoNotDisturbState(/*enabled=*/false);
......
......@@ -16,17 +16,26 @@ bool FakeDoNotDisturbController::IsDndEnabled() const {
}
void FakeDoNotDisturbController::SetDoNotDisturbStateInternal(
bool is_dnd_enabled) {
if (is_dnd_enabled_ == is_dnd_enabled)
bool is_dnd_enabled,
bool can_request_new_dnd_state) {
if (is_dnd_enabled_ == is_dnd_enabled &&
can_request_new_dnd_state_ == can_request_new_dnd_state) {
return;
}
is_dnd_enabled_ = is_dnd_enabled;
can_request_new_dnd_state_ = can_request_new_dnd_state;
NotifyDndStateChanged();
}
void FakeDoNotDisturbController::RequestNewDoNotDisturbState(bool enabled) {
if (!should_request_fail_)
SetDoNotDisturbStateInternal(enabled);
SetDoNotDisturbStateInternal(enabled, /*can_request_new_dnd_state=*/true);
}
bool FakeDoNotDisturbController::CanRequestNewDndState() const {
return can_request_new_dnd_state_;
}
void FakeDoNotDisturbController::SetShouldRequestFail(
......
......@@ -17,13 +17,16 @@ class FakeDoNotDisturbController : public DoNotDisturbController {
// DoNotDisturbController:
bool IsDndEnabled() const override;
void SetDoNotDisturbStateInternal(bool is_dnd_enabled) override;
void SetDoNotDisturbStateInternal(bool is_dnd_enabled,
bool can_request_new_dnd_state) override;
void RequestNewDoNotDisturbState(bool enabled) override;
bool CanRequestNewDndState() const override;
void SetShouldRequestFail(bool should_request_fail);
private:
bool is_dnd_enabled_ = false;
bool can_request_new_dnd_state_ = false;
// Indicates if the connection to the phone is working correctly. If it is
// true, there is a problem and the phone cannot change its state.
......
......@@ -82,7 +82,7 @@ TEST_F(FindMyDeviceControllerImplTest, RingingStateChanges) {
// Simulate flipping DoNotDisturb mode to enabled, this should set the
// FindMyPhone status to kRingingNotAvailable.
fake_do_not_disturb_controller_->SetDoNotDisturbStateInternal(
/*is_dnd_enabled=*/true);
/*is_dnd_enabled=*/true, /*can_request_new_dnd_state=*/true);
EXPECT_EQ(FindMyDeviceController::Status::kRingingNotAvailable,
GetPhoneRingingStatus());
// Simulate initiating phone ringing when DoNotDisturb mode is enabled. This
......@@ -95,7 +95,7 @@ TEST_F(FindMyDeviceControllerImplTest, RingingStateChanges) {
// Flip DoNotDisturb back to disabled, expect status to reset back to its
// previous state.
fake_do_not_disturb_controller_->SetDoNotDisturbStateInternal(
/*is_dnd_enabled=*/false);
/*is_dnd_enabled=*/false, /*can_request_new_dnd_state=*/true);
// Since we previously recorded that the phone should be ringing during
// DoNotDisturb mode was enabled, we return to that state once DoNotDisturb
// is disabled.
......@@ -117,7 +117,7 @@ TEST_F(FindMyDeviceControllerImplTest, RequestNewRingStatus) {
// Simulate flipping DoNotDisturb mode to enabled, this should set the
// FindMyPhone status to kRingingNotAvailable and not send any new messages.
fake_do_not_disturb_controller_->SetDoNotDisturbStateInternal(
/*is_dnd_enabled=*/true);
/*is_dnd_enabled=*/true, /*can_request_new_dnd_state=*/true);
EXPECT_EQ(FindMyDeviceController::Status::kRingingNotAvailable,
GetPhoneRingingStatus());
......@@ -130,7 +130,7 @@ TEST_F(FindMyDeviceControllerImplTest, RequestNewRingStatus) {
// Flip DoNotDisturb mode to disabled, expect that messages are able to be
// sent again.
fake_do_not_disturb_controller_->SetDoNotDisturbStateInternal(
/*is_dnd_enabled=*/false);
/*is_dnd_enabled=*/false, /*can_request_new_dnd_state=*/true);
EXPECT_EQ(FindMyDeviceController::Status::kRingingOff,
GetPhoneRingingStatus());
......
......@@ -216,7 +216,8 @@ void PhoneStatusProcessor::SetReceivedPhoneStatusModelStates(
do_not_disturb_controller_->SetDoNotDisturbStateInternal(
phone_properties.notification_mode() ==
proto::NotificationMode::DO_NOT_DISTURB_ON);
proto::NotificationMode::DO_NOT_DISTURB_ON,
phone_properties.profile_type() != proto::ProfileType::WORK_PROFILE);
notification_access_manager_->SetHasAccessBeenGrantedInternal(
phone_properties.notification_access_state() ==
......
......@@ -108,6 +108,8 @@ TEST_F(PhoneStatusProcessorTest, PhoneStatusSnapshotUpdate) {
auto expected_phone_properties = std::make_unique<proto::PhoneProperties>();
expected_phone_properties->set_notification_mode(
proto::NotificationMode::DO_NOT_DISTURB_ON);
expected_phone_properties->set_profile_type(
proto::ProfileType::DEFAULT_PROFILE);
expected_phone_properties->set_notification_access_state(
proto::NotificationAccessState::ACCESS_GRANTED);
expected_phone_properties->set_ring_status(
......@@ -137,6 +139,7 @@ TEST_F(PhoneStatusProcessorTest, PhoneStatusSnapshotUpdate) {
EXPECT_EQ(base::UTF8ToUTF16(test_remote_device_.name()),
*mutable_phone_model_->phone_name());
EXPECT_TRUE(fake_do_not_disturb_controller_->IsDndEnabled());
EXPECT_TRUE(fake_do_not_disturb_controller_->CanRequestNewDndState());
EXPECT_EQ(FindMyDeviceController::Status::kRingingOn,
fake_find_my_device_controller_->GetPhoneRingingStatus());
EXPECT_TRUE(fake_notification_access_manager_->HasAccessBeenGranted());
......@@ -171,6 +174,7 @@ TEST_F(PhoneStatusProcessorTest, PhoneStatusUpdate) {
auto expected_phone_properties = std::make_unique<proto::PhoneProperties>();
expected_phone_properties->set_notification_mode(
proto::NotificationMode::DO_NOT_DISTURB_ON);
expected_phone_properties->set_profile_type(proto::ProfileType::WORK_PROFILE);
expected_phone_properties->set_notification_access_state(
proto::NotificationAccessState::ACCESS_GRANTED);
expected_phone_properties->set_ring_status(
......@@ -199,6 +203,7 @@ TEST_F(PhoneStatusProcessorTest, PhoneStatusUpdate) {
EXPECT_EQ(base::UTF8ToUTF16(test_remote_device_.name()),
*mutable_phone_model_->phone_name());
EXPECT_TRUE(fake_do_not_disturb_controller_->IsDndEnabled());
EXPECT_FALSE(fake_do_not_disturb_controller_->CanRequestNewDndState());
EXPECT_EQ(FindMyDeviceController::Status::kRingingOn,
fake_find_my_device_controller_->GetPhoneRingingStatus());
EXPECT_TRUE(fake_notification_access_manager_->HasAccessBeenGranted());
......@@ -223,6 +228,7 @@ TEST_F(PhoneStatusProcessorTest, PhoneStatusUpdate) {
EXPECT_EQ(base::UTF8ToUTF16(test_remote_device_.name()),
*mutable_phone_model_->phone_name());
EXPECT_TRUE(fake_do_not_disturb_controller_->IsDndEnabled());
EXPECT_FALSE(fake_do_not_disturb_controller_->CanRequestNewDndState());
EXPECT_EQ(FindMyDeviceController::Status::kRingingOn,
fake_find_my_device_controller_->GetPhoneRingingStatus());
EXPECT_TRUE(fake_notification_access_manager_->HasAccessBeenGranted());
......
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