Combobox: Rename styles to STYLE_NORMAL and STYLE_ACTION and modify behaviors

ui/views/controls/combobox/combobox.cc

 // Copyright (c) 2012 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 "ui/views/controls/combobox/combobox.h"
 
+#include "base/bind.h"
 #include "base/logging.h"
+#include "base/message_loop/message_loop_proxy.h"
 #include "base/strings/utf_string_conversions.h"
 #include "grit/ui_resources.h"
 #include "ui/base/accessibility/accessible_view_state.h"
 #include "ui/base/models/combobox_model.h"
 #include "ui/base/resource/resource_bundle.h"
 #include "ui/events/event.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 #include "ui/gfx/animation/throb_animation.h"
 #include "ui/gfx/canvas.h"
 #include "ui/gfx/image/image.h"
@@ skipping 194 matching lines @@
 }  // namespace
 
 // static
 const char Combobox::kViewClassName[] = "views/Combobox";
 
 ////////////////////////////////////////////////////////////////////////////////
 // Combobox, public:
 
 Combobox::Combobox(ui::ComboboxModel* model)
     : model_(model),
-      style_(STYLE_SHOW_DROP_DOWN_ON_CLICK),
+      style_(STYLE_NORMAL),
       listener_(NULL),
       selected_index_(model_->GetDefaultIndex()),
       invalid_(false),
       disclosure_arrow_(ui::ResourceBundle::GetSharedInstance().GetImageNamed(
           IDR_MENU_DROPARROW).ToImageSkia()),
       dropdown_open_(false),
       text_button_(new TransparentButton(this)),
