Commit 38dab6ee authored by Noah Rose Ledesma's avatar Noah Rose Ledesma Committed by Commit Bot

GMC: Fix a bug in hiding the device picker when there are no options

crrev.com/c/2363256 introduced filtering the default device out of the
list of audio devices sent to the UI. That change did not update the
logic regarding when to hide the UI however. This change adds that
missing functionality.

Bug: 1118193
Change-Id: I13ef0e3028eabcae621f8149bc7371fc5436420c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2373271
Commit-Queue: Noah Rose Ledesma <noahrose@google.com>
Reviewed-by: default avatarTommy Steimel <steimel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#801557}
parent 4c1a4237
...@@ -43,18 +43,18 @@ class AudioDeviceEntryView : public views::Button { ...@@ -43,18 +43,18 @@ class AudioDeviceEntryView : public views::Button {
const std::string& subtext = ""); const std::string& subtext = "");
const std::string& GetDeviceId() { return raw_device_id_; } const std::string& GetDeviceId() { return raw_device_id_; }
const std::string& GetDeviceName() { return device_name_; }
void SetHighlighted(bool highlighted); void SetHighlighted(bool highlighted);
void OnColorsChanged(const SkColor& foreground_color, void OnColorsChanged(const SkColor& foreground_color,
const SkColor& background_color); const SkColor& background_color);
std::string get_label_for_testing();
bool is_highlighted_for_testing() { return is_highlighted_; } bool is_highlighted_for_testing() { return is_highlighted_; }
protected: protected:
SkColor foreground_color_, background_color_; SkColor foreground_color_, background_color_;
const std::string raw_device_id_; const std::string raw_device_id_, device_name_;
bool is_highlighted_ = false; bool is_highlighted_ = false;
views::View* icon_container_; views::View* icon_container_;
views::ImageView* device_icon_; views::ImageView* device_icon_;
...@@ -72,7 +72,8 @@ AudioDeviceEntryView::AudioDeviceEntryView(const SkColor& foreground_color, ...@@ -72,7 +72,8 @@ AudioDeviceEntryView::AudioDeviceEntryView(const SkColor& foreground_color,
const std::string& subtext) const std::string& subtext)
: foreground_color_(foreground_color), : foreground_color_(foreground_color),
background_color_(background_color), background_color_(background_color),
raw_device_id_(raw_device_id) { raw_device_id_(raw_device_id),
device_name_(name) {
SetLayoutManager(std::make_unique<views::BoxLayout>( SetLayoutManager(std::make_unique<views::BoxLayout>(
views::BoxLayout::Orientation::kHorizontal)); views::BoxLayout::Orientation::kHorizontal));
...@@ -102,7 +103,7 @@ AudioDeviceEntryView::AudioDeviceEntryView(const SkColor& foreground_color, ...@@ -102,7 +103,7 @@ AudioDeviceEntryView::AudioDeviceEntryView(const SkColor& foreground_color,
views::Label::GetDefaultFontList().DeriveWithSizeDelta(1)}; views::Label::GetDefaultFontList().DeriveWithSizeDelta(1)};
device_name_label_ = device_name_label_ =
labels_container_->AddChildView(std::make_unique<views::Label>( labels_container_->AddChildView(std::make_unique<views::Label>(
base::UTF8ToUTF16(name), device_name_label_font)); base::UTF8ToUTF16(device_name_), device_name_label_font));
device_name_label_->SetEnabledColor(foreground_color); device_name_label_->SetEnabledColor(foreground_color);
device_name_label_->SetBackgroundColor(background_color); device_name_label_->SetBackgroundColor(background_color);
...@@ -161,10 +162,6 @@ void AudioDeviceEntryView::OnColorsChanged(const SkColor& foreground_color, ...@@ -161,10 +162,6 @@ void AudioDeviceEntryView::OnColorsChanged(const SkColor& foreground_color,
SetHighlighted(is_highlighted_); SetHighlighted(is_highlighted_);
} }
std::string AudioDeviceEntryView::get_label_for_testing() {
return base::UTF16ToUTF8(device_name_label_->GetText());
}
MediaNotificationAudioDeviceSelectorView:: MediaNotificationAudioDeviceSelectorView::
MediaNotificationAudioDeviceSelectorView( MediaNotificationAudioDeviceSelectorView(
MediaNotificationAudioDeviceSelectorViewDelegate* delegate, MediaNotificationAudioDeviceSelectorViewDelegate* delegate,
...@@ -259,10 +256,6 @@ MediaNotificationAudioDeviceSelectorView:: ...@@ -259,10 +256,6 @@ MediaNotificationAudioDeviceSelectorView::
void MediaNotificationAudioDeviceSelectorView::UpdateAvailableAudioDevices( void MediaNotificationAudioDeviceSelectorView::UpdateAvailableAudioDevices(
const media::AudioDeviceDescriptions& device_descriptions) { const media::AudioDeviceDescriptions& device_descriptions) {
bool is_visible = ShouldBeVisible(device_descriptions);
SetVisible(is_visible);
delegate_->OnAudioDeviceSelectorViewSizeChanged();
audio_device_entries_container_->RemoveAllChildViews(true); audio_device_entries_container_->RemoveAllChildViews(true);
current_device_entry_view_ = nullptr; current_device_entry_view_ = nullptr;
...@@ -283,6 +276,9 @@ void MediaNotificationAudioDeviceSelectorView::UpdateAvailableAudioDevices( ...@@ -283,6 +276,9 @@ void MediaNotificationAudioDeviceSelectorView::UpdateAvailableAudioDevices(
current_device_still_exists current_device_still_exists
? current_device_id_ ? current_device_id_
: media::AudioDeviceDescription::kDefaultDeviceId); : media::AudioDeviceDescription::kDefaultDeviceId);
SetVisible(ShouldBeVisible(device_descriptions));
delegate_->OnAudioDeviceSelectorViewSizeChanged();
} }
void MediaNotificationAudioDeviceSelectorView::OnColorsChanged( void MediaNotificationAudioDeviceSelectorView::OnColorsChanged(
...@@ -327,8 +323,7 @@ void MediaNotificationAudioDeviceSelectorView::ButtonPressed( ...@@ -327,8 +323,7 @@ void MediaNotificationAudioDeviceSelectorView::ButtonPressed(
std::string std::string
MediaNotificationAudioDeviceSelectorView::get_entry_label_for_testing( MediaNotificationAudioDeviceSelectorView::get_entry_label_for_testing(
views::View* entry_view) { views::View* entry_view) {
return static_cast<AudioDeviceEntryView*>(entry_view) return static_cast<AudioDeviceEntryView*>(entry_view)->GetDeviceName();
->get_label_for_testing();
} }
// static // static
...@@ -356,5 +351,20 @@ void MediaNotificationAudioDeviceSelectorView::HideDevices() { ...@@ -356,5 +351,20 @@ void MediaNotificationAudioDeviceSelectorView::HideDevices() {
bool MediaNotificationAudioDeviceSelectorView::ShouldBeVisible( bool MediaNotificationAudioDeviceSelectorView::ShouldBeVisible(
const media::AudioDeviceDescriptions& device_descriptions) { const media::AudioDeviceDescriptions& device_descriptions) {
// The UI should be visible if there are more than one unique devices. That is
// when:
// * There are at least three devices
// * Or, there are two devices and one of them has the default ID but not the
// default name.
if (audio_device_entries_container_->children().size() == 2) {
return util::ranges::any_of(
audio_device_entries_container_->children(), [](views::View* view) {
auto* entry = static_cast<AudioDeviceEntryView*>(view);
return entry->GetDeviceId() ==
media::AudioDeviceDescription::kDefaultDeviceId &&
entry->GetDeviceName() !=
media::AudioDeviceDescription::GetDefaultDeviceName();
});
}
return device_descriptions.size() > 2; return device_descriptions.size() > 2;
} }
...@@ -265,7 +265,7 @@ TEST_F(MediaNotificationAudioDeviceSelectorViewTest, VisibilityChanges) { ...@@ -265,7 +265,7 @@ TEST_F(MediaNotificationAudioDeviceSelectorViewTest, VisibilityChanges) {
// The audio device selector view should become hidden when there is only one // The audio device selector view should become hidden when there is only one
// unique device. // unique device.
provider_->AddDevice("Speaker", "1"); provider_->AddDevice("Speaker", "1");
provider_->AddDevice("default", provider_->AddDevice(media::AudioDeviceDescription::GetDefaultDeviceName(),
media::AudioDeviceDescription::kDefaultDeviceId); media::AudioDeviceDescription::kDefaultDeviceId);
auto* provider = provider_.get(); auto* provider = provider_.get();
service_->set_device_provider_for_testing(std::move(provider_)); service_->set_device_provider_for_testing(std::move(provider_));
...@@ -279,7 +279,20 @@ TEST_F(MediaNotificationAudioDeviceSelectorViewTest, VisibilityChanges) { ...@@ -279,7 +279,20 @@ TEST_F(MediaNotificationAudioDeviceSelectorViewTest, VisibilityChanges) {
testing::Mock::VerifyAndClearExpectations(&delegate); testing::Mock::VerifyAndClearExpectations(&delegate);
provider->ResetDevices();
provider->AddDevice("Speaker", "1");
provider->AddDevice("Headphones",
media::AudioDeviceDescription::kDefaultDeviceId);
EXPECT_CALL(delegate, OnAudioDeviceSelectorViewSizeChanged).Times(1);
provider->RunUICallback();
EXPECT_TRUE(view_->GetVisible());
testing::Mock::VerifyAndClearExpectations(&delegate);
provider->ResetDevices();
provider->AddDevice("Speaker", "1");
provider->AddDevice("Headphones", "2"); provider->AddDevice("Headphones", "2");
provider->AddDevice(media::AudioDeviceDescription::GetDefaultDeviceName(),
media::AudioDeviceDescription::kDefaultDeviceId);
EXPECT_CALL(delegate, OnAudioDeviceSelectorViewSizeChanged).Times(1); EXPECT_CALL(delegate, OnAudioDeviceSelectorViewSizeChanged).Times(1);
provider->RunUICallback(); provider->RunUICallback();
EXPECT_TRUE(view_->GetVisible()); EXPECT_TRUE(view_->GetVisible());
......
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