From 7a0ee45432bf8e1c284216525de2d49421cc3e78 Mon Sep 17 00:00:00 2001 From: hychao Date: Thu, 19 May 2016 04:29:10 -0700 Subject: [PATCH] When a headset or other application changes the active node's volume on system, Chrome UI should know that and updates the stored volume accordingly. BUG=609458 TEST=Run cras_test_client --set_node_volume on the active node to verify Chrome UI shows volume bar to reflect the change. Review-Url: https://codereview.chromium.org/1952983002 Cr-Commit-Position: refs/heads/master@{#394738} --- chromeos/audio/cras_audio_handler.cc | 22 +++++++ chromeos/audio/cras_audio_handler.h | 1 + chromeos/dbus/cras_audio_client.cc | 28 +++++++++ chromeos/dbus/cras_audio_client.h | 3 + chromeos/dbus/cras_audio_client_unittest.cc | 65 +++++++++++++++++++++ 5 files changed, 119 insertions(+) diff --git a/chromeos/audio/cras_audio_handler.cc b/chromeos/audio/cras_audio_handler.cc index 75a391d576dc3..a6586bd742e07 100644 --- a/chromeos/audio/cras_audio_handler.cc +++ b/chromeos/audio/cras_audio_handler.cc @@ -592,6 +592,28 @@ void CrasAudioHandler::NodesChanged() { GetNodes(); } +void CrasAudioHandler::OutputNodeVolumeChanged(uint64_t node_id, int volume) { + const AudioDevice* device = this->GetDeviceFromId(node_id); + int old_volume; + + // If this is not an active output node, ignore this event. Because when this + // node set to active, it will be applied with the volume value stored in + // preference. + if (!device || !device->active || device->is_input) + return; + + // If this callback is triggered by a response to previous set volume command, + // do nothing. + old_volume = + static_cast(audio_pref_handler_->GetOutputVolumeValue(device)); + if (old_volume == volume) + return; + + // Otherwise another app or the hardware itself just changed volume, update + // the new volume value to all active output nodes. + SetOutputVolumePercent(volume); +} + void CrasAudioHandler::ActiveOutputNodeChanged(uint64_t node_id) { if (active_output_node_id_ == node_id) return; diff --git a/chromeos/audio/cras_audio_handler.h b/chromeos/audio/cras_audio_handler.h index 57bf47de4fe13..177e9e2f54e20 100644 --- a/chromeos/audio/cras_audio_handler.h +++ b/chromeos/audio/cras_audio_handler.h @@ -242,6 +242,7 @@ class CHROMEOS_EXPORT CrasAudioHandler : public CrasAudioClient::Observer, void NodesChanged() override; void ActiveOutputNodeChanged(uint64_t node_id) override; void ActiveInputNodeChanged(uint64_t node_id) override; + void OutputNodeVolumeChanged(uint64_t node_id, int volume) override; // AudioPrefObserver overrides. void OnAudioPolicyPrefChanged() override; diff --git a/chromeos/dbus/cras_audio_client.cc b/chromeos/dbus/cras_audio_client.cc index f2ce0291f99a2..e9957ec47a107 100644 --- a/chromeos/dbus/cras_audio_client.cc +++ b/chromeos/dbus/cras_audio_client.cc @@ -257,6 +257,15 @@ class CrasAudioClientImpl : public CrasAudioClient { weak_ptr_factory_.GetWeakPtr()), base::Bind(&CrasAudioClientImpl::SignalConnected, weak_ptr_factory_.GetWeakPtr())); + + // Monitor the D-Bus signal for output node volume change. + cras_proxy_->ConnectToSignal( + cras::kCrasControlInterface, + cras::kOutputNodeVolumeChanged, + base::Bind(&CrasAudioClientImpl::OutputNodeVolumeChangedReceived, + weak_ptr_factory_.GetWeakPtr()), + base::Bind(&CrasAudioClientImpl::SignalConnected, + weak_ptr_factory_.GetWeakPtr())); } private: @@ -321,6 +330,21 @@ class CrasAudioClientImpl : public CrasAudioClient { FOR_EACH_OBSERVER(Observer, observers_, ActiveInputNodeChanged(node_id)); } + void OutputNodeVolumeChangedReceived(dbus::Signal* signal) { + dbus::MessageReader reader(signal); + uint64_t node_id; + int volume; + + if (!reader.PopUint64(&node_id)) { + LOG(ERROR) << "Error eading signal from cras:" << signal->ToString(); + } + if (!reader.PopInt32(&volume)) { + LOG(ERROR) << "Error eading signal from cras:" << signal->ToString(); + } + FOR_EACH_OBSERVER(Observer, observers_, + OutputNodeVolumeChanged(node_id, volume)); + } + void OnGetVolumeState(const GetVolumeStateCallback& callback, dbus::Response* response) { bool success = true; @@ -469,6 +493,10 @@ void CrasAudioClient::Observer::ActiveOutputNodeChanged(uint64_t node_id) {} void CrasAudioClient::Observer::ActiveInputNodeChanged(uint64_t node_id) {} +void CrasAudioClient::Observer::OutputNodeVolumeChanged(uint64_t node_id, + int volume) { +} + CrasAudioClient::CrasAudioClient() { } diff --git a/chromeos/dbus/cras_audio_client.h b/chromeos/dbus/cras_audio_client.h index 3851e516a1995..54284be264527 100644 --- a/chromeos/dbus/cras_audio_client.h +++ b/chromeos/dbus/cras_audio_client.h @@ -42,6 +42,9 @@ class CHROMEOS_EXPORT CrasAudioClient : public DBusClient { // Called when active audio input node changed to new node with |node_id|. virtual void ActiveInputNodeChanged(uint64_t node_id); + // Called when output node's volume changed. + virtual void OutputNodeVolumeChanged(uint64_t node_id, int volume); + protected: virtual ~Observer(); }; diff --git a/chromeos/dbus/cras_audio_client_unittest.cc b/chromeos/dbus/cras_audio_client_unittest.cc index a2993bd370c1f..d25552315d9da 100644 --- a/chromeos/dbus/cras_audio_client_unittest.cc +++ b/chromeos/dbus/cras_audio_client_unittest.cc @@ -69,6 +69,7 @@ class MockObserver : public CrasAudioClient::Observer { MOCK_METHOD0(NodesChanged, void()); MOCK_METHOD1(ActiveOutputNodeChanged, void(uint64_t node_id)); MOCK_METHOD1(ActiveInputNodeChanged, void(uint64_t node_id)); + MOCK_METHOD2(OutputNodeVolumeChanged, void(uint64_t node_id, int volume)); }; // Expect the reader to be empty. @@ -281,6 +282,16 @@ class CrasAudioClientTest : public testing::Test { Invoke(this, &CrasAudioClientTest::OnConnectToActiveInputNodeChanged)); + // Set an expectation so mock_cras_proxy's monitoring + // OutputNodeVolumeChanged ConnectToSignal will use + // OnConnectToOutputNodeVolumeChanged() to run the callback. + EXPECT_CALL( + *mock_cras_proxy_.get(), + ConnectToSignal(interface_name_, cras::kOutputNodeVolumeChanged, _, _)) + .WillRepeatedly( + Invoke(this, + &CrasAudioClientTest::OnConnectToOutputNodeVolumeChanged)); + // Set an expectation so mock_bus's GetObjectProxy() for the given // service name and the object path will return mock_cras_proxy_. EXPECT_CALL(*mock_bus_.get(), @@ -344,6 +355,12 @@ class CrasAudioClientTest : public testing::Test { active_input_node_changed_handler_.Run(signal); } + // Send output node volume changed signal to the tested client. + void SendOutputNodeVolumeChangedSignal(dbus::Signal *signal) { + ASSERT_FALSE(output_node_volume_changed_handler_.is_null()); + output_node_volume_changed_handler_.Run(signal); + } + // The interface name. const std::string interface_name_; // The client to be tested. @@ -364,6 +381,8 @@ class CrasAudioClientTest : public testing::Test { dbus::ObjectProxy::SignalCallback active_output_node_changed_handler_; // The ActiveInputNodeChanged signal handler given by the tested client. dbus::ObjectProxy::SignalCallback active_input_node_changed_handler_; + // The OutputNodeVolumeChanged signal handler given by the tested client. + dbus::ObjectProxy::SignalCallback output_node_volume_changed_handler_; // The name of the method which is expected to be called. std::string expected_method_name_; // The response which the mock cras proxy returns. @@ -452,6 +471,22 @@ class CrasAudioClientTest : public testing::Test { success)); } + // Checks the requested interface name and signal name. + // Used to implement the mock cras proxy. + void OnConnectToOutputNodeVolumeChanged( + const std::string& interface_name, + const std::string& signal_name, + const dbus::ObjectProxy::SignalCallback& signal_callback, + const dbus::ObjectProxy::OnConnectedCallback& on_connected_callback) { + output_node_volume_changed_handler_ = signal_callback; + const bool success = true; + message_loop_.task_runner()->PostTask(FROM_HERE, + base::Bind(on_connected_callback, + interface_name, + signal_name, + success)); + } + // Checks the content of the method call and returns the response. // Used to implement the mock cras proxy. void OnCallMethod(dbus::MethodCall* method_call, @@ -617,6 +652,36 @@ TEST_F(CrasAudioClientTest, ActiveInputNodeChanged) { message_loop_.RunUntilIdle(); } +TEST_F(CrasAudioClientTest, OutputNodeVolumeChanged) { + const uint64_t kNodeId = 20003; + const int32_t volume = 82; + // Create a signal + dbus::Signal signal(cras::kCrasControlInterface, + cras::kOutputNodeVolumeChanged); + dbus::MessageWriter writer(&signal); + writer.AppendUint64(kNodeId); + writer.AppendInt32(volume); + + // Set expectations + MockObserver observer; + EXPECT_CALL(observer, OutputNodeVolumeChanged(kNodeId, volume)).Times(1); + + // Add the observer. + client_->AddObserver(&observer); + + // Run the signal callback. + SendOutputNodeVolumeChangedSignal(&signal); + + // Remove the observer. + client_->RemoveObserver(&observer); + EXPECT_CALL(observer, OutputNodeVolumeChanged(_, _)).Times(0); + + // Run the signal callback again and make sure the observer isn't called. + SendOutputNodeVolumeChangedSignal(&signal); + + message_loop_.RunUntilIdle(); +} + TEST_F(CrasAudioClientTest, GetNodes) { // Create the expected value. AudioNodeList expected_node_list;