Make TrayDetailsView a ViewClickListener and ButtonListener

ash/common/system/chromeos/ime_menu/ime_list_view.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "ash/common/system/chromeos/ime_menu/ime_list_view.h"
 
 #include "ash/common/system/tray/hover_highlight_view.h"
 #include "ash/common/system/tray/ime_info.h"
 #include "ash/common/system/tray/system_tray_delegate.h"
 #include "ash/common/system/tray/tray_constants.h"
@@ skipping 111 matching lines @@
   HoverHighlightView* container = new HoverHighlightView(this);
   int id = keyboard::IsKeyboardEnabled() ? IDS_ASH_STATUS_TRAY_DISABLE_KEYBOARD
                                          : IDS_ASH_STATUS_TRAY_ENABLE_KEYBOARD;
   container->AddLabel(
       ui::ResourceBundle::GetSharedInstance().GetLocalizedString(id),
       gfx::ALIGN_LEFT, false /* highlight */);
   scroll_content()->AddChildView(container);
   keyboard_status_ = container;
 }
 
-void ImeListView::OnViewClicked(views::View* sender) {
+void ImeListView::HandleViewClicked(views::View* view) {
+  if (view == keyboard_status_) {
+    WmShell::Get()->ToggleIgnoreExternalKeyboard();
+    return;
+  }
+
   SystemTrayDelegate* delegate = WmShell::Get()->system_tray_delegate();
-  if (sender == keyboard_status_) {
-    WmShell::Get()->ToggleIgnoreExternalKeyboard();
+  std::map<views::View*, std::string>::const_iterator ime_find;

[varkha, 2016/08/31 20:28:14]

Merge those two lines? Here and below for prop_find.
I would also s/ime_find/ime and s/prop_find/property.

[tdanderson, 2016/08/31 22:24:15]

Done.

+  ime_find = ime_map_.find(view);
+  if (ime_find != ime_map_.end()) {
+    WmShell::Get()->RecordUserMetricsAction(UMA_STATUS_AREA_IME_SWITCH_MODE);
+    std::string ime_id = ime_find->second;
+    delegate->SwitchIME(ime_id);
+    GetWidget()->Close();

[varkha, 2016/08/31 20:28:14]

nit: This could be taken out of {} and done at the end of this method body if
you add and else and an early return in line 153.

[tdanderson, 2016/08/31 22:24:15]

Done, but in a slightly different manner than suggested.

   } else {
-    std::map<views::View*, std::string>::const_iterator ime_find;
-    ime_find = ime_map_.find(sender);
-    if (ime_find != ime_map_.end()) {
-      WmShell::Get()->RecordUserMetricsAction(UMA_STATUS_AREA_IME_SWITCH_MODE);
-      std::string ime_id = ime_find->second;
-      delegate->SwitchIME(ime_id);
+    std::map<views::View*, std::string>::const_iterator prop_find;
+    prop_find = property_map_.find(view);
+    if (prop_find != property_map_.end()) {
+      const std::string key = prop_find->second;
+      delegate->ActivateIMEProperty(key);
       GetWidget()->Close();
-    } else {
-      std::map<views::View*, std::string>::const_iterator prop_find;
-      prop_find = property_map_.find(sender);
-      if (prop_find != property_map_.end()) {
-        const std::string key = prop_find->second;
-        delegate->ActivateIMEProperty(key);
-        GetWidget()->Close();
-      }
     }
   }
 }
 
 }  // namespace ash