From 3895114c9dd91cc02e5d289bad5ef3cb6316a1c9 Mon Sep 17 00:00:00 2001 From: Jianbing Date: Sat, 25 Sep 2021 01:31:25 +0000 Subject: [PATCH] [CameraRoll]Send camera roll setting to the Android device in CrosState 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 Auto-Submit: Jianbing Wu Reviewed-by: Jon Mann Cr-Commit-Position: refs/heads/main@{#924995} --- .../components/phonehub/cros_state_sender.cc | 10 +++++-- .../phonehub/cros_state_sender_unittest.cc | 27 ++++++++++++++----- .../phonehub/fake_message_sender.cc | 8 +++--- .../components/phonehub/fake_message_sender.h | 9 ++++--- chromeos/components/phonehub/message_sender.h | 3 ++- .../phonehub/message_sender_impl.cc | 7 ++++- .../components/phonehub/message_sender_impl.h | 3 ++- .../phonehub/message_sender_unittest.cc | 4 ++- .../phonehub/proto/phonehub_api.proto | 6 +++++ 9 files changed, 58 insertions(+), 19 deletions(-) diff --git a/chromeos/components/phonehub/cros_state_sender.cc b/chromeos/components/phonehub/cros_state_sender.cc index 361a07045d965..01786746f36d8 100644 --- a/chromeos/components/phonehub/cros_state_sender.cc +++ b/chromeos/components/phonehub/cros_state_sender.cc @@ -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, diff --git a/chromeos/components/phonehub/cros_state_sender_unittest.cc b/chromeos/components/phonehub/cros_state_sender_unittest.cc index 58a32f587544d..60dbdd771d799 100644 --- a/chromeos/components/phonehub/cros_state_sender_unittest.cc +++ b/chromeos/components/phonehub/cros_state_sender_unittest.cc @@ -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()); @@ -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. @@ -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()); } @@ -146,7 +151,8 @@ 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(); @@ -154,7 +160,7 @@ TEST_F(CrosStateSenderTest, NotificationFeatureStateChanged) { // 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(); @@ -162,7 +168,7 @@ TEST_F(CrosStateSenderTest, NotificationFeatureStateChanged) { // 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(); @@ -170,12 +176,19 @@ TEST_F(CrosStateSenderTest, NotificationFeatureStateChanged) { // 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 diff --git a/chromeos/components/phonehub/fake_message_sender.cc b/chromeos/components/phonehub/fake_message_sender.cc index 180d57a87986c..506fcdd047d91 100644 --- a/chromeos/components/phonehub/fake_message_sender.cc +++ b/chromeos/components/phonehub/fake_message_sender.cc @@ -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( @@ -77,7 +79,7 @@ size_t FakeMessageSender::GetFetchCameraRollItemsRequestCallCount() const { return fetch_camera_roll_items_requests_.size(); } -bool FakeMessageSender::GetRecentCrosState() const { +std::pair FakeMessageSender::GetRecentCrosState() const { return cros_states_.back(); } diff --git a/chromeos/components/phonehub/fake_message_sender.h b/chromeos/components/phonehub/fake_message_sender.h index 8ac1cfaf71808..aeb2b82de8ac7 100644 --- a/chromeos/components/phonehub/fake_message_sender.h +++ b/chromeos/components/phonehub/fake_message_sender.h @@ -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; @@ -34,7 +35,7 @@ class FakeMessageSender : public MessageSender { void SendFetchCameraRollItemsRequest( const proto::FetchCameraRollItemsRequest& request) override; - bool GetRecentCrosState() const; + std::pair GetRecentCrosState() const; bool GetRecentUpdateNotificationModeRequest() const; bool GetRecentUpdateBatteryModeRequest() const; int64_t GetRecentDismissNotificationRequest() const; @@ -63,7 +64,9 @@ class FakeMessageSender : public MessageSender { size_t GetFetchCameraRollItemsRequestCallCount() const; private: - std::vector cros_states_; + std::vector> + cros_states_; std::vector update_notification_mode_requests_; std::vector update_battery_mode_requests_; std::vector dismiss_notification_requests_; diff --git a/chromeos/components/phonehub/message_sender.h b/chromeos/components/phonehub/message_sender.h index 4cd09b9529448..a9c529992fe92 100644 --- a/chromeos/components/phonehub/message_sender.h +++ b/chromeos/components/phonehub/message_sender.h @@ -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( diff --git a/chromeos/components/phonehub/message_sender_impl.cc b/chromeos/components/phonehub/message_sender_impl.cc index d3d2cd312cd5f..d16e36d443042 100644 --- a/chromeos/components/phonehub/message_sender_impl.cc +++ b/chromeos/components/phonehub/message_sender_impl.cc @@ -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); } diff --git a/chromeos/components/phonehub/message_sender_impl.h b/chromeos/components/phonehub/message_sender_impl.h index 0f19ec24c89d4..cd689b7061f0e 100644 --- a/chromeos/components/phonehub/message_sender_impl.h +++ b/chromeos/components/phonehub/message_sender_impl.h @@ -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; diff --git a/chromeos/components/phonehub/message_sender_unittest.cc b/chromeos/components/phonehub/message_sender_unittest.cc index 5df0ddd7c9858..9d1d053155ccd 100644 --- a/chromeos/components/phonehub/message_sender_unittest.cc +++ b/chromeos/components/phonehub/message_sender_unittest.cc @@ -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()); } diff --git a/chromeos/components/phonehub/proto/phonehub_api.proto b/chromeos/components/phonehub/proto/phonehub_api.proto index 527dee8b76458..6ff53376be024 100644 --- a/chromeos/components/phonehub/proto/phonehub_api.proto +++ b/chromeos/components/phonehub/proto/phonehub_api.proto @@ -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; @@ -162,6 +167,7 @@ message App { message CrosState { NotificationSetting notification_setting = 1; + CameraRollSetting camera_roll_setting = 2; } message Action {