Use a ui::MenuModel in views::Combobox

ui/views/controls/combobox/combobox.h

 // 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.
 
 #ifndef UI_VIEWS_CONTROLS_COMBOBOX_COMBOBOX_H_
 #define UI_VIEWS_CONTROLS_COMBOBOX_COMBOBOX_H_
 
-#include <string>
-
 #include "base/memory/weak_ptr.h"
+#include "base/strings/string16.h"
 #include "base/time/time.h"
-#include "ui/base/models/combobox_model_observer.h"
-#include "ui/gfx/animation/animation_delegate.h"
-#include "ui/gfx/native_widget_types.h"
 #include "ui/views/controls/button/button.h"
-#include "ui/views/controls/menu/menu_delegate.h"
 #include "ui/views/controls/prefix_delegate.h"
 
 namespace gfx {
 class FontList;
-class SlideAnimation;
 }
 
 namespace ui {
 class ComboboxModel;
+class MenuModel;
 }
 
 namespace views {
+namespace test {
+class ComboboxTestApi;
+}
 
 class ComboboxListener;
-class ComboboxMenuRunner;
 class CustomButton;
-class FocusableBorder;
 class MenuRunner;
-class MenuRunnerHandler;
 class Painter;
 class PrefixSelector;
 
 // A non-editable combobox (aka a drop-down list or selector).
 // Combobox has two distinct parts, the drop down arrow and the text. Combobox
 // offers two distinct behaviors:
 // * STYLE_NORMAL: typical combobox, clicking on the text and/or button shows
 // the drop down, arrow keys change selection, selected index can be changed by
 // the user to something other than the first item.
 // * STYLE_ACTION: clicking on the text notifies the listener. The menu can be
 // shown only by clicking on the arrow. The selected index is always reverted to
 // 0 after the listener is notified.
-class VIEWS_EXPORT Combobox : public MenuDelegate,
-                              public PrefixDelegate,
-                              public ui::ComboboxModelObserver,
-                              public ButtonListener {
+class VIEWS_EXPORT Combobox : public PrefixDelegate, public ButtonListener {
  public:
   // The style of the combobox.
   enum Style {
     STYLE_NORMAL,
     STYLE_ACTION,
   };
 
   // The combobox's class name.
   static const char kViewClassName[];
 
@@ skipping 35 matching lines @@
   const char* GetClassName() const override;
   bool SkipDefaultKeyEventProcessing(const ui::KeyEvent& e) override;
   bool OnKeyPressed(const ui::KeyEvent& e) override;
   bool OnKeyReleased(const ui::KeyEvent& e) override;
   void OnPaint(gfx::Canvas* canvas) override;
   void OnFocus() override;
   void OnBlur() override;
   void GetAccessibleState(ui::AXViewState* state) override;
   void Layout() override;
 
-  // Overridden from MenuDelegate:
-  bool IsItemChecked(int id) const override;
-  bool IsCommandEnabled(int id) const override;
-  void ExecuteCommand(int id) override;
-  bool GetAccelerator(int id, ui::Accelerator* accelerator) const override;
-
   // Overridden from PrefixDelegate:
   int GetRowCount() override;
   int GetSelectedRow() override;
   void SetSelectedRow(int row) override;
   base::string16 GetTextForRow(int row) override;
 
-  // Overriden from ComboboxModelObserver:
-  void OnComboboxModelChanged(ui::ComboboxModel* model) override;
-
   // Overriden from ButtonListener:
   void ButtonPressed(Button* sender, const ui::Event& event) override;
 
  private:
-  FRIEND_TEST_ALL_PREFIXES(ComboboxTest, Click);
-  FRIEND_TEST_ALL_PREFIXES(ComboboxTest, ClickButDisabled);
-  FRIEND_TEST_ALL_PREFIXES(ComboboxTest, NotifyOnClickWithMouse);
-  FRIEND_TEST_ALL_PREFIXES(ComboboxTest, ContentWidth);
+  friend class test::ComboboxTestApi;

[sky, 2015/09/03 17:52:27]

If you're going to friend this, then nuke the GetMenuModelForTest and have
ComboboxTestApi have the ability to return the menumodel.

[tapted, 2015/09/04 00:46:54]

Yeah - I didn't really like GetMenuModelForTest() :). The test API doesn't know
how to cast from a ComboboxMenuModelAdapter to a ui::MenuModel and the adapter
leans on its nested-class-is-implicit-friend a lot so I didn't want to expose it
the adapter declaration.

But! I can fix this by moving GetContentSize into Combobox - maybe it even fits
nicer there - and changing the static type of |menu_model_adapter_|

Done.

 
-  // Updates the combobox's content from its model.
-  void UpdateFromModel();
+  class ComboboxMenuModelAdapter;
 
   // Updates the border according to the current state.
   void UpdateBorder();
 
   // Given bounds within our View, this helper mirrors the bounds if necessary.
   void AdjustBoundsForRTLUI(gfx::Rect* rect) const;
 
   // Draws the selected value of the drop down list
   void PaintText(gfx::Canvas* canvas);
 
   // Draws the button images.
   void PaintButtons(gfx::Canvas* canvas);
 
   // Show the drop down list
   void ShowDropDownMenu(ui::MenuSourceType source_type);
 
   // Called when the selection is changed by the user.
   void OnPerformAction();
-  void NotifyPerformAction();
-  void AfterPerformAction();
-
-  // Converts a menu command ID to a menu item index.
-  int MenuCommandToIndex(int menu_command_id) const;
 
   int GetDisclosureArrowLeftPadding() const;
   int GetDisclosureArrowRightPadding() const;
 
   // Returns the size of the disclosure arrow.
   gfx::Size ArrowSize() const;
 
   // Handles the clicking event.
   void HandleClickEvent();
 
   PrefixSelector* GetPrefixSelector();
 
+  ui::MenuModel* GetMenuModelForTest();
+
   // Our model. Not owned.
   ui::ComboboxModel* model_;
 
   // The visual style of this combobox.
   Style style_;
 
   // Our listener. Not owned. Notified when the selected index change.
   ComboboxListener* listener_;
 
   // The current selected index; -1 and means no selection.
   int selected_index_;
 
   // True when the selection is visually denoted as invalid.
   bool invalid_;
 
   // The accessible name of this combobox.
   base::string16 accessible_name_;
 
   // A helper used to select entries by keyboard input.
   scoped_ptr<PrefixSelector> selector_;
 
-  // Responsible for showing the context menu.
-  scoped_ptr<MenuRunner> dropdown_list_menu_runner_;
-
-  // Weak. Owned by dropdown_list_menu_runner_.
-  MenuItemView* menu_;
-
-  // Is the drop down list showing
-  bool dropdown_open_;
+  // Adapts a ComboboxModel for use by a views MenuRunner.
+  scoped_ptr<ComboboxMenuModelAdapter> menu_model_adapter_;
 
   // Like MenuButton, we use a time object in order to keep track of when the
   // combobox was closed. The time is used for simulating menu behavior; that
   // is, if the menu is shown and the button is pressed, we need to close the
   // menu. There is no clean way to get the second click event because the
   // menu is displayed using a modal loop and, unlike regular menus in Windows,
   // the button is not part of the displayed menu.
   base::Time closed_time_;
 
-  // The maximum dimensions of the content in the dropdown
-  mutable gfx::Size content_size_;
+  // The maximum dimensions of the content in the dropdown.
+  gfx::Size content_size_;
 
   // The painters or images that are used when |style_| is STYLE_BUTTONS. The
   // first index means the state of unfocused or focused.
   // The images are owned by ResourceBundle.
   scoped_ptr<Painter> body_button_painters_[2][Button::STATE_COUNT];
   std::vector<const gfx::ImageSkia*>
       menu_button_images_[2][Button::STATE_COUNT];
 
   // The transparent buttons to handle events and render buttons. These are
   // placed on top of this combobox as child views, accept event and manage the
   // button states. These are not rendered but when |style_| is
   // STYLE_NOTIFY_ON_CLICK, a Combobox renders the button images according to
   // these button states.
   // The base View takes the ownerships of these as child views.
   CustomButton* text_button_;
   CustomButton* arrow_button_;
 
+  // Set while the dropdown is showing. Ensures the menu is closed if |this| is
+  // destroyed.
+  scoped_ptr<views::MenuRunner> menu_runner_;
+
   // Used for making calbacks.
   base::WeakPtrFactory<Combobox> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(Combobox);
 };
 
 }  // namespace views
 
 #endif  // UI_VIEWS_CONTROLS_COMBOBOX_COMBOBOX_H_