Skip to content

Commit

Permalink
Rename misleading names in FakeCrasAudioClient
Browse files Browse the repository at this point in the history
The methods `set_notify_volume_change_with_delay` and
`set_notify_gain_change_with_delay` did not *delay* informing
the observers, but instead they *stopped* informing the observers.

This CL renames them to `disable_volume_change_events` and
`disable_gain_change_events` because:
   * The old names lied, and ended up costing me quite some time today.
   * I will actually introduce methods to make these events asynchronous
     in a follow-up CL (since that closer matches the reality, and this
     caused things to be broken even when tests passed).

Bug: b/271813821
Change-Id: Idbbf34bfb8ddce7b4ec8456611aa06dc77ac1625
Cq-Include-Trybots: luci.chrome.try:linux-chromeos-chrome
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4324020
Commit-Queue: Jeroen Dhollander <jeroendh@google.com>
Reviewed-by: Li-Yu Yu <aaronyu@google.com>
Cr-Commit-Position: refs/heads/main@{#1115600}
  • Loading branch information
Jeroen Dhollander authored and Chromium LUCI CQ committed Mar 10, 2023
1 parent 0a9136a commit 6de20cc
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 21 deletions.
14 changes: 7 additions & 7 deletions chromeos/ash/components/audio/cras_audio_handler_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2469,7 +2469,7 @@ TEST_P(CrasAudioHandlerTest, RestartAudioClientWithCrasReady) {

const int kDefaultVolume = cras_audio_handler_->GetOutputVolumePercent();
// Disable the auto OutputNodeVolumeChanged signal.
fake_cras_audio_client()->set_notify_volume_change_with_delay(true);
fake_cras_audio_client()->disable_volume_change_events();

fake_cras_audio_client()->SetAudioNodesForTesting(audio_nodes);
RestartAudioClient();
Expand Down Expand Up @@ -2499,7 +2499,7 @@ TEST_P(CrasAudioHandlerTest, RestartAudioClientWithCrasDropRequest) {

const int kDefaultVolume = cras_audio_handler_->GetOutputVolumePercent();
// Disable the auto OutputNodeVolumeChanged signal.
fake_cras_audio_client()->set_notify_volume_change_with_delay(true);
fake_cras_audio_client()->disable_volume_change_events();

fake_cras_audio_client()->SetAudioNodesForTesting(audio_nodes);
RestartAudioClient();
Expand Down Expand Up @@ -2533,7 +2533,7 @@ TEST_P(CrasAudioHandlerTest, SetOutputVolumeWithDelayedSignal) {
EXPECT_EQ(kDefaultVolume, cras_audio_handler_->GetOutputVolumePercent());

// Disable the auto OutputNodeVolumeChanged signal.
fake_cras_audio_client()->set_notify_volume_change_with_delay(true);
fake_cras_audio_client()->disable_volume_change_events();

// Verify the volume state is not changed before OutputNodeVolumeChanged
// signal fires.
Expand Down Expand Up @@ -2565,7 +2565,7 @@ TEST_P(CrasAudioHandlerTest,
EXPECT_EQ(kDefaultVolume, cras_audio_handler_->GetOutputVolumePercent());

// Disable the auto OutputNodeVolumeChanged signal.
fake_cras_audio_client()->set_notify_volume_change_with_delay(true);
fake_cras_audio_client()->disable_volume_change_events();

// Verify the volume state is not changed before OutputNodeVolumeChanged
// signal fires.
Expand Down Expand Up @@ -2664,7 +2664,7 @@ TEST_P(CrasAudioHandlerTest, SetInputGainWithDelayedSignal) {
const int kDefaultGain = cras_audio_handler_->GetInputGainPercent();

// Disable the auto InputNodeGainChanged signal.
fake_cras_audio_client()->set_notify_gain_change_with_delay(true);
fake_cras_audio_client()->disable_gain_change_events();

// Verify the gain state is not changed before InputNodeGainChanged
// signal fires.
Expand Down Expand Up @@ -2695,7 +2695,7 @@ TEST_P(CrasAudioHandlerTest,
const int kDefaultGain = cras_audio_handler_->GetInputGainPercent();

// Disable the auto InputNodeGainChanged signal.
fake_cras_audio_client()->set_notify_gain_change_with_delay(true);
fake_cras_audio_client()->disable_gain_change_events();

// Verify the gain state is not changed before InputNodeGainChanged
// signal fires.
Expand Down Expand Up @@ -3410,7 +3410,7 @@ TEST_P(CrasAudioHandlerTest, ChangeVolumeHotrodDualSpeakersWithDelayedSignals) {
EXPECT_EQ(kDefaultVolume, cras_audio_handler_->GetOutputVolumePercent());

// Disable the auto OutputNodeVolumeChanged signal.
fake_cras_audio_client()->set_notify_volume_change_with_delay(true);
fake_cras_audio_client()->disable_volume_change_events();
test_observer_->reset_output_volume_changed_count();

// Adjust the volume of output devices continuously.
Expand Down
4 changes: 2 additions & 2 deletions chromeos/ash/components/dbus/audio/fake_cras_audio_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ void FakeCrasAudioClient::GetSpeakOnMuteDetectionEnabled(

void FakeCrasAudioClient::SetOutputNodeVolume(uint64_t node_id,
int32_t volume) {
if (!notify_volume_change_with_delay_) {
if (enable_volume_change_events_) {
NotifyOutputNodeVolumeChangedForTesting(node_id, volume);
}
}
Expand All @@ -217,7 +217,7 @@ void FakeCrasAudioClient::SetOutputUserMute(bool mute_on) {

void FakeCrasAudioClient::SetInputNodeGain(uint64_t node_id,
int32_t input_gain) {
if (!notify_gain_change_with_delay_) {
if (enable_gain_change_events_) {
NotifyInputNodeGainChangedForTesting(node_id, input_gain);
}
}
Expand Down
23 changes: 11 additions & 12 deletions chromeos/ash/components/dbus/audio/fake_cras_audio_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,12 +134,15 @@ class COMPONENT_EXPORT(DBUS_AUDIO) FakeCrasAudioClient
const uint64_t& active_output_node_id() const {
return active_output_node_id_;
}
void set_notify_volume_change_with_delay(bool notify_with_delay) {
notify_volume_change_with_delay_ = notify_with_delay;
}
void set_notify_gain_change_with_delay(bool notify_with_delay) {
notify_gain_change_with_delay_ = notify_with_delay;
}

// By default the observers are informed when `SetOutputNodeVolume` is
// invoked. This disables that. You can then manually invoke the observers by
// calling `NotifyOutputNodeVolumeChangedForTesting`.
void disable_volume_change_events() { enable_volume_change_events_ = false; }
// By default the observers are informed when `SetInputGain` is
// invoked. This disables that. You can then manually invoke the observers by
// calling `NotifyInputNodeGainChangedForTesting`.
void disable_gain_change_events() { enable_gain_change_events_ = false; }

bool noise_cancellation_enabled() const {
return noise_cancellation_enabled_;
Expand All @@ -153,12 +156,8 @@ class COMPONENT_EXPORT(DBUS_AUDIO) FakeCrasAudioClient
AudioNodeList node_list_;
uint64_t active_input_node_id_ = 0;
uint64_t active_output_node_id_ = 0;
// By default, immediately sends OutputNodeVolumeChange signal following the
// SetOutputNodeVolume fake dbus call.
bool notify_volume_change_with_delay_ = false;
// By default, immediately sends InputNodeVolumeChange signal following the
// SetInputNodeGain fake dbus call.
bool notify_gain_change_with_delay_ = false;
bool enable_volume_change_events_ = true;
bool enable_gain_change_events_ = true;
bool noise_cancellation_supported_ = false;
uint32_t battery_level_ = 0;
uint32_t noise_cancellation_enabled_counter_ = 0;
Expand Down

0 comments on commit 6de20cc

Please sign in to comment.