Skip to content

Commit

Permalink
[CameraRoll]Send camera roll setting to the Android device in CrosState
Browse files Browse the repository at this point in the history
Include camera roll setting state as part of CrosState,
so connected mobile device would be able to get update
when setting value is toggled.

Change-Id: I04d0ed3872d5adeff5e8f8dc76c6eb6df3a50b9c
Bug: https://crbug.com/1221297
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3173740
Commit-Queue: Jianbing Wu <jianbing@google.com>
Auto-Submit: Jianbing Wu <jianbing@google.com>
Reviewed-by: Jon Mann <jonmann@chromium.org>
Cr-Commit-Position: refs/heads/main@{#924995}
  • Loading branch information
jianbingfruit authored and Chromium LUCI CQ committed Sep 25, 2021
1 parent 3c560fa commit 3895114
Show file tree
Hide file tree
Showing 9 changed files with 58 additions and 19 deletions.
10 changes: 8 additions & 2 deletions chromeos/components/phonehub/cros_state_sender.cc
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,16 @@ void CrosStateSender::PerformUpdateCrosState() {
bool are_notifications_enabled =
multidevice_setup_client_->GetFeatureState(
Feature::kPhoneHubNotifications) == FeatureState::kEnabledByUser;
bool is_camera_roll_enabled =
multidevice_setup_client_->GetFeatureState(
Feature::kPhoneHubCameraRoll) == FeatureState::kEnabledByUser;

PA_LOG(INFO) << "Attempting to send cros state with notifications enabled "
<< "state as: " << are_notifications_enabled;
message_sender_->SendCrosState(are_notifications_enabled);
<< "state as: " << are_notifications_enabled
<< " and camera roll enabled state as: "
<< is_camera_roll_enabled;
message_sender_->SendCrosState(are_notifications_enabled,
is_camera_roll_enabled);

retry_timer_->Start(FROM_HERE, retry_delay_,
base::BindOnce(&CrosStateSender::OnRetryTimerFired,
Expand Down
27 changes: 20 additions & 7 deletions chromeos/components/phonehub/cros_state_sender_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,9 @@ TEST_F(CrosStateSenderTest, UpdatesOnConnected) {
// Set notification feature to be enabled.
fake_multidevice_setup_client_->SetFeatureState(
Feature::kPhoneHubNotifications, FeatureState::kEnabledByUser);
// Set camera roll feature to be enabled.
fake_multidevice_setup_client_->SetFeatureState(Feature::kPhoneHubCameraRoll,
FeatureState::kEnabledByUser);
// Expect no new messages since connection has not been established.
EXPECT_EQ(0u, fake_message_sender_->GetCrosStateCallCount());
EXPECT_FALSE(mock_timer_->IsRunning());
Expand All @@ -120,7 +123,8 @@ TEST_F(CrosStateSenderTest, UpdatesOnConnected) {
// Simulate connected state. Expect a new message to be sent.
fake_connection_manager_->SetStatus(
secure_channel::ConnectionManager::Status::kConnected);
EXPECT_TRUE(fake_message_sender_->GetRecentCrosState());
EXPECT_TRUE(fake_message_sender_->GetRecentCrosState().first);
EXPECT_TRUE(fake_message_sender_->GetRecentCrosState().second);
EXPECT_EQ(1u, fake_message_sender_->GetCrosStateCallCount());

// Phone model is populated.
Expand All @@ -131,7 +135,8 @@ TEST_F(CrosStateSenderTest, UpdatesOnConnected) {
// Simulate disconnected state, this should not trigger a new request.
fake_connection_manager_->SetStatus(
secure_channel::ConnectionManager::Status::kDisconnected);
EXPECT_TRUE(fake_message_sender_->GetRecentCrosState());
EXPECT_TRUE(fake_message_sender_->GetRecentCrosState().first);
EXPECT_TRUE(fake_message_sender_->GetRecentCrosState().second);
EXPECT_EQ(1u, fake_message_sender_->GetCrosStateCallCount());
EXPECT_FALSE(mock_timer_->IsRunning());
}
Expand All @@ -146,36 +151,44 @@ TEST_F(CrosStateSenderTest, NotificationFeatureStateChanged) {
EXPECT_TRUE(mock_timer_->IsRunning());

// Expect new messages to be sent when connection state is connected.
EXPECT_FALSE(fake_message_sender_->GetRecentCrosState());
EXPECT_FALSE(fake_message_sender_->GetRecentCrosState().first);
EXPECT_FALSE(fake_message_sender_->GetRecentCrosState().second);
EXPECT_EQ(1u, fake_message_sender_->GetCrosStateCallCount());
mock_timer_->Fire();

// Simulate enabling notification feature state and expect cros state to be
// enabled.
fake_multidevice_setup_client_->SetFeatureState(
Feature::kPhoneHubNotifications, FeatureState::kEnabledByUser);
EXPECT_TRUE(fake_message_sender_->GetRecentCrosState());
EXPECT_TRUE(fake_message_sender_->GetRecentCrosState().first);
EXPECT_EQ(2u, fake_message_sender_->GetCrosStateCallCount());
mock_timer_->Fire();

// Update a different feature state and expect that it did not affect the
// cros state.
fake_multidevice_setup_client_->SetFeatureState(
Feature::kSmartLock, FeatureState::kDisabledByUser);
EXPECT_TRUE(fake_message_sender_->GetRecentCrosState());
EXPECT_TRUE(fake_message_sender_->GetRecentCrosState().first);
EXPECT_EQ(3u, fake_message_sender_->GetCrosStateCallCount());
mock_timer_->Fire();

// Simulate disabling notification feature state and expect cros state to be
// disabled.
fake_multidevice_setup_client_->SetFeatureState(
Feature::kPhoneHubNotifications, FeatureState::kDisabledByUser);
EXPECT_FALSE(fake_message_sender_->GetRecentCrosState());
EXPECT_FALSE(fake_message_sender_->GetRecentCrosState().first);
EXPECT_EQ(4u, fake_message_sender_->GetCrosStateCallCount());

// Simulate enabling camera roll feature state and expect cros state to be
// updated.
fake_multidevice_setup_client_->SetFeatureState(Feature::kPhoneHubCameraRoll,
FeatureState::kEnabledByUser);
EXPECT_TRUE(fake_message_sender_->GetRecentCrosState().second);
EXPECT_EQ(5u, fake_message_sender_->GetCrosStateCallCount());

// Firing the timer does not cause the cros state to be sent again.
mock_timer_->Fire();
EXPECT_EQ(4u, fake_message_sender_->GetCrosStateCallCount());
EXPECT_EQ(5u, fake_message_sender_->GetCrosStateCallCount());
}

} // namespace phonehub
Expand Down
8 changes: 5 additions & 3 deletions chromeos/components/phonehub/fake_message_sender.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ namespace phonehub {
FakeMessageSender::FakeMessageSender() = default;
FakeMessageSender::~FakeMessageSender() = default;

void FakeMessageSender::SendCrosState(bool notification_enabled) {
cros_states_.push_back(notification_enabled);
void FakeMessageSender::SendCrosState(bool notification_enabled,
bool camera_roll_enabled) {
cros_states_.push_back(
std::make_pair(notification_enabled, camera_roll_enabled));
}

void FakeMessageSender::SendUpdateNotificationModeRequest(
Expand Down Expand Up @@ -77,7 +79,7 @@ size_t FakeMessageSender::GetFetchCameraRollItemsRequestCallCount() const {
return fetch_camera_roll_items_requests_.size();
}

bool FakeMessageSender::GetRecentCrosState() const {
std::pair<bool, bool> FakeMessageSender::GetRecentCrosState() const {
return cros_states_.back();
}

Expand Down
9 changes: 6 additions & 3 deletions chromeos/components/phonehub/fake_message_sender.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ class FakeMessageSender : public MessageSender {
~FakeMessageSender() override;

// MessageSender:
void SendCrosState(bool notification_enabled) override;
void SendCrosState(bool notification_enabled,
bool camera_roll_enabled) override;
void SendUpdateNotificationModeRequest(bool do_not_disturb_enabled) override;
void SendUpdateBatteryModeRequest(bool battery_saver_mode_enabled) override;
void SendDismissNotificationRequest(int64_t notification_id) override;
Expand All @@ -34,7 +35,7 @@ class FakeMessageSender : public MessageSender {
void SendFetchCameraRollItemsRequest(
const proto::FetchCameraRollItemsRequest& request) override;

bool GetRecentCrosState() const;
std::pair<bool, bool> GetRecentCrosState() const;
bool GetRecentUpdateNotificationModeRequest() const;
bool GetRecentUpdateBatteryModeRequest() const;
int64_t GetRecentDismissNotificationRequest() const;
Expand Down Expand Up @@ -63,7 +64,9 @@ class FakeMessageSender : public MessageSender {
size_t GetFetchCameraRollItemsRequestCallCount() const;

private:
std::vector<bool> cros_states_;
std::vector<std::pair</*is_notifications_setting_enabled*/ bool,
/*is_camera_roll_setting_enabled*/ bool>>
cros_states_;
std::vector<bool> update_notification_mode_requests_;
std::vector<bool> update_battery_mode_requests_;
std::vector<int64_t> dismiss_notification_requests_;
Expand Down
3 changes: 2 additions & 1 deletion chromeos/components/phonehub/message_sender.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ class MessageSender {
virtual ~MessageSender() = default;

// Sends whether the notification setting is enabled in the Chrome OS device.
virtual void SendCrosState(bool notification_setting_enabled) = 0;
virtual void SendCrosState(bool notification_setting_enabled,
bool camera_roll_setting_enabled) = 0;

// Requests that the phone enables or disables Do Not Disturb mode.
virtual void SendUpdateNotificationModeRequest(
Expand Down
7 changes: 6 additions & 1 deletion chromeos/components/phonehub/message_sender_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,18 @@ MessageSenderImpl::MessageSenderImpl(

MessageSenderImpl::~MessageSenderImpl() = default;

void MessageSenderImpl::SendCrosState(bool notification_setting_enabled) {
void MessageSenderImpl::SendCrosState(bool notification_setting_enabled,
bool camera_roll_setting_enabled) {
proto::NotificationSetting is_notification_enabled =
notification_setting_enabled
? proto::NotificationSetting::NOTIFICATIONS_ON
: proto::NotificationSetting::NOTIFICATIONS_OFF;
proto::CameraRollSetting is_camera_roll_enabled =
camera_roll_setting_enabled ? proto::CameraRollSetting::CAMERA_ROLL_ON
: proto::CameraRollSetting::CAMERA_ROLL_OFF;
proto::CrosState request;
request.set_notification_setting(is_notification_enabled);
request.set_camera_roll_setting(is_camera_roll_enabled);

SendMessage(proto::MessageType::PROVIDE_CROS_STATE, &request);
}
Expand Down
3 changes: 2 additions & 1 deletion chromeos/components/phonehub/message_sender_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ class MessageSenderImpl : public MessageSender {
~MessageSenderImpl() override;

// MessageSender:
void SendCrosState(bool notification_setting_enabled) override;
void SendCrosState(bool notification_setting_enabled,
bool camera_roll_setting_enabled) override;
void SendUpdateNotificationModeRequest(bool do_not_disturb_enabled) override;
void SendUpdateBatteryModeRequest(bool battery_saver_mode_enabled) override;
void SendDismissNotificationRequest(int64_t notification_id) override;
Expand Down
4 changes: 3 additions & 1 deletion chromeos/components/phonehub/message_sender_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,10 @@ TEST_F(MessageSenderImplTest, SendCrossState) {
proto::CrosState request;
request.set_notification_setting(
proto::NotificationSetting::NOTIFICATIONS_ON);
request.set_camera_roll_setting(proto::CameraRollSetting::CAMERA_ROLL_OFF);

message_sender_->SendCrosState(/*notification_enabled=*/true);
message_sender_->SendCrosState(/*notification_enabled=*/true,
/*camera_roll_enabled=*/false);
VerifyMessage(proto::MessageType::PROVIDE_CROS_STATE, &request,
fake_connection_manager_->sent_messages().back());
}
Expand Down
6 changes: 6 additions & 0 deletions chromeos/components/phonehub/proto/phonehub_api.proto
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,11 @@ enum NotificationSetting {
NOTIFICATIONS_ON = 1;
}

enum CameraRollSetting {
CAMERA_ROLL_OFF = 0;
CAMERA_ROLL_ON = 1;
}

enum ChargingState {
NOT_CHARGING = 0;
CHARGING_AC = 1;
Expand Down Expand Up @@ -162,6 +167,7 @@ message App {

message CrosState {
NotificationSetting notification_setting = 1;
CameraRollSetting camera_roll_setting = 2;
}

message Action {
Expand Down

0 comments on commit 3895114

Please sign in to comment.