views::Separator cleanup.

ui/views/controls/separator.h

 // Copyright (c) 2011 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_SEPARATOR_H_
 #define UI_VIEWS_CONTROLS_SEPARATOR_H_
 
 #include <string>
 
 #include "base/macros.h"
+#include "base/optional.h"
 #include "ui/views/view.h"
 
 namespace views {
 
 // The Separator class is a view that shows a line used to visually separate
 // other views.
 
 class VIEWS_EXPORT Separator : public View {
  public:
-  enum Orientation {
-    HORIZONTAL,
-    VERTICAL
-  };
-
   // The separator's class name.
   static const char kViewClassName[];
 
-  explicit Separator(Orientation orientation);
+  // The separator's thickness in dip.

[sky, 2017/02/09 03:16:09]

optional: Generally values are assumed to be dip unless otherwise stated, so I
wouldn't bother mentioning dip here.

[Evan Stade, 2017/02/09 15:46:11]

Yea, you're right, my desire for extra clarity here is because MD borders and
other lines are often a single pixel.

+  static const int kThickness;
+
+  // Constructs a separator that is ambivalent to orientation. To be used in the
+  // common case where sizing will be controlled by the layout manager.

[sky, 2017/02/09 03:16:09]

Please document the preferred size is kThickness in both directions.

[Evan Stade, 2017/02/09 15:46:11]

will do

+  Separator();
   ~Separator() override;
 
-  SkColor color() const { return color_; }
   void SetColor(SkColor color);
 
-  int size() const { return size_; }
-  // Preferred size of one axis: height for horizontal separator
-  // and width for vertical separator
-  void SetPreferredSize(int size);
+  void SetPreferredHeight(int height);
+
+  void SetPreferredWidth(int width);
 
   // Overridden from View:
   gfx::Size GetPreferredSize() const override;
   void GetAccessibleNodeData(ui::AXNodeData* node_data) override;
   void OnPaint(gfx::Canvas* canvas) override;
-  void OnNativeThemeChanged(const ui::NativeTheme* theme) override;
   const char* GetClassName() const override;
 
  private:
-  void SetColorFromNativeTheme();
-
-  const Orientation orientation_;
-  SkColor color_;
-  bool color_overridden_;
-  int size_;
+  gfx::Size preferred_size_;
+  base::Optional<SkColor> overridden_color_;
 
   DISALLOW_COPY_AND_ASSIGN(Separator);
 };
 
 }  // namespace views
 
 #endif  // UI_VIEWS_CONTROLS_SEPARATOR_H_