Add the button style for combobox

ui/views/controls/combobox/combobox.cc

Index: ui/views/controls/combobox/combobox.cc
diff --git a/ui/views/controls/combobox/combobox.cc b/ui/views/controls/combobox/combobox.cc
index 2600b174f07d27d2e9f3f8511ca71049270a444c..f38fc0a58b4c7b503df4ed92df8d8048dfdf6d70 100644
--- a/ui/views/controls/combobox/combobox.cc
+++ b/ui/views/controls/combobox/combobox.cc
@@ -12,9 +12,13 @@
 #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"
 #include "ui/native_theme/native_theme.h"
 #include "ui/views/color_constants.h"
+#include "ui/views/controls/button/custom_button.h"
+#include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/combobox/combobox_listener.h"
 #include "ui/views/controls/focusable_border.h"
 #include "ui/views/controls/menu/menu_runner.h"
@@ -22,6 +26,7 @@
 #include "ui/views/controls/prefix_selector.h"
 #include "ui/views/ime/input_method.h"
 #include "ui/views/mouse_constants.h"
+#include "ui/views/painter.h"
 #include "ui/views/widget/widget.h"
 
 namespace views {
@@ -39,6 +44,8 @@ const int kMinComboboxWidth = 25;
 // Size of the combobox arrow margins
 const int kDisclosureArrowLeftPadding = 7;
 const int kDisclosureArrowRightPadding = 7;
+const int kDisclosureArrowButtonLeftPadding = 11;
+const int kDisclosureArrowButtonRightPadding = 12;
 
 // Define the id of the first item in the menu (since it needs to be > 0)
 const int kFirstMenuItemId = 1000;
@@ -48,6 +55,19 @@ const SkColor kInvalidTextColor = SK_ColorWHITE;
 // Used to indicate that no item is currently selected by the user.
 const int kNoSelection = -1;
 
+const int kBodyButtonImages[] = IMAGE_GRID(IDR_COMBOBOX_BUTTON);
+const int kHoveredBodyButtonImages[] = IMAGE_GRID(IDR_COMBOBOX_BUTTON_H);
+const int kPressedBodyButtonImages[] = IMAGE_GRID(IDR_COMBOBOX_BUTTON_P);
+
+#define MENU_IMAGE_GRID(x) { \
+    x ## _MENU_TOP, x ## _MENU_CENTER, x ## _MENU_BOTTOM, }
+
+const int kMenuButtonImages[] = MENU_IMAGE_GRID(IDR_COMBOBOX_BUTTON);
+const int kHoveredMenuButtonImages[] = MENU_IMAGE_GRID(IDR_COMBOBOX_BUTTON_H);
+const int kPressedMenuButtonImages[] = MENU_IMAGE_GRID(IDR_COMBOBOX_BUTTON_P);
+
+#undef MENU_IMAGE_GRID
+
 // The background to use for invalid comboboxes.
 class InvalidBackground : public Background {
  public:
@@ -66,6 +86,33 @@ class InvalidBackground : public Background {
   DISALLOW_COPY_AND_ASSIGN(InvalidBackground);
 };
 
+// The transparent button which holds a button state but is not rendered.
+class TransparentButton : public CustomButton {
+ public:
+  TransparentButton(ButtonListener* listener)
+      : CustomButton(listener) {
+    SetAnimationDuration(LabelButton::kHoverAnimationDurationMs);
+  }
+  virtual ~TransparentButton() {}
+
+  // CustomButton:
+  virtual void StateChanged() OVERRIDE {
+    parent()->SchedulePaint();
+  }
+
+  // gfx::AnimationDelegate:

[sky, 2013/12/03 20:52:30]

Does SchedulePaint() on this always need to force a SchedulePaint on the parent
the size of the parent?

[hajimehoshi, 2013/12/04 06:30:02]

Done. (ScheudulePaintInRect with its bounds should be fine)

[sky, 2013/12/04 16:34:05]

Why is that? Don't you need to force the whole combobox to redraw? For example,
presumably a mouse press on one of the buttons needs to redraw the whole
combobox, right?

[hajimehoshi, 2013/12/05 08:05:57]

Each button has its own animation value only for hovering, so I don't think
pressing a mouse button needs redrawing the whole combobox. You mean that
pressing one button should stop all hovering animations of two buttons on the
combobox?

+  virtual void AnimationProgressed(const gfx::Animation* animation) OVERRIDE {
+    parent()->SchedulePaint();
+  }
+
+  double GetAnimationValue() const {
+    return hover_animation_->GetCurrentValue();
+  }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TransparentButton);
+};
+
 // Returns the next or previous valid index (depending on |increment|'s value).
 // Skips separator or disabled indices. Returns -1 if there is no valid adjacent
 // index.
@@ -81,6 +128,132 @@ int GetAdjacentIndex(ui::ComboboxModel* model, int increment, int index) {
   return kNoSelection;
 }
 
+// Returns the image resource ids of an array for the body button.
+//
+// TODO(hajimehoshi): This function should return the images for the 'disabled'
+// status. (crbug/270052)
+//
+// TODO(hajimehoshi): Currently, |focused| is ignored. This should return the
+// images for the 'focused' status. (crbug/270052)
+const int* GetBodyButtonImageIds(bool focused, Button::ButtonState state) {
+  switch (state) {
+    case Button::STATE_DISABLED:
+      return kBodyButtonImages;
+    case Button::STATE_NORMAL:
+      return kBodyButtonImages;
+    case Button::STATE_HOVERED:
+      return kHoveredBodyButtonImages;
+    case Button::STATE_PRESSED:
+      return kPressedBodyButtonImages;
+    default:
+      NOTREACHED();
+  }
+  return NULL;
+}
+
+// Returns the image resource ids of an array for the menu button.
+const int* GetMenuButtonImageIds(bool focused, Button::ButtonState state) {
+  switch (state) {
+    case Button::STATE_DISABLED:
+      return kMenuButtonImages;
+    case Button::STATE_NORMAL:
+      return kMenuButtonImages;
+    case Button::STATE_HOVERED:
+      return kHoveredMenuButtonImages;
+    case Button::STATE_PRESSED:
+      return kPressedMenuButtonImages;
+    default:
+      NOTREACHED();
+  }
+  return NULL;
+}
+
+// Returns the images for the buttons.
+std::vector<const gfx::ImageSkia*> GetButtonImages(bool menu,
+                                                   bool focused,
+                                                   Button::ButtonState state) {
+  const int* ids;
+  size_t num_ids;
+  if (!menu) {
+    ids = GetBodyButtonImageIds(focused, state);

[sky, 2013/12/03 20:52:30]

This and GetMenuButtonImageIds should return the num_ids too (less error prone).

[hajimehoshi, 2013/12/04 06:30:02]

Done.

+    num_ids = 9;
+  } else {
+    ids = GetMenuButtonImageIds(focused, state);
+    num_ids = 3;
+  }
+  std::vector<const gfx::ImageSkia*> images;
+  images.reserve(num_ids);
+  ui::ResourceBundle& rb = ui::ResourceBundle::GetSharedInstance();
+  for (size_t i = 0; i < num_ids; i++) {
+    int id = ids[i];

[sky, 2013/12/03 20:52:30]

nit: no need for the temporary here.

[hajimehoshi, 2013/12/04 06:30:02]

Done.

+    images.push_back(rb.GetImageSkiaNamed(id));
+  }
+  return images;
+}
+
+// Paints three images in a row at the given location.
+void PaintTandemImages(gfx::Canvas* canvas,

[sky, 2013/12/03 20:52:30]

Isn't tandem too, yet this paints three.

[hajimehoshi, 2013/12/04 06:30:02]

Done.

+                       const gfx::ImageSkia& top_image,
+                       const gfx::ImageSkia& center_image,
+                       const gfx::ImageSkia& bottom_image,
+                       int x, int y, int width, int height) {
+  canvas->DrawImageInt(top_image,
+                       0, 0, top_image.width(), top_image.height(),
+                       x, y, width, top_image.height(), false);
+  y += top_image.height();
+  int center_height = height - top_image.height() - bottom_image.height();
+  canvas->DrawImageInt(center_image,
+                       0, 0, center_image.width(), center_image.height(),
+                       x, y, width, center_height, false);
+  y += center_height;
+  canvas->DrawImageInt(bottom_image,
+                       0, 0, bottom_image.width(), bottom_image.height(),
+                       x, y, width, bottom_image.height(), false);
+}
+
+// Paints the text button.
+void PaintTextButton(

[sky, 2013/12/03 20:52:30]

This code looks a lot like that offered by Painter. Specifically imagegrid or
HorizontalPainter. How do they differ?

[hajimehoshi, 2013/12/04 06:30:02]

Done. Menu button images remain. Is it better to create VerticalPainter?

+    gfx::Canvas* canvas,
+    const std::vector<const gfx::ImageSkia*>& text_button_images,
+    int width, int height) {
+  int current_x = 0;
+  int current_width = text_button_images[0]->width();
+  PaintTandemImages(canvas,
+                    *text_button_images[0],
+                    *text_button_images[3],
+                    *text_button_images[6],
+                    current_x, 0, current_width, height);
+
+  current_x += current_width;
+  current_width = width - text_button_images[0]->width() -
+      text_button_images[2]->width();
+  PaintTandemImages(canvas,
+                    *text_button_images[1],
+                    *text_button_images[4],
+                    *text_button_images[7],
+                    current_x, 0, current_width, height);
+
+  current_x += current_width;
+  current_width = text_button_images[2]->width();
+  PaintTandemImages(canvas,
+                    *text_button_images[2],
+                    *text_button_images[5],
+                    *text_button_images[8],
+                    current_x, 0, current_width, height);
+}
+
+// Paints the arrow button.
+void PaintArrowButton(
+    gfx::Canvas* canvas,
+    const std::vector<const gfx::ImageSkia*>& arrow_button_images,
+    int x, int height) {
+  PaintTandemImages(canvas,
+                    *arrow_button_images[0],
+                    *arrow_button_images[1],
+                    *arrow_button_images[2],
+                    x, 0, arrow_button_images[0]->width(), height);
+}
+
 }  // namespace
 
 // static