-      arrow_button_(new TransparentButton(this)) {
+      arrow_button_(new TransparentButton(this)),
+      weak_ptr_factory_(this) {
   model_->AddObserver(this);
   UpdateFromModel();
   SetFocusable(true);
   UpdateBorder();
 
   // Initialize the button images.
   Button::ButtonState button_states[] = {
     Button::STATE_DISABLED,
     Button::STATE_NORMAL,
     Button::STATE_HOVERED,
@@ skipping 28 matching lines @@
 const gfx::FontList& Combobox::GetFontList() {
   ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
   return rb.GetFontList(ui::ResourceBundle::BaseFont);
 }
 
 void Combobox::SetStyle(Style style) {
   if (style_ == style)
     return;
 
   style_ = style;
+  if (style_ == STYLE_ACTION)
+    selected_index_ = 0;
 
   UpdateBorder();
+  UpdateFromModel();
   PreferredSizeChanged();
 }
 
 void Combobox::ModelChanged() {
   selected_index_ = std::min(0, model_->GetItemCount());
   UpdateFromModel();
   PreferredSizeChanged();
 }
 
 void Combobox::SetSelectedIndex(int index) {
+  if (style_ == STYLE_ACTION)
+    return;
+
   selected_index_ = index;
   SchedulePaint();
 }
 
 bool Combobox::SelectValue(const base::string16& value) {
+  if (style_ == STYLE_ACTION)
+    return false;
+
   for (int i = 0; i < model()->GetItemCount(); ++i) {
     if (value == model()->GetItemAt(i)) {
       SetSelectedIndex(i);
       return true;
     }
   }
   return false;
 }
 
 void Combobox::SetAccessibleName(const base::string16& name) {
@@ skipping 17 matching lines @@
 }
 
 void Combobox::Layout() {
   PrefixDelegate::Layout();
 
   gfx::Insets insets = GetInsets();
   int text_button_width = 0;
   int arrow_button_width = 0;
 
   switch (style_) {
-    case STYLE_SHOW_DROP_DOWN_ON_CLICK: {
+    case STYLE_NORMAL: {
       arrow_button_width = width();
       break;
     }
-    case STYLE_NOTIFY_ON_CLICK: {
+    case STYLE_ACTION: {
       arrow_button_width = GetDisclosureArrowLeftPadding() +
           disclosure_arrow_->width() + GetDisclosureArrowRightPadding();
       text_button_width = width() - arrow_button_width;
       break;
     }
   }
 
   int arrow_button_x = std::max(0, text_button_width);
   text_button_->SetBounds(0, 0, std::max(0, text_button_width), height());
   arrow_button_->SetBounds(arrow_button_x, 0, arrow_button_width, height());
 }
 
 bool Combobox::IsItemChecked(int id) const {
   return false;
 }
 
 bool Combobox::IsCommandEnabled(int id) const {
   return model()->IsItemEnabledAt(MenuCommandToIndex(id));
 }
 
 void Combobox::ExecuteCommand(int id) {
   selected_index_ = MenuCommandToIndex(id);
-  OnSelectionChanged();
+  OnPerformAction();
 }
 
 bool Combobox::GetAccelerator(int id, ui::Accelerator* accel) {
   return false;
 }
 
 int Combobox::GetRowCount() {
   return model()->GetItemCount();
 }
 
@@ skipping 78 matching lines @@
       new_index = GetAdjacentIndex(model(), 1, -1);
       break;
 
     // Move to the previous item if any.
     case ui::VKEY_UP:
       new_index = GetAdjacentIndex(model(), -1, selected_index_);
       break;
 
     // Click the button only when the button style mode.
     case ui::VKEY_SPACE:
-      if (style_ == STYLE_NOTIFY_ON_CLICK) {
+      if (style_ == STYLE_ACTION) {
         // When pressing space, the click event will be raised after the key is
         // released.
         text_button_->SetState(Button::STATE_PRESSED);
       } else {
         return false;
       }
       break;
 
     // Click the button only when the button style mode.
     case ui::VKEY_RETURN:
-      if (style_ != STYLE_NOTIFY_ON_CLICK)
+      if (style_ != STYLE_ACTION)
         return false;
-      HandleClickEvent();
+      OnPerformAction();
       break;
 
     default:
       return false;
   }
 
   if (show_menu) {
     UpdateFromModel();
     ShowDropDownMenu(ui::MENU_SOURCE_KEYBOARD);
   } else if (new_index != selected_index_ && new_index != kNoSelection) {
-    DCHECK(!model()->IsItemSeparatorAt(new_index));
-    selected_index_ = new_index;
-    OnSelectionChanged();
+    if (style_ != STYLE_ACTION) {

[sky, 2014/02/03 16:58:19]

nit: combine with if on 494.

[hajimehoshi, 2014/02/04 04:06:32]

Done.

+      DCHECK(!model()->IsItemSeparatorAt(new_index));
+      selected_index_ = new_index;
+      OnPerformAction();
+    }
   }
 
   return true;
 }
 
 bool Combobox::OnKeyReleased(const ui::KeyEvent& e) {
-  if (style_ != STYLE_NOTIFY_ON_CLICK)
+  if (style_ != STYLE_ACTION)
     return false;  // crbug.com/127520
 
-  if (e.key_code() == ui::VKEY_SPACE)
-    HandleClickEvent();
+  if (e.key_code() == ui::VKEY_SPACE && style_ == STYLE_ACTION)
+    OnPerformAction();
 
   return false;
 }
 
 void Combobox::OnPaint(gfx::Canvas* canvas) {
   switch (style_) {
-    case STYLE_SHOW_DROP_DOWN_ON_CLICK: {
+    case STYLE_NORMAL: {
       OnPaintBackground(canvas);
       PaintText(canvas);
       OnPaintBorder(canvas);
       break;
     }
-    case STYLE_NOTIFY_ON_CLICK: {
+    case STYLE_ACTION: {
       PaintButtons(canvas);
       PaintText(canvas);
       break;
     }
   }
 }
 
 void Combobox::OnFocus() {
   GetInputMethod()->OnFocus();
   View::OnFocus();
@@ skipping 19 matching lines @@
 
 void Combobox::OnComboboxModelChanged(ui::ComboboxModel* model) {
   DCHECK_EQ(model, model_);
   ModelChanged();
 }
 
 void Combobox::ButtonPressed(Button* sender, const ui::Event& event) {
   RequestFocus();
 
   if (sender == text_button_) {
-    HandleClickEvent();
+    OnPerformAction();
   } else {
     DCHECK_EQ(arrow_button_, sender);
     // TODO(hajimehoshi): Fix the problem that the arrow button blinks when
     // cliking this while the dropdown menu is opened.
     const base::TimeDelta delta = base::Time::Now() - closed_time_;
     if (delta.InMilliseconds() <= kMinimumMsBetweenButtonClicks)
       return;
 
     ui::MenuSourceType source_type = ui::MENU_SOURCE_MOUSE;
     if (event.IsKeyEvent())
       source_type = ui::MENU_SOURCE_KEYBOARD;
     else if (event.IsGestureEvent() || event.IsTouchEvent())
       source_type = ui::MENU_SOURCE_TOUCH;
     ShowDropDownMenu(source_type);
   }
 }
 
 void Combobox::UpdateFromModel() {
-  int max_width = 0;
   const gfx::FontList& font_list = Combobox::GetFontList();
 
   MenuItemView* menu = new MenuItemView(this);
   // MenuRunner owns |menu|.
   dropdown_list_menu_runner_.reset(new MenuRunner(menu));
 
   int num_items = model()->GetItemCount();
+  int width = 0;
   for (int i = 0; i < num_items; ++i) {
     if (model()->IsItemSeparatorAt(i)) {
       menu->AppendSeparator();
       continue;
     }
 
     base::string16 text = model()->GetItemAt(i);
 
     // Inserting the Unicode formatting characters if necessary so that the
     // text is displayed correctly in right-to-left UIs.
     base::i18n::AdjustStringForLocaleDirection(&text);
 
     menu->AppendMenuItem(i + kFirstMenuItemId, text, MenuItemView::NORMAL);
-    max_width = std::max(max_width, gfx::GetStringWidth(text, font_list));
+
+    if (style_ != STYLE_ACTION || i == selected_index_)
+      width = std::max(width, gfx::GetStringWidth(text, font_list));
   }
 
-  content_size_.SetSize(max_width, font_list.GetHeight());
+  content_size_.SetSize(width, font_list.GetHeight());
 }
 
 void Combobox::UpdateBorder() {
   scoped_ptr<FocusableBorder> border(new FocusableBorder());
-  if (style_ == STYLE_NOTIFY_ON_CLICK)
+  if (style_ == STYLE_ACTION)
     border->SetInsets(8, 13, 8, 13);
   if (invalid_)
     border->SetColor(kWarningColor);
   SetBorder(border.PassAs<Border>());
 }
 
 void Combobox::AdjustBoundsForRTLUI(gfx::Rect* rect) const {
   rect->set_x(GetMirroredXForRect(*rect));
 }
 
@@ skipping 31 matching lines @@
   gfx::Rect arrow_bounds(arrow_x,
                          height() / 2 - disclosure_arrow_->height() / 2,
                          disclosure_arrow_->width(),
                          disclosure_arrow_->height());
   AdjustBoundsForRTLUI(&arrow_bounds);
 
   canvas->DrawImageInt(*disclosure_arrow_, arrow_bounds.x(), arrow_bounds.y());
 }
 
 void Combobox::PaintButtons(gfx::Canvas* canvas) {
-  DCHECK(style_ == STYLE_NOTIFY_ON_CLICK);
+  DCHECK(style_ == STYLE_ACTION);
 
   gfx::ScopedCanvas scoped_canvas(canvas);
   if (base::i18n::IsRTL()) {
     canvas->Translate(gfx::Vector2d(width(), 0));
     canvas->Scale(-1, 1);
   }
 
   bool focused = HasFocus();
   const std::vector<const gfx::ImageSkia*>& arrow_button_images =
       menu_button_images_[focused][
@@ skipping 83 matching lines @@
   if (arrow_button_)
     arrow_button_->SetState(original_state);
   closed_time_ = base::Time::Now();
 
   // Need to explicitly clear mouse handler so that events get sent
   // properly after the menu finishes running. If we don't do this, then
   // the first click to other parts of the UI is eaten.
   SetMouseHandler(NULL);
 }
 
-void Combobox::OnSelectionChanged() {
+void Combobox::OnPerformAction() {
   NotifyAccessibilityEvent(ui::AccessibilityTypes::EVENT_VALUE_CHANGED, false);
   SchedulePaint();
+
+  // Because the combobox might die during processing at the lister, the
+  // cleaning up should be executed as a posted task with a weak ref pointer.
+  base::MessageLoop::current()->message_loop_proxy()->PostTaskAndReply(
+      FROM_HERE,
+      base::Bind(&Combobox::NotifyPerformAction,

[sky, 2014/02/03 16:58:19]

You should not delay the notification. Instead before calling listener grab
reference from the weak factory. After calling the listener if the reference
points to NULL, you know Combobox has been destroyed.

[hajimehoshi, 2014/02/04 04:06:32]

Done.

+                 weak_ptr_factory_.GetWeakPtr()),
+      base::Bind(&Combobox::AfterPerformAction,
+                 weak_ptr_factory_.GetWeakPtr()));
+}
+
+void Combobox::NotifyPerformAction() {
   if (listener_)
-    listener_->OnSelectedIndexChanged(this);
-  // |this| may now be deleted.
+    listener_->OnPerformAction(this);
+}
+
+void Combobox::AfterPerformAction() {
+  if (style_ == STYLE_ACTION)
+    selected_index_ = 0;
 }
 
 int Combobox::MenuCommandToIndex(int menu_command_id) const {
   // (note that the id received is offset by kFirstMenuItemId)
   // Revert menu ID offset to map back to combobox model.
   int index = menu_command_id - kFirstMenuItemId;
   DCHECK_LT(index, model()->GetItemCount());
   return index;
 }
 
 int Combobox::GetDisclosureArrowLeftPadding() const {
   switch (style_) {
-    case STYLE_SHOW_DROP_DOWN_ON_CLICK:
+    case STYLE_NORMAL:
       return kDisclosureArrowLeftPadding;
-    case STYLE_NOTIFY_ON_CLICK:
+    case STYLE_ACTION:
       return kDisclosureArrowButtonLeftPadding;
   }
   NOTREACHED();
   return 0;
 }
 
 int Combobox::GetDisclosureArrowRightPadding() const {
   switch (style_) {
-    case STYLE_SHOW_DROP_DOWN_ON_CLICK:
+    case STYLE_NORMAL:
       return kDisclosureArrowRightPadding;
-    case STYLE_NOTIFY_ON_CLICK:
+    case STYLE_ACTION:
       return kDisclosureArrowButtonRightPadding;
   }
   NOTREACHED();
   return 0;
 }
 
-void Combobox::HandleClickEvent() {
-  if (style_ != STYLE_NOTIFY_ON_CLICK)
-    return;
-
-  if (listener_)
-    listener_->OnComboboxTextButtonClicked(this);
-}
-
 }  // namespace views