Commit 3792c739 authored by Jimmy Gong's avatar Jimmy Gong Committed by Commit Bot

Add additional states to FindMyDeviceController

Fully implements the FindMyDeviceControllerImpl class. This change
also features a new state: kRingingNotAvailable.
This state is only set if the DoNotDisturb is enabled.

Bug: 1106937
Test: chromeos_components_unittests
Change-Id: Ifeb4dd88ea19ad6949bad2a92d840f392d7ccd47
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2438773
Commit-Queue: Jimmy Gong <jimmyxgong@chromium.org>
Reviewed-by: default avatarKyle Horimoto <khorimoto@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812962}
parent 2b85bd3d
......@@ -247,12 +247,12 @@ void MultidevicePhoneHubHandler::OnDndStateChanged() {
}
void MultidevicePhoneHubHandler::OnPhoneRingingStateChanged() {
// TODO(jimmyxgong): Change to casting the enum TBA to int.
bool is_ringing = fake_phone_hub_manager_->fake_find_my_device_controller()
->IsPhoneRinging();
int status_as_int = is_ringing ? 2 : 1;
phonehub::FindMyDeviceController::Status ringing_status =
fake_phone_hub_manager_->fake_find_my_device_controller()
->GetPhoneRingingStatus();
FireWebUIListener("find-my-device-status-changed",
base::Value(status_as_int));
base::Value(static_cast<int>(ringing_status)));
}
void MultidevicePhoneHubHandler::OnTetherStatusChanged() {
......
......@@ -11,22 +11,33 @@ FakeFindMyDeviceController::FakeFindMyDeviceController() = default;
FakeFindMyDeviceController::~FakeFindMyDeviceController() = default;
bool FakeFindMyDeviceController::IsPhoneRinging() const {
return is_phone_ringing_;
void FakeFindMyDeviceController::SetPhoneRingingState(
Status phone_ringing_status) {
if (phone_ringing_status_ == phone_ringing_status)
return;
phone_ringing_status_ = phone_ringing_status;
NotifyPhoneRingingStateChanged();
}
void FakeFindMyDeviceController::SetIsPhoneRingingInternal(
bool is_phone_ringing) {
if (is_phone_ringing_ == is_phone_ringing)
Status phone_ringing_status =
is_phone_ringing ? Status::kRingingOn : Status::kRingingOff;
if (phone_ringing_status_ == Status::kRingingNotAvailable)
return;
is_phone_ringing_ = is_phone_ringing;
NotifyPhoneRingingStateChanged();
SetPhoneRingingState(phone_ringing_status);
}
void FakeFindMyDeviceController::RequestNewPhoneRingingState(bool ringing) {
SetIsPhoneRingingInternal(ringing);
}
FindMyDeviceController::Status
FakeFindMyDeviceController::GetPhoneRingingStatus() {
return phone_ringing_status_;
}
} // namespace phonehub
} // namespace chromeos
......@@ -15,13 +15,15 @@ class FakeFindMyDeviceController : public FindMyDeviceController {
FakeFindMyDeviceController();
~FakeFindMyDeviceController() override;
void SetPhoneRingingState(Status status);
// FindMyDeviceController:
bool IsPhoneRinging() const override;
void SetIsPhoneRingingInternal(bool is_phone_ringing) override;
void RequestNewPhoneRingingState(bool ringing) override;
Status GetPhoneRingingStatus() override;
private:
bool is_phone_ringing_ = false;
Status phone_ringing_status_ = Status::kRingingOff;
};
} // namespace phonehub
......
......@@ -22,20 +22,31 @@ class FindMyDeviceController {
virtual void OnPhoneRingingStateChanged() = 0;
};
enum class Status {
// The connected phone is not currently ringing.
kRingingOff = 0,
// The connected phone is currently ringing.
kRingingOn = 1,
// Ringing is not available if the phone's DoNotDisturb mode is enabled.
// To re-enable ringing, DoNotDisturb mode must be disabled.
kRingingNotAvailable = 2,
};
FindMyDeviceController(const FindMyDeviceController&) = delete;
FindMyDeviceController& operator=(const FindMyDeviceController&) = delete;
virtual ~FindMyDeviceController();
// Returns whether the phone is ringing as a result of Find My Device
// functionality. Note that this function does not return true if the phone is
// ringing for another reason (e.g., a normal phone call).
virtual bool IsPhoneRinging() const = 0;
// Note: Ringing the phone via Find My Device 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.
virtual void RequestNewPhoneRingingState(bool ringing) = 0;
// Returns the current ringing state of the connected phone. There are three
// possible states (on, off, disabled). The status is a result of Find My
// Device Functionality. Note that this function does not return true if the
// phone is ringing for another reason (e.g., a normal phone call)
virtual Status GetPhoneRingingStatus() = 0;
void AddObserver(Observer* observer);
void RemoveObserver(Observer* observer);
......
......@@ -5,26 +5,70 @@
#include "chromeos/components/phonehub/find_my_device_controller_impl.h"
#include "chromeos/components/multidevice/logging/logging.h"
#include "chromeos/components/phonehub/message_sender.h"
namespace chromeos {
namespace phonehub {
FindMyDeviceControllerImpl::FindMyDeviceControllerImpl() = default;
FindMyDeviceControllerImpl::FindMyDeviceControllerImpl(
DoNotDisturbController* do_not_disturb_controller,
MessageSender* message_sender)
: do_not_disturb_controller_(do_not_disturb_controller),
message_sender_(message_sender) {
DCHECK(do_not_disturb_controller_);
DCHECK(message_sender_);
FindMyDeviceControllerImpl::~FindMyDeviceControllerImpl() = default;
do_not_disturb_controller_->AddObserver(this);
}
bool FindMyDeviceControllerImpl::IsPhoneRinging() const {
return is_phone_ringing_;
FindMyDeviceControllerImpl::~FindMyDeviceControllerImpl() {
do_not_disturb_controller_->RemoveObserver(this);
}
void FindMyDeviceControllerImpl::SetIsPhoneRingingInternal(
bool is_phone_ringing) {
is_phone_ringing_ = is_phone_ringing;
UpdateStatus();
}
FindMyDeviceController::Status
FindMyDeviceControllerImpl::GetPhoneRingingStatus() {
return phone_ringing_status_;
}
void FindMyDeviceControllerImpl::RequestNewPhoneRingingState(bool ringing) {
if (phone_ringing_status_ == Status::kRingingNotAvailable) {
PA_LOG(WARNING) << "Cannot request new ringing status because DoNotDisturb "
<< "mode is enabled.";
return;
}
PA_LOG(INFO) << "Attempting to set Find My Device phone ring state; new "
<< "value: " << ringing;
message_sender_->SendRingDeviceRequest(ringing);
}
void FindMyDeviceControllerImpl::OnDndStateChanged() {
UpdateStatus();
}
FindMyDeviceController::Status FindMyDeviceControllerImpl::ComputeStatus()
const {
if (do_not_disturb_controller_->IsDndEnabled()) {
PA_LOG(WARNING) << "Cannot set ringing status because DoNotDisturb mode is "
<< "enabled.";
return Status::kRingingNotAvailable;
}
return is_phone_ringing_ ? Status::kRingingOn : Status::kRingingOff;
}
void FindMyDeviceControllerImpl::UpdateStatus() {
Status status = ComputeStatus();
if (phone_ringing_status_ == status)
return;
phone_ringing_status_ = status;
NotifyPhoneRingingStateChanged();
}
} // namespace phonehub
......
......@@ -5,24 +5,42 @@
#ifndef CHROMEOS_COMPONENTS_PHONEHUB_FIND_MY_DEVICE_CONTROLLER_IMPL_H_
#define CHROMEOS_COMPONENTS_PHONEHUB_FIND_MY_DEVICE_CONTROLLER_IMPL_H_
#include "chromeos/components/phonehub/do_not_disturb_controller.h"
#include "chromeos/components/phonehub/find_my_device_controller.h"
namespace chromeos {
namespace phonehub {
// TODO(https://crbug.com/1106937): Add real implementation.
class FindMyDeviceControllerImpl : public FindMyDeviceController {
class MessageSender;
// Responsible for sending and receiving updates in regards to the Find My
// Device feature which involves ringing the user's remote phone.
class FindMyDeviceControllerImpl : public FindMyDeviceController,
public DoNotDisturbController::Observer {
public:
FindMyDeviceControllerImpl();
FindMyDeviceControllerImpl(DoNotDisturbController* do_not_disturb_controller,
MessageSender* message_sender);
~FindMyDeviceControllerImpl() override;
private:
friend class FindMyDeviceControllerImplTest;
Status ComputeStatus() const;
void UpdateStatus();
// FindMyDeviceController:
bool IsPhoneRinging() const override;
void SetIsPhoneRingingInternal(bool is_phone_ringing) override;
void RequestNewPhoneRingingState(bool ringing) override;
Status GetPhoneRingingStatus() override;
// DoNotDisturbController::Observer:
void OnDndStateChanged() override;
bool is_phone_ringing_ = false;
Status phone_ringing_status_ = Status::kRingingOff;
DoNotDisturbController* do_not_disturb_controller_;
MessageSender* message_sender_;
};
} // namespace phonehub
......
......@@ -6,6 +6,9 @@
#include <memory>
#include "chromeos/components/phonehub/fake_do_not_disturb_controller.h"
#include "chromeos/components/phonehub/fake_message_sender.h"
#include "chromeos/components/phonehub/find_my_device_controller.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace chromeos {
......@@ -39,25 +42,101 @@ class FindMyDeviceControllerImplTest : public testing::Test {
// testing::Test:
void SetUp() override {
controller_ = std::make_unique<FindMyDeviceControllerImpl>();
fake_do_not_disturb_controller_ =
std::make_unique<FakeDoNotDisturbController>();
fake_message_sender_ = std::make_unique<FakeMessageSender>();
controller_ = std::make_unique<FindMyDeviceControllerImpl>(
fake_do_not_disturb_controller_.get(), fake_message_sender_.get());
controller_->AddObserver(&fake_observer_);
}
void TearDown() override { controller_->RemoveObserver(&fake_observer_); }
bool IsPhoneRinging() const { return controller_->IsPhoneRinging(); }
FindMyDeviceController::Status GetPhoneRingingStatus() const {
return controller_->GetPhoneRingingStatus();
}
void SetIsPhoneRingingInternal(bool is_phone_ringing) {
controller_->SetIsPhoneRingingInternal(is_phone_ringing);
}
void RequestNewPhoneRingingState(bool ringing) {
controller_->RequestNewPhoneRingingState(ringing);
}
size_t GetNumObserverCalls() const { return fake_observer_.num_calls(); }
protected:
std::unique_ptr<FakeDoNotDisturbController> fake_do_not_disturb_controller_;
std::unique_ptr<FakeMessageSender> fake_message_sender_;
private:
std::unique_ptr<FindMyDeviceControllerImpl> controller_;
FakeObserver fake_observer_;
std::unique_ptr<FindMyDeviceController> controller_;
};
// TODO(https://crbug.com/1106937): Remove this test once we have real
// functionality to test.
TEST_F(FindMyDeviceControllerImplTest, Initialize) {
EXPECT_FALSE(IsPhoneRinging());
TEST_F(FindMyDeviceControllerImplTest, RingingStateChanges) {
EXPECT_EQ(FindMyDeviceController::Status::kRingingOff,
GetPhoneRingingStatus());
// Simulate flipping DoNotDisturb mode to enabled, this should set the
// FindMyPhone status to kRingingNotAvailable.
fake_do_not_disturb_controller_->SetDoNotDisturbStateInternal(
/*is_dnd_enabled=*/true);
EXPECT_EQ(FindMyDeviceController::Status::kRingingNotAvailable,
GetPhoneRingingStatus());
// Simulate initiating phone ringing when DoNotDisturb mode is enabled. This
// will not update the internal status.
SetIsPhoneRingingInternal(/*is_phone_ringing=*/true);
EXPECT_EQ(FindMyDeviceController::Status::kRingingNotAvailable,
GetPhoneRingingStatus());
EXPECT_EQ(1u, GetNumObserverCalls());
// Flip DoNotDisturb back to disabled, expect status to reset back to its
// previous state.
fake_do_not_disturb_controller_->SetDoNotDisturbStateInternal(
/*is_dnd_enabled=*/false);
// Since we previously recorded that the phone should be ringing during
// DoNotDisturb mode was enabled, we return to that state once DoNotDisturb
// is disabled.
EXPECT_EQ(FindMyDeviceController::Status::kRingingOn,
GetPhoneRingingStatus());
EXPECT_EQ(2u, GetNumObserverCalls());
// Attempt to set ringing status with the same previous state. Expect that no
// observer calls were made.
SetIsPhoneRingingInternal(/*is_phone_ringing=*/true);
EXPECT_EQ(2u, GetNumObserverCalls());
}
TEST_F(FindMyDeviceControllerImplTest, RequestNewRingStatus) {
RequestNewPhoneRingingState(/*ringing=*/true);
EXPECT_EQ(1u, fake_message_sender_->GetRingDeviceRequestCallCount());
EXPECT_TRUE(fake_message_sender_->GetRecentRingDeviceRequest());
// 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);
EXPECT_EQ(FindMyDeviceController::Status::kRingingNotAvailable,
GetPhoneRingingStatus());
RequestNewPhoneRingingState(/*ringing=*/false);
EXPECT_EQ(1u, fake_message_sender_->GetRingDeviceRequestCallCount());
// No new messages were sent, expect that the last request was still the
// previous "true" value.
EXPECT_TRUE(fake_message_sender_->GetRecentRingDeviceRequest());
// Flip DoNotDisturb mode to disabled, expect that messages are able to be
// sent again.
fake_do_not_disturb_controller_->SetDoNotDisturbStateInternal(
/*is_dnd_enabled=*/false);
EXPECT_EQ(FindMyDeviceController::Status::kRingingOff,
GetPhoneRingingStatus());
RequestNewPhoneRingingState(/*ringing=*/false);
EXPECT_EQ(2u, fake_message_sender_->GetRingDeviceRequestCallCount());
EXPECT_FALSE(fake_message_sender_->GetRecentRingDeviceRequest());
}
} // namespace phonehub
......
......@@ -45,8 +45,9 @@ PhoneHubManagerImpl::PhoneHubManagerImpl(
connection_scheduler_(std::make_unique<ConnectionSchedulerImpl>(
connection_manager_.get(),
feature_status_provider_.get())),
find_my_device_controller_(
std::make_unique<FindMyDeviceControllerImpl>()),
find_my_device_controller_(std::make_unique<FindMyDeviceControllerImpl>(
do_not_disturb_controller_.get(),
message_sender_.get())),
notification_access_manager_(
std::make_unique<NotificationAccessManagerImpl>(pref_service)),
notification_manager_(std::make_unique<NotificationManagerImpl>()),
......
......@@ -137,7 +137,8 @@ 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_find_my_device_controller_->IsPhoneRinging());
EXPECT_EQ(FindMyDeviceController::Status::kRingingOn,
fake_find_my_device_controller_->GetPhoneRingingStatus());
EXPECT_TRUE(fake_notification_access_manager_->HasAccessBeenGranted());
base::Optional<PhoneStatusModel> phone_status_model =
......@@ -198,7 +199,8 @@ 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_TRUE(fake_find_my_device_controller_->IsPhoneRinging());
EXPECT_EQ(FindMyDeviceController::Status::kRingingOn,
fake_find_my_device_controller_->GetPhoneRingingStatus());
EXPECT_TRUE(fake_notification_access_manager_->HasAccessBeenGranted());
base::Optional<PhoneStatusModel> phone_status_model =
......@@ -221,7 +223,8 @@ 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_TRUE(fake_find_my_device_controller_->IsPhoneRinging());
EXPECT_EQ(FindMyDeviceController::Status::kRingingOn,
fake_find_my_device_controller_->GetPhoneRingingStatus());
EXPECT_TRUE(fake_notification_access_manager_->HasAccessBeenGranted());
phone_status_model = mutable_phone_model_->phone_status_model();
......
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