Two pieces of cleanup for ui/views/controls/.

(1) Link() calls Init() unconditionally from its constructor and nowhere else.
    Inline this.
(2) No one calls MessageBoxView::SetLink() with empty |text|.  Remove the
    handling for that case and add DCHECKs.

Bug: none
Change-Id: Id5d95010d8c06340d30707db42515f4769d2cba5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1983431
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Peter Boström <pbos@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#728140}

ui/views/controls/link.cc

@@ -14,7 +14,6 @@
 #include "ui/events/event.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 #include "ui/gfx/canvas.h"
-#include "ui/gfx/color_palette.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/font_list.h"
 #include "ui/native_theme/native_theme.h"
@@ -28,25 +27,23 @@ namespace views {
 constexpr gfx::Insets Link::kFocusBorderPadding;
 
 Link::Link(const base::string16& title, int text_context, int text_style)
-    : Label(title, text_context, text_style),
-      requested_enabled_color_(gfx::kPlaceholderColor),
-      requested_enabled_color_set_(false) {
-  Init();
-}
+    : Label(title, text_context, text_style) {
+  RecalculateFont();
 
-Link::~Link() = default;
+  enabled_changed_subscription_ = AddEnabledChangedCallback(
+      base::BindRepeating(&Link::RecalculateFont, base::Unretained(this)));
 
-// static
-Link::FocusStyle Link::GetDefaultFocusStyle() {
-  return FocusStyle::kUnderline;
+  // Label() indirectly calls SetText(), but at that point our virtual override
+  // will not be reached.  Call it explicitly here to configure focus.
+  SetText(GetText());
 }
 
-Link::FocusStyle Link::GetFocusStyle() const {
-  // Use the default, unless the link would "always" be underlined.
-  if (underline_ && GetDefaultFocusStyle() == FocusStyle::kUnderline)
-    return FocusStyle::kRing;
+Link::~Link() = default;
 
-  return GetDefaultFocusStyle();
+Link::FocusStyle Link::GetFocusStyle() const {
+  // Use an underline to indicate focus unless the link is always drawn with an
+  // underline.
+  return underline_ ? FocusStyle::kRing : FocusStyle::kUnderline;
 }
 
 SkColor Link::GetColor() const {
@@ -56,8 +53,8 @@ SkColor Link::GetColor() const {
   if (!GetEnabled())
     return theme->GetSystemColor(ui::NativeTheme::kColorId_LinkDisabled);
 
-  if (requested_enabled_color_set_)
-    return requested_enabled_color_;
+  if (requested_enabled_color_.has_value())
+    return requested_enabled_color_.value();
 
   return GetNativeTheme()->GetSystemColor(
       pressed_ ? ui::NativeTheme::kColorId_LinkPressed
@@ -210,7 +207,6 @@ void Link::OnThemeChanged() {
 }
 
 void Link::SetEnabledColor(SkColor color) {
-  requested_enabled_color_set_ = true;
   requested_enabled_color_ = color;
   Label::SetEnabledColor(GetColor());
 }
@@ -231,22 +227,6 @@ void Link::SetUnderline(bool underline) {
   OnPropertyChanged(&underline_, kPropertyEffectsPreferredSizeChanged);
 }
 
-void Link::Init() {
-  listener_ = nullptr;
-  pressed_ = false;
-  underline_ = GetDefaultFocusStyle() != FocusStyle::kUnderline;
-  RecalculateFont();
-
-  enabled_changed_subscription_ = AddEnabledChangedCallback(
-      base::BindRepeating(&Link::RecalculateFont, base::Unretained(this)));
-
-  // Label::Init() calls SetText(), but if that's being called from Label(), our
-  // SetText() override will not be reached (because the constructed class is
-  // only a Label at the moment, not yet a Link).  So explicitly configure focus
-  // here.
-  ConfigureFocus();
-}
-
 void Link::SetPressed(bool pressed) {
   if (pressed_ != pressed) {
     pressed_ = pressed;

ui/views/controls/link.h

@@ -8,7 +8,9 @@
 #include <string>
 
 #include "base/macros.h"
+#include "base/optional.h"
 #include "third_party/skia/include/core/SkColor.h"
+#include "ui/gfx/color_palette.h"
 #include "ui/views/controls/label.h"
 #include "ui/views/style/typography.h"
 
@@ -43,10 +45,6 @@ class VIEWS_EXPORT Link : public Label {
                 int text_style = style::STYLE_LINK);
   ~Link() override;
 
-  // Returns the default FocusStyle for a views::Link. Calling SetUnderline()
-  // may change it: E.g. SetUnderline(true) forces FocusStyle::kRing.
-  static FocusStyle GetDefaultFocusStyle();
-
   // Returns the current FocusStyle of this Link.
   FocusStyle GetFocusStyle() const;
 
@@ -84,25 +82,22 @@ class VIEWS_EXPORT Link : public Label {
   void SetUnderline(bool underline);
 
  private:
-  void Init();
-
   void SetPressed(bool pressed);
 
   void RecalculateFont();
 
   void ConfigureFocus();
 
-  LinkListener* listener_;
+  LinkListener* listener_ = nullptr;
 
   // Whether the link should be underlined when enabled.
-  bool underline_;
+  bool underline_ = false;
 
   // Whether the link is currently pressed.
-  bool pressed_;
+  bool pressed_ = false;
 
   // The color when the link is neither pressed nor disabled.
-  SkColor requested_enabled_color_;
-  bool requested_enabled_color_set_;
+  base::Optional<SkColor> requested_enabled_color_;
 
   PropertyChangedSubscription enabled_changed_subscription_;
 

ui/views/controls/message_box_view.cc

@@ -119,24 +119,14 @@ void MessageBoxView::SetCheckBoxSelected(bool selected) {
 
 void MessageBoxView::SetLink(const base::string16& text,
                              LinkListener* listener) {
-  size_t child_count = children().size();
-  if (text.empty()) {
-    DCHECK(!listener);
-    delete link_;
-    link_ = nullptr;
-  } else {
+  DCHECK(!text.empty());
   DCHECK(listener);
-    if (!link_) {
-      // See the comment above in SetCheckBoxLabel();
+  DCHECK(!link_);
+  // See the comment in SetCheckBoxLabel();
   SetLayoutManager(nullptr);
   link_ = AddChildView(std::make_unique<Link>(text));
   link_->SetHorizontalAlignment(gfx::ALIGN_LEFT);
-    } else {
-      link_->SetText(text);
-    }
   link_->set_listener(listener);
-  }
-  if (child_count != children().size())
   ResetLayoutManager();
 }