Clean up/simplify StyledLabel, part 5.

* Replace map<View*, ... with map<Link*, ... since that more accurately
  captures the intent.  Add a type alias as well.
* Slight modifications to variable names and comments in advance of functional
  patch, to minimize its diff.

Bug: 1015717
Change-Id: If153e69129c2d66a9d12825bdaddfb7e9a1787f2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1884359
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710230}

ash/login/ui/login_expanded_public_account_view_unittest.cc

@@ -17,6 +17,7 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/events/test/event_generator.h"
+#include "ui/views/controls/link.h"
 #include "ui/views/layout/box_layout.h"
 #include "ui/views/widget/widget.h"
 
@@ -162,8 +163,8 @@ TEST_P(LoginExpandedPublicAccountViewTest, ShowWarningDialog) {
   EXPECT_EQ(styled_label_test.link_targets().size(), 1U);
 
   // Tap on the learn more link.
-  views::View* link_view = styled_label_test.link_targets().begin()->first;
-  TapOnView(link_view);
+  views::Link* link = styled_label_test.link_targets().begin()->first;
+  TapOnView(link);
   EXPECT_NE(test_api.warning_dialog(), nullptr);
   EXPECT_TRUE(test_api.warning_dialog()->GetVisible());
 

ui/views/controls/styled_label.cc

@@ -27,14 +27,14 @@ StyledLabel::TestApi::TestApi(StyledLabel* view) : view_(view) {}
 
 StyledLabel::TestApi::~TestApi() = default;
 
-const std::map<View*, gfx::Range>& StyledLabel::TestApi::link_targets() {
+const StyledLabel::LinkTargets& StyledLabel::TestApi::link_targets() {
   return view_->link_targets_;
 }
 
 StyledLabel::RangeStyleInfo::RangeStyleInfo() = default;
-StyledLabel::RangeStyleInfo::RangeStyleInfo(const RangeStyleInfo& copy) =
-    default;
-
+StyledLabel::RangeStyleInfo::RangeStyleInfo(const RangeStyleInfo&) = default;
+StyledLabel::RangeStyleInfo& StyledLabel::RangeStyleInfo::operator=(
+    const RangeStyleInfo&) = default;
 StyledLabel::RangeStyleInfo::~RangeStyleInfo() = default;
 
 // static
@@ -268,7 +268,7 @@ gfx::Size StyledLabel::CalculateAndDoLayout(int width, bool dry_run) {
     width_at_last_layout_ = width;
 
   const gfx::Insets insets = GetInsets();
-  width -= insets.width();
+  const int content_width = width - insets.width();
 
   if (!dry_run) {
     RemoveAllChildViews(true);
@@ -279,18 +279,16 @@ gfx::Size StyledLabel::CalculateAndDoLayout(int width, bool dry_run) {
 
   // The current child view's position, relative to content bounds, in pixels.
   gfx::Point offset(0, insets.top());
-  int total_height = 0;
-  // The max width of all lines. Guaranteed to be no larger than |width|.
-  int max_width = 0;
 
   RangeStyleInfo default_style;
   default_style.text_style = default_text_style_;
+  int max_width = 0, total_height = 0;
 
   // Try to preserve leading whitespace on the first line.
   bool can_trim_leading_whitespace = false;
   StyleRanges::const_iterator current_range = style_ranges_.begin();
   for (base::string16 remaining_string = text_;
-       width > 0 && !remaining_string.empty();) {
+       content_width > 0 && !remaining_string.empty();) {
     // Max height of the views in a line.
     int line_height = default_line_height;
 
@@ -299,7 +297,7 @@ gfx::Size StyledLabel::CalculateAndDoLayout(int width, bool dry_run) {
 
     while (!remaining_string.empty()) {
       if (offset.x() == 0 && can_trim_leading_whitespace) {
-        if (remaining_string.front() == L'\n') {
+        if (remaining_string.front() == '\n') {
           // Wrapped to the next line on \n, remove it. Other whitespace,
           // e.g. spaces to indent the next line, are preserved.
           remaining_string.erase(0, 1);
@@ -324,7 +322,7 @@ gfx::Size StyledLabel::CalculateAndDoLayout(int width, bool dry_run) {
       if (position != range.start() ||
           (current_range != style_ranges_.end() &&
            !current_range->style_info.custom_view)) {
-        const gfx::Rect chunk_bounds(offset.x(), 0, width - offset.x(),
+        const gfx::Rect chunk_bounds(offset.x(), 0, content_width - offset.x(),
                                      default_line_height);
         // If the start of the remaining text is inside a styled range, the font
         // style may differ from the base font. The font specified by the range
@@ -392,7 +390,8 @@ gfx::Size StyledLabel::CalculateAndDoLayout(int width, bool dry_run) {
         }
 
         if (((custom_view &&
-              offset.x() + custom_view->GetPreferredSize().width() > width) ||
+              offset.x() + custom_view->GetPreferredSize().width() >
+                  content_width) ||
              (style_info.disable_line_wrapping &&
               chunk.size() < range.length())) &&
             position == range.start() && offset.x() != 0) {
@@ -407,8 +406,9 @@ gfx::Size StyledLabel::CalculateAndDoLayout(int width, bool dry_run) {
         if (!custom_view) {
           label = CreateLabel(chunk, style_info);
           if (style_info.IsLink() && !dry_run) {
-            static_cast<Link*>(label.get())->set_listener(this);
-            link_targets_[label.get()] = range;
+            Link* link = static_cast<Link*>(label.get());
+            link->set_listener(this);
+            link_targets_[link] = range;
           }
         }
 
@@ -424,13 +424,14 @@ gfx::Size StyledLabel::CalculateAndDoLayout(int width, bool dry_run) {
       }
 
       View* child_view = custom_view ? custom_view : label.get();
-      gfx::Size size = child_view->GetPreferredSize();
+      gfx::Size child_size = child_view->GetPreferredSize();
       // A custom view could be wider than the available width.
-      size.set_width(std::min(size.width(), width - offset.x()));
+      child_size.set_width(
+          std::min(child_size.width(), content_width - offset.x()));
 
-      child_view->SetBoundsRect(gfx::Rect(offset, size));
-      offset.set_x(offset.x() + size.width());
-      line_height = std::max(line_height, size.height());
+      child_view->SetBoundsRect(gfx::Rect(offset, child_size));
+      offset.set_x(offset.x() + child_size.width());
+      line_height = std::max(line_height, child_size.height());
 
       if (!dry_run) {
         views_in_a_line.push_back(child_view);
@@ -454,11 +455,12 @@ gfx::Size StyledLabel::CalculateAndDoLayout(int width, bool dry_run) {
     }
     total_height += line_height;
     max_width = std::max(max_width, offset.x());
+
     // Trim whitespace at the start of the next line.
     can_trim_leading_whitespace = true;
 
     // Adjust the positions of the views in the line.
-    const int start_x = StartX(width - offset.x());
+    const int start_x = StartX(content_width - offset.x());
     for (auto* view : views_in_a_line) {
       view->SetPosition({start_x + view->x(),
                          offset.y() + (line_height - view->height()) / 2.0f});
@@ -468,9 +470,6 @@ gfx::Size StyledLabel::CalculateAndDoLayout(int width, bool dry_run) {
     offset = gfx::Point(0, offset.y() + line_height);
   }
 
-  // |width| may be negative if the value passed in was smaller than
-  // insets.width().
-  DCHECK_LE(max_width, std::max(width, 0));
   calculated_size_ =
       gfx::Size(max_width + insets.width(), total_height + insets.height());
   return calculated_size_;
@@ -478,7 +477,7 @@ gfx::Size StyledLabel::CalculateAndDoLayout(int width, bool dry_run) {
 
 std::unique_ptr<Label> StyledLabel::CreateLabel(
     const base::string16& text,
-    const StyledLabel::RangeStyleInfo& style_info) const {
+    const RangeStyleInfo& style_info) const {
   std::unique_ptr<Label> result;
   if (style_info.IsLink()) {
     // Nothing should (and nothing does) use a custom font for links.

ui/views/controls/styled_label.h

@@ -38,13 +38,15 @@ class VIEWS_EXPORT StyledLabel : public View, public LinkListener {
  public:
   METADATA_HEADER(StyledLabel);
 
+  using LinkTargets = std::map<Link*, gfx::Range>;
+
   // TestApi is used for tests to get internal implementation details.
   class VIEWS_EXPORT TestApi {
    public:
     explicit TestApi(StyledLabel* view);
     ~TestApi();
 
-    const std::map<View*, gfx::Range>& link_targets();
+    const LinkTargets& link_targets();
 
    private:
     StyledLabel* const view_;
@@ -55,7 +57,8 @@ class VIEWS_EXPORT StyledLabel : public View, public LinkListener {
   // Parameters that define label style for a styled label's text range.
   struct VIEWS_EXPORT RangeStyleInfo {
     RangeStyleInfo();
-    RangeStyleInfo(const RangeStyleInfo& copy);
+    RangeStyleInfo(const RangeStyleInfo&);
+    RangeStyleInfo& operator=(const RangeStyleInfo&);
     ~RangeStyleInfo();
 
     // Creates a range style info with default values for link.
@@ -210,7 +213,7 @@ class VIEWS_EXPORT StyledLabel : public View, public LinkListener {
 
   // A mapping from a view to the range it corresponds to in |text_|. Only views
   // that correspond to ranges with is_link style set will be added to the map.
-  std::map<View*, gfx::Range> link_targets_;
+  LinkTargets link_targets_;
 
   // Owns the custom views used to replace ranges of text with icons, etc.
   std::set<std::unique_ptr<View>> custom_views_;