Make menu separator default constructible

Change the type of a menu separator to a property which can be set
separately. Also add some basic unit tests for this class.

Bug: 1108460
Change-Id: I7201e2b22c56e0b5efa366758bd217bba15ede10
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2314658
Commit-Queue: Wei Li <weili@chromium.org>
Reviewed-by: Michael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791918}

ui/views/BUILD.gn

@@ -1071,6 +1071,7 @@ test("views_unittests") {
     "controls/menu/menu_item_view_unittest.cc",
     "controls/menu/menu_model_adapter_unittest.cc",
     "controls/menu/menu_runner_unittest.cc",
+    "controls/menu/menu_separator_unittest.cc",
     "controls/menu/submenu_view_unittest.cc",
     "controls/menu/test_menu_item_view.cc",
     "controls/menu/test_menu_item_view.h",

ui/views/controls/menu/menu_separator.cc

@@ -88,8 +88,21 @@ gfx::Size MenuSeparator::CalculatePreferredSize() const {
                    height);
 }
 
+ui::MenuSeparatorType MenuSeparator::GetType() const {
+  return type_;
+}
+
+void MenuSeparator::SetType(ui::MenuSeparatorType type) {
+  if (type_ == type)
+    return;
+
+  type_ = type;
+  OnPropertyChanged(&type_, kPropertyEffectsPreferredSizeChanged);
+}
+
 BEGIN_METADATA(MenuSeparator)
 METADATA_PARENT_CLASS(View)
+ADD_PROPERTY_METADATA(MenuSeparator, ui::MenuSeparatorType, Type)
 END_METADATA()
 
 }  // namespace views

ui/views/controls/menu/menu_separator.h

@@ -8,6 +8,7 @@
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "ui/base/models/menu_separator_types.h"
+#include "ui/views/metadata/metadata_header_macros.h"
 #include "ui/views/view.h"
 #include "ui/views/views_export.h"
 
@@ -17,15 +18,20 @@ class VIEWS_EXPORT MenuSeparator : public View {
  public:
   METADATA_HEADER(MenuSeparator);
 
-  explicit MenuSeparator(ui::MenuSeparatorType type) : type_(type) {}
+  explicit MenuSeparator(
+      ui::MenuSeparatorType type = ui::MenuSeparatorType::NORMAL_SEPARATOR)
+      : type_(type) {}
 
   // View overrides.
   void OnPaint(gfx::Canvas* canvas) override;
   gfx::Size CalculatePreferredSize() const override;
 
+  ui::MenuSeparatorType GetType() const;
+  void SetType(ui::MenuSeparatorType type);
+
  private:
   // The type of the separator.
-  const ui::MenuSeparatorType type_;
+  ui::MenuSeparatorType type_;
 
   DISALLOW_COPY_AND_ASSIGN(MenuSeparator);
 };

ui/views/controls/menu/menu_separator_unittest.cc

+// Copyright 2020 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/menu/menu_separator.h"
+
+#include <memory>
+
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/models/menu_separator_types.h"
+#include "ui/views/controls/menu/menu_config.h"
+#include "ui/views/test/view_metadata_test_utils.h"
+#include "ui/views/test/views_test_base.h"
+
+namespace views {
+
+using MenuSeparatorTest = ViewsTestBase;
+
+TEST_F(MenuSeparatorTest, Metadata) {
+  auto separator = std::make_unique<MenuSeparator>();
+  test::TestViewMetadata(separator.get());
+}
+
+TEST_F(MenuSeparatorTest, TypeChangeEffect) {
+  auto separator = std::make_unique<MenuSeparator>();
+  separator->SizeToPreferredSize();
+  const MenuConfig& config = MenuConfig::instance();
+  EXPECT_EQ(config.separator_height, separator->height());
+
+  separator->SetType(ui::MenuSeparatorType::DOUBLE_SEPARATOR);
+  separator->SizeToPreferredSize();
+  EXPECT_EQ(config.double_separator_height, separator->height());
+}
+
+}  // namespace views

ui/views/metadata/type_conversion.cc

@@ -317,6 +317,22 @@ DEFINE_ENUM_CONVERTERS(ui::TextInputType,
                        {ui::TextInputType::TEXT_INPUT_TYPE_MAX,
                         base::ASCIIToUTF16("TEXT_INPUT_TYPE_MAX")})
 
+DEFINE_ENUM_CONVERTERS(ui::MenuSeparatorType,
+                       {ui::MenuSeparatorType::NORMAL_SEPARATOR,
+                        base::ASCIIToUTF16("NORMAL_SEPARATOR")},
+                       {ui::MenuSeparatorType::DOUBLE_SEPARATOR,
+                        base::ASCIIToUTF16("DOUBLE_SEPARATOR")},
+                       {ui::MenuSeparatorType::UPPER_SEPARATOR,
+                        base::ASCIIToUTF16("UPPER_SEPARATOR")},
+                       {ui::MenuSeparatorType::LOWER_SEPARATOR,
+                        base::ASCIIToUTF16("LOWER_SEPARATOR")},
+                       {ui::MenuSeparatorType::SPACING_SEPARATOR,
+                        base::ASCIIToUTF16("SPACING_SEPARATOR")},
+                       {ui::MenuSeparatorType::VERTICAL_SEPARATOR,
+                        base::ASCIIToUTF16("VERTICAL_SEPARATOR")},
+                       {ui::MenuSeparatorType::PADDED_SEPARATOR,
+                        base::ASCIIToUTF16("PADDED_SEPARATOR")})
+
 #define OP(enum_name) \
   { ui::NativeTheme::enum_name, base::ASCIIToUTF16(#enum_name) }
 DEFINE_ENUM_CONVERTERS(ui::NativeTheme::ColorId, NATIVE_THEME_COLOR_IDS)