Display current output device icon in the floating volume row when switching output devices

ash/system/audio/volume_view.cc

Index: ash/system/audio/volume_view.cc
diff --git a/ash/system/audio/volume_view.cc b/ash/system/audio/volume_view.cc
index 7fdf5711016736597436a6cb70f733c2be3d2804..4d18ab9badc775c6890cbb6ef004ed88998362a7 100644
--- a/ash/system/audio/volume_view.cc
+++ b/ash/system/audio/volume_view.cc
@@ -131,29 +131,6 @@ VolumeView::VolumeView(SystemTrayItem* owner,
   tri_view_->AddView(TriView::Container::CENTER, slider_);
 
   set_background(views::Background::CreateSolidBackground(kBackgroundColor));
-
-  if (!is_default_view_) {
-    tri_view_->SetContainerVisible(TriView::Container::END, false);
-    Update();
-    return;
-  }
-
-  more_button_ = new ButtonListenerActionableView(
-      owner_, TrayPopupInkDropStyle::INSET_BOUNDS, this);
-  TrayPopupUtils::ConfigureContainer(TriView::Container::END, more_button_);
-
-  more_button_->SetInkDropMode(views::InkDropHostView::InkDropMode::ON);
-  more_button_->SetBorder(views::CreateEmptyBorder(gfx::Insets(
-      0, kTrayPopupButtonEndMargin)));
-  tri_view_->AddView(TriView::Container::END, more_button_);
-
-  device_type_ = TrayPopupUtils::CreateMoreImageView();
-  device_type_->SetVisible(false);
-  more_button_->AddChildView(device_type_);
-
-  more_button_->AddChildView(TrayPopupUtils::CreateMoreImageView());
-  more_button_->SetAccessibleName(
-      l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_AUDIO));
   Update();
 }
 
@@ -186,19 +163,61 @@ void VolumeView::SetVolumeLevel(float percent) {
 
 void VolumeView::UpdateDeviceTypeAndMore() {
   CrasAudioHandler* audio_handler = CrasAudioHandler::Get();
-  bool show_more =
-      is_default_view_ && (audio_handler->has_alternative_output() ||
-                           audio_handler->has_alternative_input());
-
-  if (!show_more)
+  // There is no point in letting the user open the audio detailed submenu if
+  // there are no alternative output devices present, so do not show the 'more'

[tdanderson, 2017/04/18 23:50:29]

nit: "if there are no alternative input or output devices present"

+  // button in the default audio row.
+  if (is_default_view_ && !audio_handler->has_alternative_output() &&

[tdanderson, 2017/04/18 23:50:29]

nit: can you run 'git cl format ash' on this? I think the conditional should be
formatted as:

if (A &&
    B &&
    C) {
}

[minch1, 2017/04/19 00:01:41]

I run 'git cl format' before upload the cl. It still looks like this. I guess it
should because the first condition is too short?

[tdanderson, 2017/04/19 15:18:09]

Ah OK, if git cl format is fine with it, then I'm fine with it too :)

+      !audio_handler->has_alternative_input()) {
+    tri_view_->SetContainerVisible(TriView::Container::END, false);
+    // TODO(tdanderson): TriView itself should trigger a relayout when the
+    // visibility of one of its containers is changed.
+    tri_view_->InvalidateLayout();
     return;
+  }
 
   const gfx::VectorIcon& device_icon = GetActiveOutputDeviceVectorIcon();
-  const bool target_visibility = !device_icon.is_empty();
-  if (target_visibility)
+  const bool device_icon_visibility = !device_icon.is_empty();
+
+  if (is_default_view_) {
+    if (!more_button_) {
+      more_button_ = new ButtonListenerActionableView(
+          owner_, TrayPopupInkDropStyle::INSET_BOUNDS, this);
+      TrayPopupUtils::ConfigureContainer(TriView::Container::END, more_button_);
+
+      more_button_->SetInkDropMode(views::InkDropHostView::InkDropMode::ON);
+      more_button_->SetBorder(
+          views::CreateEmptyBorder(gfx::Insets(0, kTrayPopupButtonEndMargin)));
+      tri_view_->AddView(TriView::Container::END, more_button_);
+
+      device_type_ = TrayPopupUtils::CreateMoreImageView();
+      device_type_->SetVisible(false);
+      more_button_->AddChildView(device_type_);
+
+      more_button_->AddChildView(TrayPopupUtils::CreateMoreImageView());
+      more_button_->SetAccessibleName(
+          l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_AUDIO));
+    }
+    tri_view_->SetContainerVisible(TriView::Container::END, true);

[tdanderson, 2017/04/18 23:50:29]

Lines 200-201 and lines 213-214 are identical and appear at the end of both 'if'
and 'else' blocks. Can you move that logic to line 216 (outside of the if-else
blocks) instead?

+    tri_view_->InvalidateLayout();
+  } else {
+    if (!device_icon_visibility) {
+      tri_view_->SetContainerVisible(TriView::Container::END, false);
+      tri_view_->InvalidateLayout();
+      return;
+    }
+    if (!device_type_) {
+      device_type_ = TrayPopupUtils::CreateMoreImageView();
+      device_type_->SetVisible(false);
+      tri_view_->AddView(TriView::Container::END, device_type_);
+    }
+    tri_view_->SetContainerVisible(TriView::Container::END, true);
+    tri_view_->InvalidateLayout();
+  }
+
+  if (device_icon_visibility)
     device_type_->SetImage(gfx::CreateVectorIcon(device_icon, kMenuIconColor));
-  if (device_type_->visible() != target_visibility) {
-    device_type_->SetVisible(target_visibility);
+  if (device_type_->visible() != device_icon_visibility) {
+    device_type_->SetVisible(device_icon_visibility);
     device_type_->InvalidateLayout();
   }
 }