@@ -91,17 +264,43 @@ const char Combobox::kViewClassName[] = "views/Combobox";
 
 Combobox::Combobox(ui::ComboboxModel* model)
     : model_(model),
+      style_(STYLE_SHOW_DROP_DOWN_ON_CLICK),
       listener_(NULL),
       selected_index_(model_->GetDefaultIndex()),
       invalid_(false),
       text_border_(new FocusableBorder()),
       disclosure_arrow_(ui::ResourceBundle::GetSharedInstance().GetImageNamed(
           IDR_MENU_DROPARROW).ToImageSkia()),
-      dropdown_open_(false) {
+      dropdown_open_(false),
+      text_button_(new TransparentButton(this)),

[sky, 2013/12/03 20:52:30]

Is there a reason these need to be always created?

[hajimehoshi, 2013/12/04 06:30:02]

Done. (It could be simpler that the buttons always exist.)

+      arrow_button_(new TransparentButton(this)) {
   model_->AddObserver(this);
   UpdateFromModel();
   set_focusable(true);
   set_border(text_border_);
+
+  // Initialize the button images.
+  Button::ButtonState button_states[] = {
+    Button::STATE_DISABLED,
+    Button::STATE_NORMAL,
+    Button::STATE_HOVERED,
+    Button::STATE_PRESSED,
+  };
+  for (int focused = 0; focused < 2; focused++) {
+    for (size_t state_index = 0; state_index < arraysize(button_states);
+         state_index++) {
+      Button::ButtonState state = button_states[state_index];
+      body_button_images_[focused][state] =
+          GetButtonImages(false, focused, state);
+      menu_button_images_[focused][state] =
+          GetButtonImages(true, focused, state);
+    }
+  }
+
+  text_button_->SetVisible(false);
+  arrow_button_->SetVisible(false);
+  AddChildView(text_button_);
+  AddChildView(arrow_button_);

[sky, 2013/12/03 20:52:30]

You should also make these non focusable.

[hajimehoshi, 2013/12/04 06:30:02]

Done.

 }
 
 Combobox::~Combobox() {
@@ -114,6 +313,30 @@ const gfx::Font& Combobox::GetFont() {
   return rb.GetFont(ui::ResourceBundle::BaseFont);
 }
 
+void Combobox::SetStyle(Style style) {
+  if (style_ == style)
+    return;
+
+  style_ = style;
+  switch (style) {
+    case STYLE_SHOW_DROP_DOWN_ON_CLICK: {
+      text_border_ = new FocusableBorder();

[sky, 2013/12/03 20:52:30]

Why do you create a new border here?

[hajimehoshi, 2013/12/04 06:30:02]

It's because there is no way to reset the insets of the FocusableBorder. I'll
add a comment.

[sky, 2013/12/04 16:34:05]

FocusableBorder has a SetInsets, can't you use it? That said, since SetBorder is
public and would effectively delete text_border_ out from under you it's best to
reset. So, go with resetting but you should also remove text_border_ completely.

[hajimehoshi, 2013/12/05 08:05:57]

The default insets values of FocusableBorder is internally hidden and I should
make them public if I use SetInsets. (You were against adding a method
ResetInsets to FocusableBorder at the other review).

IIUC, the border is owned by the base view and set_border with a new border
deletes the old border, so my current change works correctly, right? If I delete
the border here explicitly, set_border will cause a double free error.

+      set_border(text_border_);
+      text_button_->SetVisible(false);
+      arrow_button_->SetVisible(false);
+      break;
+    }
+    case STYLE_NOTIFY_ON_CLICK: {
+      text_border_->SetInsets(8, 13, 8, 13);
+      text_button_->SetVisible(true);
+      arrow_button_->SetVisible(true);
+      break;
+    }
+  }
+
+  PreferredSizeChanged();
+}
+
 void Combobox::ModelChanged() {
   selected_index_ = std::min(0, model_->GetItemCount());
   UpdateFromModel();
@@ -156,6 +379,18 @@ ui::TextInputClient* Combobox::GetTextInputClient() {
   return selector_.get();
 }
 
+void Combobox::PreferredSizeChanged() {
+  gfx::Insets insets = GetInsets();
+  int height = content_size_.height() + insets.height();
+  int text_button_width =
+      std::max(kMinComboboxWidth, content_size_.width()) + insets.width();
+  text_button_->SetBounds(0, 0, text_button_width, height);
+  int arrow_button_x = text_button_width;
+  int arrow_button_width = GetDisclosureArrowLeftPadding() +
+      disclosure_arrow_->width() + GetDisclosureArrowRightPadding();
+  arrow_button_->SetBounds(arrow_button_x, 0, arrow_button_width, height);

[sky, 2013/12/03 20:52:30]

This should be done in Layout.

[hajimehoshi, 2013/12/04 06:30:02]

Done.

+  PrefixDelegate::PreferredSizeChanged();
+}
 
 bool Combobox::IsItemChecked(int id) const {
   return false;
@@ -201,9 +436,8 @@ gfx::Size Combobox::GetPreferredSize() {
   // the minimum width for the dropdown list.
   gfx::Insets insets = GetInsets();
   int total_width = std::max(kMinComboboxWidth, content_size_.width()) +
-      insets.width() + kDisclosureArrowLeftPadding +
-      disclosure_arrow_->width() + kDisclosureArrowRightPadding;
-
+      insets.width() + GetDisclosureArrowLeftPadding() +
+      disclosure_arrow_->width() + GetDisclosureArrowRightPadding();
   return gfx::Size(total_width, content_size_.height() + insets.height());
 }
 
@@ -228,7 +462,6 @@ bool Combobox::OnMousePressed(const ui::MouseEvent& mouse_event) {
     UpdateFromModel();
     ShowDropDownMenu(ui::MENU_SOURCE_MOUSE);
   }
-
   return true;
 }
 
@@ -280,6 +513,18 @@ bool Combobox::OnKeyPressed(const ui::KeyEvent& e) {
       new_index = GetAdjacentIndex(model(), -1, selected_index_);
       break;
 
+    // Click the button only when the button style mode.
+    case ui::VKEY_SPACE:
+      // When pressing space, the click event will be raised after the key is
+      // released.
+      text_button_->SetState(Button::STATE_PRESSED);
+      break;
+
+    // Click the button only when the button style mode.
+    case ui::VKEY_RETURN:
+      HandleClickEvent();
+      break;
+
     default:
       return false;
   }
@@ -297,7 +542,13 @@ bool Combobox::OnKeyPressed(const ui::KeyEvent& e) {
 }
 
 bool Combobox::OnKeyReleased(const ui::KeyEvent& e) {
-  return false;  // crbug.com/127520
+  if (style_ != STYLE_NOTIFY_ON_CLICK)
+    return false;  // crbug.com/127520
+
+  if (e.key_code() == ui::VKEY_SPACE)
+    HandleClickEvent();
+
+  return false;
 }
 
 void Combobox::OnGestureEvent(ui::GestureEvent* gesture) {
@@ -311,9 +562,19 @@ void Combobox::OnGestureEvent(ui::GestureEvent* gesture) {
 }
 
 void Combobox::OnPaint(gfx::Canvas* canvas) {
-  OnPaintBackground(canvas);
-  PaintText(canvas);
-  OnPaintBorder(canvas);
+  switch (style_) {
+    case STYLE_SHOW_DROP_DOWN_ON_CLICK: {
+      OnPaintBackground(canvas);
+      PaintText(canvas);
+      OnPaintBorder(canvas);
+      break;
+    }
+    case STYLE_NOTIFY_ON_CLICK: {
+      PaintButtons(canvas);
+      PaintText(canvas);
+      break;
+    }
+  }
 }
 
 void Combobox::OnFocus() {
@@ -341,6 +602,26 @@ void Combobox::OnModelChanged() {
   ModelChanged();
 }
 
+void Combobox::ButtonPressed(Button* sender, const ui::Event& event) {
+  if (sender == text_button_) {
+    HandleClickEvent();
+  } else {
+    // TODO(hajimehoshi): Fix the problem that the arrow button blinks when

[sky, 2013/12/03 20:52:30]

DCHECK sender is what you expect.

[hajimehoshi, 2013/12/04 06:30:02]

Done.

+    // cliking this while the dropdown menu is opened.
+    const base::TimeDelta delta = base::Time::Now() - closed_time_;
+    if (delta.InMilliseconds() <= kMinimumMsBetweenButtonClicks)
+      return;
+
+    UpdateFromModel();
+    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::Font& font = Combobox::GetFont();
@@ -392,8 +673,8 @@ void Combobox::PaintText(gfx::Canvas* canvas) {
     selected_index_ = 0;
   string16 text = model()->GetItemAt(selected_index_);
 
-  int disclosure_arrow_offset = width() - disclosure_arrow_->width()
-      - kDisclosureArrowLeftPadding - kDisclosureArrowRightPadding;
+  int disclosure_arrow_offset = width() - disclosure_arrow_->width() -
+      GetDisclosureArrowLeftPadding() - GetDisclosureArrowRightPadding();
 
   const gfx::Font& font = Combobox::GetFont();
   int text_width = font.GetStringWidth(text);
@@ -404,7 +685,8 @@ void Combobox::PaintText(gfx::Canvas* canvas) {
   AdjustBoundsForRTLUI(&text_bounds);
   canvas->DrawStringInt(text, font, text_color, text_bounds);
 
-  gfx::Rect arrow_bounds(disclosure_arrow_offset + kDisclosureArrowLeftPadding,
+  int arrow_x = disclosure_arrow_offset + GetDisclosureArrowLeftPadding();
+  gfx::Rect arrow_bounds(arrow_x,
                          height() / 2 - disclosure_arrow_->height() / 2,
                          disclosure_arrow_->width(),
                          disclosure_arrow_->height());
@@ -420,6 +702,64 @@ void Combobox::PaintText(gfx::Canvas* canvas) {
   canvas->Restore();
 }
 
+void Combobox::PaintButtons(gfx::Canvas* canvas) {
+  canvas->Save();
+  if (base::i18n::IsRTL()) {
+    canvas->Translate(gfx::Vector2d(width(), 0));
+    canvas->Scale(-1, 1);
+  }
+
+  bool focused = HasFocus() && IsAccessibilityFocusable();

[sky, 2013/12/03 20:52:30]

You should only need check HasFocus here.

[hajimehoshi, 2013/12/04 06:30:02]

Done.

+  const std::vector<const gfx::ImageSkia*>& text_button_images =
+      body_button_images_[focused][
+          text_button_->state() == Button::STATE_HOVERED ?
+          Button::STATE_NORMAL : text_button_->state()];
+  const std::vector<const gfx::ImageSkia*>& arrow_button_images =
+      menu_button_images_[focused][
+          arrow_button_->state() == Button::STATE_HOVERED ?
+          Button::STATE_NORMAL : arrow_button_->state()];
+
+  int text_button_width = width() - arrow_button_images[0]->width();
+  int text_button_hover_alpha =
+      text_button_->state() == Button::STATE_PRESSED ? 0 :
+      static_cast<int>(static_cast<TransparentButton*>(text_button_)->
+                       GetAnimationValue() * 255);
+  if (text_button_hover_alpha < 255) {
+    canvas->SaveLayerAlpha(255 - text_button_hover_alpha);
+    PaintTextButton(canvas, text_button_images, text_button_width, height());
+    canvas->Restore();
+  }
+  if (0 < text_button_hover_alpha) {
+    canvas->SaveLayerAlpha(text_button_hover_alpha);
+    const std::vector<const gfx::ImageSkia*>& text_button_hovered_images =
+        body_button_images_[focused][Button::STATE_HOVERED];
+    PaintTextButton(canvas, text_button_hovered_images,
+                    text_button_width, height());
+    canvas->Restore();
+  }
+
+  int arrow_button_x = text_button_width;
+  int arrow_button_hover_alpha =
+      arrow_button_->state() == Button::STATE_PRESSED ? 0 :
+      static_cast<int>(static_cast<TransparentButton*>(arrow_button_)->
+                       GetAnimationValue() * 255);
+  if (arrow_button_hover_alpha < 255) {
+    canvas->SaveLayerAlpha(255 - arrow_button_hover_alpha);
+    PaintArrowButton(canvas, arrow_button_images, arrow_button_x, height());
+    canvas->Restore();
+  }
+  if (0 < arrow_button_hover_alpha) {
+    canvas->SaveLayerAlpha(arrow_button_hover_alpha);
+    const std::vector<const gfx::ImageSkia*>& arrow_button_hovered_images =
+        menu_button_images_[focused][Button::STATE_HOVERED];
+    PaintArrowButton(canvas, arrow_button_hovered_images,
+                     arrow_button_x, height());
+    canvas->Restore();
+  }
+
+  canvas->Restore();
+}
+
 void Combobox::ShowDropDownMenu(ui::MenuSourceType source_type) {
   if (!dropdown_list_menu_runner_.get())
     UpdateFromModel();
@@ -427,8 +767,8 @@ void Combobox::ShowDropDownMenu(ui::MenuSourceType source_type) {
   // Extend the menu to the width of the combobox.
   MenuItemView* menu = dropdown_list_menu_runner_->GetMenu();
   SubmenuView* submenu = menu->CreateSubmenu();
-  submenu->set_minimum_preferred_width(size().width() -
-                                (kMenuBorderWidthLeft + kMenuBorderWidthRight));
+  submenu->set_minimum_preferred_width(
+      size().width() - (kMenuBorderWidthLeft + kMenuBorderWidthRight));
 
   gfx::Rect lb = GetLocalBounds();
   gfx::Point menu_position(lb.origin());
@@ -441,16 +781,20 @@ void Combobox::ShowDropDownMenu(ui::MenuSourceType source_type) {
 
   View::ConvertPointToScreen(this, &menu_position);
   if (menu_position.x() < 0)
-      menu_position.set_x(0);
+    menu_position.set_x(0);
 
   gfx::Rect bounds(menu_position, lb.size());
 
+  Button::ButtonState original_state = arrow_button_->state();
+  arrow_button_->SetState(Button::STATE_PRESSED);
   dropdown_open_ = true;
   if (dropdown_list_menu_runner_->RunMenuAt(GetWidget(), NULL, bounds,
           MenuItemView::TOPLEFT, source_type, MenuRunner::COMBOBOX) ==
-      MenuRunner::MENU_DELETED)
+      MenuRunner::MENU_DELETED) {
     return;
+  }
   dropdown_open_ = false;
+  arrow_button_->SetState(original_state);
   closed_time_ = base::Time::Now();
 
   // Need to explicitly clear mouse handler so that events get sent
@@ -474,4 +818,34 @@ int Combobox::MenuCommandToIndex(int menu_command_id) const {
   return index;
 }
 
+int Combobox::GetDisclosureArrowLeftPadding() const {
+  switch (style_) {
+    case STYLE_SHOW_DROP_DOWN_ON_CLICK:
+      return kDisclosureArrowLeftPadding;
+    case STYLE_NOTIFY_ON_CLICK:
+      return kDisclosureArrowButtonLeftPadding;
+  }
+  NOTREACHED();
+  return 0;
+}
+
+int Combobox::GetDisclosureArrowRightPadding() const {
+  switch (style_) {
+    case STYLE_SHOW_DROP_DOWN_ON_CLICK:
+      return kDisclosureArrowRightPadding;
+    case STYLE_NOTIFY_ON_CLICK:
+      return kDisclosureArrowButtonRightPadding;
+  }
+  NOTREACHED();
+  return 0;
+}
+
+void Combobox::HandleClickEvent() {
+  if (style_ != STYLE_NOTIFY_ON_CLICK)
+    return;
+
+  if (listener_)
+    listener_->OnComboboxTextButtonClicked(this);
+}
+
 }  // namespace views