Commit 76cddaa8 authored by Wei Li's avatar Wei Li Committed by Commit Bot

Make MenuItemView default constructible

Make MenuItemView's delegate an optional argument in its constructor so
it can be default constructed. Its delegate can always be set thru the
existing set_delegate() function. Also need to add a few more checks for
the return delegate pointer.

Bug: 1108460
Change-Id: Ie90709271c971cadccf3e13166df0c240d73dfca
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2321526Reviewed-by: default avatarScott Violet <sky@chromium.org>
Commit-Queue: Wei Li <weili@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792836}
parent c2ec4f12
...@@ -174,7 +174,9 @@ base::string16 MenuItemView::GetTooltipText(const gfx::Point& p) const { ...@@ -174,7 +174,9 @@ base::string16 MenuItemView::GetTooltipText(const gfx::Point& p) const {
} }
const MenuDelegate* delegate = GetDelegate(); const MenuDelegate* delegate = GetDelegate();
CHECK(delegate); if (!delegate)
return base::string16();
gfx::Point location(p); gfx::Point location(p);
ConvertPointToScreen(this, &location); ConvertPointToScreen(this, &location);
return delegate->GetTooltipText(command_, location); return delegate->GetTooltipText(command_, location);
...@@ -215,7 +217,8 @@ void MenuItemView::GetAccessibleNodeData(ui::AXNodeData* node_data) { ...@@ -215,7 +217,8 @@ void MenuItemView::GetAccessibleNodeData(ui::AXNodeData* node_data) {
break; break;
case Type::kCheckbox: case Type::kCheckbox:
case Type::kRadio: { case Type::kRadio: {
const bool is_checked = GetDelegate()->IsItemChecked(GetCommand()); const bool is_checked =
GetDelegate() && GetDelegate()->IsItemChecked(GetCommand());
node_data->SetCheckedState(is_checked ? ax::mojom::CheckedState::kTrue node_data->SetCheckedState(is_checked ? ax::mojom::CheckedState::kTrue
: ax::mojom::CheckedState::kFalse); : ax::mojom::CheckedState::kFalse);
} break; } break;
...@@ -816,7 +819,7 @@ void MenuItemView::Init(MenuItemView* parent, ...@@ -816,7 +819,7 @@ void MenuItemView::Init(MenuItemView* parent,
if (type_ == Type::kCheckbox || type_ == Type::kRadio) { if (type_ == Type::kCheckbox || type_ == Type::kRadio) {
radio_check_image_view_ = AddChildView(std::make_unique<ImageView>()); radio_check_image_view_ = AddChildView(std::make_unique<ImageView>());
bool show_check_radio_icon = bool show_check_radio_icon =
type_ == Type::kRadio || (type_ == Type::kCheckbox && type_ == Type::kRadio || (type_ == Type::kCheckbox && GetDelegate() &&
GetDelegate()->IsItemChecked(GetCommand())); GetDelegate()->IsItemChecked(GetCommand()));
radio_check_image_view_->SetVisible(show_check_radio_icon); radio_check_image_view_->SetVisible(show_check_radio_icon);
radio_check_image_view_->set_can_process_events_within_subtree(false); radio_check_image_view_->set_can_process_events_within_subtree(false);
...@@ -943,7 +946,6 @@ void MenuItemView::PaintButton(gfx::Canvas* canvas, PaintButtonMode mode) { ...@@ -943,7 +946,6 @@ void MenuItemView::PaintButton(gfx::Canvas* canvas, PaintButtonMode mode) {
if (forced_visual_selection_.has_value()) if (forced_visual_selection_.has_value())
render_selection = *forced_visual_selection_; render_selection = *forced_visual_selection_;
MenuDelegate* delegate = GetDelegate();
// Render the background. As MenuScrollViewContainer draws the background, we // Render the background. As MenuScrollViewContainer draws the background, we
// only need the background when we want it to look different, as when we're // only need the background when we want it to look different, as when we're
// selected. // selected.
...@@ -966,10 +968,12 @@ void MenuItemView::PaintButton(gfx::Canvas* canvas, PaintButtonMode mode) { ...@@ -966,10 +968,12 @@ void MenuItemView::PaintButton(gfx::Canvas* canvas, PaintButtonMode mode) {
top_margin += (available_height - total_text_height) / 2; top_margin += (available_height - total_text_height) / 2;
// Render the check. // Render the check.
if (type_ == Type::kCheckbox && delegate->IsItemChecked(GetCommand())) { MenuDelegate* delegate = GetDelegate();
if (type_ == Type::kCheckbox && delegate &&
delegate->IsItemChecked(GetCommand())) {
radio_check_image_view_->SetImage(GetMenuCheckImage(icon_color)); radio_check_image_view_->SetImage(GetMenuCheckImage(icon_color));
} else if (type_ == Type::kRadio) { } else if (type_ == Type::kRadio) {
const bool toggled = delegate->IsItemChecked(GetCommand()); const bool toggled = delegate && delegate->IsItemChecked(GetCommand());
const gfx::VectorIcon& radio_icon = const gfx::VectorIcon& radio_icon =
toggled ? kMenuRadioSelectedIcon : kMenuRadioEmptyIcon; toggled ? kMenuRadioSelectedIcon : kMenuRadioEmptyIcon;
const SkColor radio_icon_color = GetNativeTheme()->GetSystemColor( const SkColor radio_icon_color = GetNativeTheme()->GetSystemColor(
......
...@@ -117,7 +117,7 @@ class VIEWS_EXPORT MenuItemView : public View { ...@@ -117,7 +117,7 @@ class VIEWS_EXPORT MenuItemView : public View {
// Constructor for use with the top level menu item. This menu is never // Constructor for use with the top level menu item. This menu is never
// shown to the user, rather its use as the parent for all menu items. // shown to the user, rather its use as the parent for all menu items.
explicit MenuItemView(MenuDelegate* delegate); explicit MenuItemView(MenuDelegate* delegate = nullptr);
// Overridden from View: // Overridden from View:
base::string16 GetTooltipText(const gfx::Point& p) const override; base::string16 GetTooltipText(const gfx::Point& p) const override;
......
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
namespace views { namespace views {
TEST(SubmenuViewTest, GetLastItem) { TEST(SubmenuViewTest, GetLastItem) {
MenuItemView* parent = new MenuItemView(nullptr); MenuItemView* parent = new MenuItemView();
MenuRunner menu_runner(parent, 0); MenuRunner menu_runner(parent, 0);
SubmenuView* submenu = parent->CreateSubmenu(); SubmenuView* submenu = parent->CreateSubmenu();
...@@ -20,14 +20,14 @@ TEST(SubmenuViewTest, GetLastItem) { ...@@ -20,14 +20,14 @@ TEST(SubmenuViewTest, GetLastItem) {
submenu->AddChildView(new View()); submenu->AddChildView(new View());
EXPECT_EQ(nullptr, submenu->GetLastItem()); EXPECT_EQ(nullptr, submenu->GetLastItem());
MenuItemView* first = new MenuItemView(nullptr); MenuItemView* first = new MenuItemView();
submenu->AddChildView(first); submenu->AddChildView(first);
EXPECT_EQ(first, submenu->GetLastItem()); EXPECT_EQ(first, submenu->GetLastItem());
submenu->AddChildView(new View()); submenu->AddChildView(new View());
EXPECT_EQ(first, submenu->GetLastItem()); EXPECT_EQ(first, submenu->GetLastItem());
MenuItemView* second = new MenuItemView(nullptr); MenuItemView* second = new MenuItemView();
submenu->AddChildView(second); submenu->AddChildView(second);
EXPECT_EQ(second, submenu->GetLastItem()); EXPECT_EQ(second, submenu->GetLastItem());
} }
......
...@@ -6,7 +6,7 @@ ...@@ -6,7 +6,7 @@
namespace views { namespace views {
TestMenuItemView::TestMenuItemView() : MenuItemView(nullptr) {} TestMenuItemView::TestMenuItemView() = default;
TestMenuItemView::TestMenuItemView(MenuDelegate* delegate) TestMenuItemView::TestMenuItemView(MenuDelegate* delegate)
: MenuItemView(delegate) {} : MenuItemView(delegate) {}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment