Remove non-MD code from Bluetooth tray.

ash/common/system/chromeos/bluetooth/tray_bluetooth.cc

Index: ash/common/system/chromeos/bluetooth/tray_bluetooth.cc
diff --git a/ash/common/system/chromeos/bluetooth/tray_bluetooth.cc b/ash/common/system/chromeos/bluetooth/tray_bluetooth.cc
index 559b56328120e9d41f916e699ad701fe62128960..21e428607a66850d7c3811bdd358e7ba22fd1b4f 100644
--- a/ash/common/system/chromeos/bluetooth/tray_bluetooth.cc
+++ b/ash/common/system/chromeos/bluetooth/tray_bluetooth.cc
@@ -4,7 +4,6 @@
 
 #include "ash/common/system/chromeos/bluetooth/tray_bluetooth.h"
 
-#include "ash/common/material_design/material_design_controller.h"
 #include "ash/common/session/session_state_delegate.h"
 #include "ash/common/system/tray/hover_highlight_view.h"
 #include "ash/common/system/tray/system_tray.h"
@@ -41,10 +40,6 @@ namespace ash {
 namespace tray {
 namespace {
 
-bool UseMd() {
-  return MaterialDesignController::IsSystemTrayMenuMaterial();
-}
-
 // Updates bluetooth device |device| in the |list|. If it is new, append to the
 // end of the |list|; otherwise, keep it at the same place, but update the data
 // with new device info provided by |device|.
@@ -119,14 +114,7 @@ const int kDisabledPanelLabelBaselineY = 20;
 class BluetoothDefaultView : public TrayItemMore {
  public:
   BluetoothDefaultView(SystemTrayItem* owner, bool show_more)
-      : TrayItemMore(owner, show_more) {
-    if (!UseMd()) {
-      // The icon doesn't change in non-md.
-      ui::ResourceBundle& bundle = ui::ResourceBundle::GetSharedInstance();
-      SetImage(
-          *bundle.GetImageNamed(IDR_AURA_UBER_TRAY_BLUETOOTH).ToImageSkia());
-    }
-  }
+      : TrayItemMore(owner, show_more) {}
 
   ~BluetoothDefaultView() override {}
 
@@ -164,10 +152,6 @@ class BluetoothDefaultView : public TrayItemMore {
 
   void UpdateStyle() override {
     TrayItemMore::UpdateStyle();
-
-    if (!UseMd())
-      return;
-
     std::unique_ptr<TrayPopupItemStyle> style = CreateStyle();
     SetImage(gfx::CreateVectorIcon(GetCurrentIcon(), style->GetIconColor()));
   }
@@ -199,9 +183,6 @@ class BluetoothDetailedView : public TrayDetailsView {
   BluetoothDetailedView(SystemTrayItem* owner, LoginStatus login)
       : TrayDetailsView(owner),
         login_(login),
-        manage_devices_(nullptr),
-        throbber_(nullptr),
-        toggle_bluetooth_(nullptr),
         enable_bluetooth_(nullptr),
         toggle_(nullptr),
         settings_(nullptr),
@@ -227,7 +208,6 @@ class BluetoothDetailedView : public TrayDetailsView {
  private:
   void CreateItems() {
     CreateScrollableList();
-    AppendSettingsEntries();
     CreateTitleRow(IDS_ASH_STATUS_TRAY_BLUETOOTH);
   }
 
@@ -260,10 +240,6 @@ class BluetoothDetailedView : public TrayDetailsView {
     WmShell::Get()->system_tray_delegate()->GetAvailableBluetoothDevices(&list);
     for (size_t i = 0; i < list.size(); ++i) {
       if (list[i].connecting) {
-        if (!UseMd()) {
-          list[i].display_name = l10n_util::GetStringFUTF16(
-              IDS_ASH_STATUS_TRAY_BLUETOOTH_CONNECTING, list[i].display_name);

[tdanderson, 2017/02/10 15:51:01]

I think you can remove this string resource too since it doesn't look to be used
in MD

[fukino, 2017/02/11 00:25:01]

You are right. Thank you for pointing this out!
Done.

-        }
         new_connecting_devices.insert(list[i].address);
         UpdateBluetoothDeviceListHelper(&connecting_devices_, list[i]);
       } else if (list[i].connected && list[i].paired) {
@@ -294,8 +270,6 @@ class BluetoothDetailedView : public TrayDetailsView {
         WmShell::Get()->system_tray_delegate()->GetBluetoothEnabled();
     if (toggle_)
       toggle_->SetIsOn(is_bluetooth_enabled, false);
-    else if (toggle_bluetooth_)
-      toggle_bluetooth_->SetToggled(!is_bluetooth_enabled);
   }
 
   void UpdateDeviceScrollList() {
@@ -306,21 +280,14 @@ class BluetoothDetailedView : public TrayDetailsView {
     SystemTrayDelegate* delegate = WmShell::Get()->system_tray_delegate();
     bool bluetooth_enabled = delegate->GetBluetoothEnabled();
     bool bluetooth_available = delegate->GetBluetoothAvailable();
-    if (bluetooth_available && !bluetooth_enabled && toggle_bluetooth_) {
-      enable_bluetooth_ = AddScrollListItem(
-          l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_ENABLE_BLUETOOTH),

[tdanderson, 2017/02/10 15:51:01]

This string also looks like it's no longer used

[fukino, 2017/02/11 00:25:01]

DONE.
I removed IDS_ASH_STATUS_TRAY_DISABLE_BLUETOOTH and
IDS_ASH_STATUS_TRAY_BLUETOOTH_MANAGE_DEVICES as well.

[tdanderson, 2017/02/13 17:14:59]

Great, thanks for catching those other two strings.

[fukino, 2017/02/13 19:56:17]

I should have noticed earlier. Thank you for catching them!

-          false /* highlight */, false /* checked */, true /* enabled */);
-    }
 
     // If Bluetooth is disabled, show a panel which only indicates that it is
     // disabled, instead of the scroller with Bluetooth devices.
-    if (UseMd()) {
-      if (bluetooth_enabled) {
-        HideDisabledPanel();
-      } else {
-        ShowDisabledPanel();
-        return;
-      }
+    if (bluetooth_enabled) {
+      HideDisabledPanel();
+    } else {
+      ShowDisabledPanel();
+      return;
     }
 
     // Add paired devices (and their section header in MD) in the list.
@@ -328,8 +295,7 @@ class BluetoothDetailedView : public TrayDetailsView {
                                 connecting_devices_.size() +
                                 paired_not_connected_devices_.size();
     if (num_paired_devices > 0) {
-      if (UseMd())
-        AddSubHeader(IDS_ASH_STATUS_TRAY_BLUETOOTH_PAIRED_DEVICES);
+      AddSubHeader(IDS_ASH_STATUS_TRAY_BLUETOOTH_PAIRED_DEVICES);
       AppendSameTypeDevicesToScrollList(connected_devices_, true, true,
                                         bluetooth_enabled);
       AppendSameTypeDevicesToScrollList(connecting_devices_, true, false,
@@ -340,12 +306,8 @@ class BluetoothDetailedView : public TrayDetailsView {
 
     // Add paired devices (and their section header in MD) in the list.
     if (discovered_not_paired_devices_.size() > 0) {
-      if (UseMd()) {
-        if (num_paired_devices > 0)
-          AddSubHeader(IDS_ASH_STATUS_TRAY_BLUETOOTH_UNPAIRED_DEVICES);
-      } else {
-        AddScrollSeparator();
-      }
+      if (num_paired_devices > 0)
+        AddSubHeader(IDS_ASH_STATUS_TRAY_BLUETOOTH_UNPAIRED_DEVICES);
       AppendSameTypeDevicesToScrollList(discovered_not_paired_devices_, false,
                                         false, bluetooth_enabled);
     }
@@ -353,10 +315,11 @@ class BluetoothDetailedView : public TrayDetailsView {
     // Show user Bluetooth state if there is no bluetooth devices in list.
     if (device_map_.size() == 0) {
       if (bluetooth_available && bluetooth_enabled) {
-        AddScrollListItem(l10n_util::GetStringUTF16(
-                              IDS_ASH_STATUS_TRAY_BLUETOOTH_DISCOVERING),
-                          false /* highlight */, false /* checked */,
-                          true /* enabled */);
+        HoverHighlightView* container = new HoverHighlightView(this);

[tdanderson, 2017/02/10 15:51:01]

If I understand correctly, is this just displaying a simple "Scanning for
devices..." message at the top of the detailed view? Is there any way you can
grab a screenshot of what this looks like?

We should maybe be displaying something nicer for MD, with the message centered
along with an icon of some sort (similar to what we do for the "Bluetooth
disabled" case. But that can be done in a separate patch set. Can you please
file a bug and attach what this looks like currently? We can then discuss with
Sebastien what should happen.

[fukino, 2017/02/11 00:25:01]

I filed crbug.com/691138 and attached a screenshot on it.
Thank you for the suggestion.

[tdanderson, 2017/02/13 17:14:59]

Thank you!

+        container->AddLabel(l10n_util::GetStringUTF16(
+                                IDS_ASH_STATUS_TRAY_BLUETOOTH_DISCOVERING),
+                            gfx::ALIGN_LEFT, false);
+        scroll_content()->AddChildView(container);
       }
     }
 
@@ -369,45 +332,24 @@ class BluetoothDetailedView : public TrayDetailsView {
                                          bool enabled) {
     for (size_t i = 0; i < list.size(); ++i) {
       HoverHighlightView* container = nullptr;
-      if (UseMd()) {
-        gfx::ImageSkia icon_image = CreateVectorIcon(
-            GetBluetoothDeviceIcon(list[i].device_type, list[i].connected),
-            kMenuIconColor);
-        container = AddScrollListItemMd(list[i].display_name, icon_image,
-                                        list[i].connected, list[i].connecting);
-      } else {
-        container = AddScrollListItem(list[i].display_name, highlight, checked,
-                                      enabled);
-      }
+      gfx::ImageSkia icon_image = CreateVectorIcon(
+          GetBluetoothDeviceIcon(list[i].device_type, list[i].connected),
+          kMenuIconColor);
+      container = AddScrollListItem(list[i].display_name, icon_image,
+                                    list[i].connected, list[i].connecting);
       device_map_[container] = list[i].address;
     }
   }
 
-  // TODO(fukino): Remove this code when material design is enabled by default,
-  // since AddScrollListItem should be used only in the old design.
-  // See crbug.com/614453".
   HoverHighlightView* AddScrollListItem(const base::string16& text,
-                                        bool highlight,
-                                        bool checked,
-                                        bool enabled) {
-    HoverHighlightView* container = new HoverHighlightView(this);
-    views::Label* label =
-        container->AddCheckableLabel(text, highlight, checked);
-    label->SetEnabled(enabled);
-    scroll_content()->AddChildView(container);
-    return container;
-  }
-
-  HoverHighlightView* AddScrollListItemMd(const base::string16& text,
-                                          const gfx::ImageSkia& image,
-                                          bool connected,
-                                          bool connecting) {
-    DCHECK(UseMd());
+                                        const gfx::ImageSkia& image,
+                                        bool connected,
+                                        bool connecting) {
     HoverHighlightView* container = new HoverHighlightView(this);
     if (connected) {
-      SetupConnectedItemMd(container, text, image);
+      SetupConnectedItem(container, text, image);
     } else if (connecting) {
-      SetupConnectingItemMd(container, text, image);
+      SetupConnectingItem(container, text, image);
     } else {
       container->AddIconAndLabel(image, text, false);
     }
@@ -428,9 +370,9 @@ class BluetoothDetailedView : public TrayDetailsView {
     scroll_content()->AddChildView(header);
   }
 
-  void SetupConnectedItemMd(HoverHighlightView* container,
-                            const base::string16& text,
-                            const gfx::ImageSkia& image) {
+  void SetupConnectedItem(HoverHighlightView* container,
+                          const base::string16& text,
+                          const gfx::ImageSkia& image) {
     container->AddIconAndLabels(
         image, text, l10n_util::GetStringUTF16(
                          IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTED));
@@ -439,9 +381,9 @@ class BluetoothDetailedView : public TrayDetailsView {
     style.SetupLabel(container->sub_text_label());
   }
 
-  void SetupConnectingItemMd(HoverHighlightView* container,
-                             const base::string16& text,
-                             const gfx::ImageSkia& image) {
+  void SetupConnectingItem(HoverHighlightView* container,
+                           const base::string16& text,
+                           const gfx::ImageSkia& image) {
     container->AddIconAndLabels(
         image, text, l10n_util::GetStringUTF16(
                          IDS_ASH_STATUS_TRAY_NETWORK_STATUS_CONNECTING));
@@ -450,32 +392,6 @@ class BluetoothDetailedView : public TrayDetailsView {
     container->AddRightView(throbber);
   }
 
-  // Add settings entries.
-  void AppendSettingsEntries() {
-    // Do not append the bottom button row in material design; this is replaced
-    // by the settings button in the header row.
-    if (UseMd())
-      return;
-
-    if (!WmShell::Get()->system_tray_delegate()->ShouldShowSettings())
-      return;
-
-    // Add bluetooth device requires a browser window, hide it for non logged in
-    // user.
-    if (!TrayPopupUtils::CanOpenWebUISettings(login_))
-      return;
-
-    SystemTrayDelegate* delegate = WmShell::Get()->system_tray_delegate();
-    ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
-    HoverHighlightView* container = new HoverHighlightView(this);
-    container->AddLabel(
-        rb.GetLocalizedString(IDS_ASH_STATUS_TRAY_BLUETOOTH_MANAGE_DEVICES),
-        gfx::ALIGN_LEFT, false /* highlight */);
-    container->SetEnabled(delegate->GetBluetoothAvailable());
-    AddChildView(container);
-    manage_devices_ = container;
-  }
-
   // Returns true if the device with |device_id| is found in |device_list|,
   // and the display_name of the device will be returned in |display_name| if
   // it's not NULL.
@@ -506,17 +422,11 @@ class BluetoothDetailedView : public TrayDetailsView {
       item_container->RemoveAllChildViews(true);
       HoverHighlightView* container =
           static_cast<HoverHighlightView*>(item_container);
-      if (UseMd()) {
-        TrayPopupItemStyle style(
-            TrayPopupItemStyle::FontStyle::DETAILED_VIEW_LABEL);
-        gfx::ImageSkia icon_image = CreateVectorIcon(
-            GetBluetoothDeviceIcon(device_type, false), style.GetIconColor());
-        SetupConnectingItemMd(container, display_name, icon_image);
-      } else {
-        display_name = l10n_util::GetStringFUTF16(
-            IDS_ASH_STATUS_TRAY_BLUETOOTH_CONNECTING, display_name);
-        container->AddCheckableLabel(display_name, true /* highlight */, false);
-      }
+      TrayPopupItemStyle style(
+          TrayPopupItemStyle::FontStyle::DETAILED_VIEW_LABEL);
+      gfx::ImageSkia icon_image = CreateVectorIcon(
+          GetBluetoothDeviceIcon(device_type, false), style.GetIconColor());
+      SetupConnectingItem(container, display_name, icon_image);
       scroll_content()->SizeToPreferredSize();
       scroller()->Layout();
     }
@@ -524,11 +434,6 @@ class BluetoothDetailedView : public TrayDetailsView {
 
   // TrayDetailsView:
   void HandleViewClicked(views::View* view) override {
-    if (view == manage_devices_) {
-      ShowSettings();
-      return;
-    }
-
     SystemTrayDelegate* delegate = WmShell::Get()->system_tray_delegate();
     if (view == enable_bluetooth_) {

[tdanderson, 2017/02/10 15:51:01]

On line 310 you are removing the instantiation of |enable_bluetooth_|. I think
this member was only relevant to non-MD and can now be removed, can you please
double check? Note the metrics collection below should also probably be
collected when the MD toggle is clicked.

[fukino, 2017/02/11 00:25:01]

Removed |enable_bluetooth_| and its related code.
As for the metrics, I filed crbug.com/691152 to make the issue clear.

[tdanderson, 2017/02/13 17:14:59]

Acknowledged, thanks.

       WmShell::Get()->RecordUserMetricsAction(
@@ -556,62 +461,29 @@ class BluetoothDetailedView : public TrayDetailsView {
 
   void HandleButtonPressed(views::Button* sender,
                            const ui::Event& event) override {
-    if (UseMd()) {
-      if (sender == toggle_)
-        WmShell::Get()->system_tray_delegate()->ToggleBluetooth();
-      else if (sender == settings_)
-        ShowSettings();
-      else
-        NOTREACHED();
-      return;
-    }
+    if (sender == toggle_)
+      WmShell::Get()->system_tray_delegate()->ToggleBluetooth();
+    else if (sender == settings_)
+      ShowSettings();
 
-    SystemTrayDelegate* delegate = WmShell::Get()->system_tray_delegate();
-    if (sender == toggle_bluetooth_)
-      delegate->ToggleBluetooth();
-    else
-      NOTREACHED();
+    NOTREACHED();
   }
 
   void CreateExtraTitleRowButtons() override {
     if (login_ == LoginStatus::LOCKED)
       return;
 
-    if (UseMd()) {
-      DCHECK(!toggle_);
-      DCHECK(!settings_);
+    DCHECK(!toggle_);
+    DCHECK(!settings_);
 
-      tri_view()->SetContainerVisible(TriView::Container::END, true);
+    tri_view()->SetContainerVisible(TriView::Container::END, true);
 
-      toggle_ = TrayPopupUtils::CreateToggleButton(
-          this, IDS_ASH_STATUS_TRAY_BLUETOOTH);
-      tri_view()->AddView(TriView::Container::END, toggle_);
+    toggle_ =
+        TrayPopupUtils::CreateToggleButton(this, IDS_ASH_STATUS_TRAY_BLUETOOTH);
+    tri_view()->AddView(TriView::Container::END, toggle_);
 
-      settings_ = CreateSettingsButton(login_);
-      tri_view()->AddView(TriView::Container::END, settings_);
-      return;
-    }
-
-    throbber_ = new ThrobberView;
-    throbber_->SetTooltipText(
-        l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_BLUETOOTH_DISCOVERING));
-    title_row()->AddViewToRowNonMd(throbber_, false);
-
-    // Do not allow toggling bluetooth in the lock screen.
-    SystemTrayDelegate* delegate = WmShell::Get()->system_tray_delegate();
-    toggle_bluetooth_ =
-        new TrayPopupHeaderButton(this, IDR_AURA_UBER_TRAY_BLUETOOTH_ENABLED,
-                                  IDR_AURA_UBER_TRAY_BLUETOOTH_DISABLED,
-                                  IDR_AURA_UBER_TRAY_BLUETOOTH_ENABLED_HOVER,
-                                  IDR_AURA_UBER_TRAY_BLUETOOTH_DISABLED_HOVER,
-                                  IDS_ASH_STATUS_TRAY_BLUETOOTH);
-    toggle_bluetooth_->SetToggled(!delegate->GetBluetoothEnabled());
-    toggle_bluetooth_->SetTooltipText(
-        l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_DISABLE_BLUETOOTH));
-    toggle_bluetooth_->SetToggledTooltipText(
-        l10n_util::GetStringUTF16(IDS_ASH_STATUS_TRAY_ENABLE_BLUETOOTH));
-    toggle_bluetooth_->EnableCanvasFlippingForRTLUI(false);
-    title_row()->AddViewToRowNonMd(toggle_bluetooth_, true);
+    settings_ = CreateSettingsButton(login_);
+    tri_view()->AddView(TriView::Container::END, settings_);
   }
 
   void ShowSettings() {
@@ -622,23 +494,13 @@ class BluetoothDetailedView : public TrayDetailsView {
   }
 
   void ShowLoadingIndicator() {
-    if (throbber_) {
-      throbber_->Start();
-    } else if (UseMd()) {
-      // Setting a value of -1 gives progress_bar an infinite-loading behavior.
-      ShowProgress(-1, true);
-    }
+    // Setting a value of -1 gives progress_bar an infinite-loading behavior.
+    ShowProgress(-1, true);
   }
 
-  void HideLoadingIndicator() {
-    if (throbber_)
-      throbber_->Stop();
-    else if (UseMd())
-      ShowProgress(0, false);
-  }
+  void HideLoadingIndicator() { ShowProgress(0, false); }

[tdanderson, 2017/02/10 15:51:01]

Please format as

void ...() {
  ...
}

[fukino, 2017/02/11 00:25:01]

Done, but I had to ignore presubmit warning "Please run git cl format ash" to
upload the patch.
Can I ignore it?

[tdanderson, 2017/02/13 17:14:59]

Oh, that's strange. In this case just go with what 'git cl format ash' says.

[fukino, 2017/02/13 19:56:18]

Got it. I did git cl format for PS3.

 
   void ShowDisabledPanel() {
-    DCHECK(UseMd());
     DCHECK(scroller());
     if (!disabled_panel_) {
       disabled_panel_ = CreateDisabledPanel();
@@ -654,7 +516,6 @@ class BluetoothDetailedView : public TrayDetailsView {
   }
 
   void HideDisabledPanel() {
-    DCHECK(UseMd());
     DCHECK(scroller());
     if (disabled_panel_)
       disabled_panel_->SetVisible(false);
@@ -698,28 +559,15 @@ class BluetoothDetailedView : public TrayDetailsView {
 
   std::map<views::View*, std::string> device_map_;
 
-  // Not used in material design.
-  views::View* manage_devices_;
-
-  // Not used in material design.
-  ThrobberView* throbber_;
-
-  // Not used in material design.
-  TrayPopupHeaderButton* toggle_bluetooth_;
-
   HoverHighlightView* enable_bluetooth_;
   BluetoothDeviceList connected_devices_;
   BluetoothDeviceList connecting_devices_;
   BluetoothDeviceList paired_not_connected_devices_;
   BluetoothDeviceList discovered_not_paired_devices_;
 
-  // The on/off toggle button used in material design.
   views::ToggleButton* toggle_;
-
-  // Only used in material design.
   views::Button* settings_;
 
-  // Only used in material design.
   // The container of the message "Bluetooth is disabled" and an icon. It should
   // be shown instead of Bluetooth device list when Bluetooth is disabled.
   views::View* disabled_panel_;