Add support to view builders for 'root' views. Update FindBarView using this.

Bug: 938501, 1130078
Change-Id: I50e6c35c97b1d25250bb45d2341e49602e86c29c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2498829
Commit-Queue: Allen Bauer <kylixrd@chromium.org>
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Cr-Commit-Position: refs/heads/master@{#821001}

chrome/browser/ui/views/find_bar_view.cc

@@ -128,70 +128,6 @@ END_METADATA
 // FindBarView, public:
 
 FindBarView::FindBarView(FindBarHost* host) {
-  auto find_text =
-      views::Builder<views::Textfield>()
-          .SetAccessibleName(l10n_util::GetStringUTF16(IDS_ACCNAME_FIND))
-          .SetBorder(views::NullBorder())
-          .SetDefaultWidthInChars(30)
-          .SetID(VIEW_ID_FIND_IN_PAGE_TEXT_FIELD)
-          .SetMinimumWidthInChars(1)
-          .SetTextInputFlags(ui::TEXT_INPUT_FLAG_AUTOCORRECT_OFF)
-          .Build();
-
-  // TODO(kerenzhu): set_controller() should be supported in Builder<TextField>.
-  find_text->set_controller(this);
-
-  find_text_ = AddChildView(std::move(find_text));
-
-  SetHost(host);
-
-  auto match_count_text = views::Builder<FindBarMatchCountLabel>()
-                              .SetCanProcessEventsWithinSubtree(false)
-                              .Build();
-  match_count_text_ = AddChildView(std::move(match_count_text));
-
-  auto separator = views::Builder<views::Separator>()
-                       .SetCanProcessEventsWithinSubtree(false)
-                       .Build();
-  separator_ = AddChildView(std::move(separator));
-
-  auto find_previous_button =
-      views::Builder<views::ImageButton>()
-          .SetAccessibleName(l10n_util::GetStringUTF16(IDS_ACCNAME_PREVIOUS))
-          .SetID(VIEW_ID_FIND_IN_PAGE_PREVIOUS_BUTTON)
-          .SetTooltipText(
-              l10n_util::GetStringUTF16(IDS_FIND_IN_PAGE_PREVIOUS_TOOLTIP))
-          .Build();
-  find_previous_button->SetCallback(base::BindRepeating(
-      &FindBarView::FindNext, base::Unretained(this), true));
-  find_previous_button_ = AddChildView(std::move(find_previous_button));
-  SetCommonButtonAttributes(find_previous_button_);
-
-  auto find_next_button =
-      views::Builder<views::ImageButton>()
-          .SetID(VIEW_ID_FIND_IN_PAGE_NEXT_BUTTON)
-          .SetTooltipText(
-              l10n_util::GetStringUTF16(IDS_FIND_IN_PAGE_NEXT_TOOLTIP))
-          .SetAccessibleName(l10n_util::GetStringUTF16(IDS_ACCNAME_NEXT))
-          .Build();
-  find_next_button->SetCallback(base::BindRepeating(
-      &FindBarView::FindNext, base::Unretained(this), false));
-  find_next_button_ = AddChildView(std::move(find_next_button));
-  SetCommonButtonAttributes(find_next_button_);
-
-  auto close_button = views::Builder<views::ImageButton>()
-                          .SetID(VIEW_ID_FIND_IN_PAGE_CLOSE_BUTTON)
-                          .SetTooltipText(l10n_util::GetStringUTF16(
-                              IDS_FIND_IN_PAGE_CLOSE_TOOLTIP))
-                          .SetAnimationDuration(base::TimeDelta())
-                          .Build();
-  close_button->SetCallback(base::BindRepeating(&FindBarView::EndFindSession,
-                                                base::Unretained(this)));
-  close_button_ = AddChildView(std::move(close_button));
-  SetCommonButtonAttributes(close_button_);
-
-  SetFlipCanvasOnPaintForRTLUI(true);
-
   // Normally we could space objects horizontally by simply passing a constant
   // value to BoxLayout for between-child spacing.  But for the vector image
   // buttons, we want the spacing to apply between the inner "glyph" portions
@@ -213,31 +149,74 @@ FindBarView::FindBarView(FindBarHost* host) {
       provider->GetDistanceMetric(DISTANCE_TOAST_CONTROL_VERTICAL), 0);
   const gfx::Insets toast_label_vertical_margin(
       provider->GetDistanceMetric(DISTANCE_TOAST_LABEL_VERTICAL), 0);
-  find_previous_button_->SetProperty(
-      views::kMarginsKey, gfx::Insets(toast_control_vertical_margin +
-                                      vector_button_horizontal_margin));
-  find_next_button_->SetProperty(views::kMarginsKey,
-                                 gfx::Insets(toast_control_vertical_margin +
-                                             vector_button_horizontal_margin));
-  close_button_->SetProperty(views::kMarginsKey,
-                             gfx::Insets(toast_control_vertical_margin +
-                                         vector_button_horizontal_margin));
-  separator_->SetProperty(
-      views::kMarginsKey,
-      gfx::Insets(toast_control_vertical_margin + horizontal_margin));
-  find_text_->SetProperty(
-      views::kMarginsKey,
-      gfx::Insets(toast_control_vertical_margin + horizontal_margin));
-  match_count_text_->SetProperty(
-      views::kMarginsKey,
-      gfx::Insets(toast_label_vertical_margin + horizontal_margin));
-
-  auto* manager = SetLayoutManager(std::make_unique<views::BoxLayout>(
-      views::BoxLayout::Orientation::kHorizontal,
-      gfx::Insets(provider->GetInsetsMetric(INSETS_TOAST) - horizontal_margin),
-      0));
-
-  manager->SetFlexForView(find_text_, 1, true);
+  const gfx::Insets image_button_margins(toast_control_vertical_margin +
+                                         vector_button_horizontal_margin);
+
+  views::Builder<FindBarView>(this)
+      .SetOrientation(views::BoxLayout::Orientation::kHorizontal)
+      .SetInsideBorderInsets(gfx::Insets(
+          provider->GetInsetsMetric(INSETS_TOAST) - horizontal_margin))
+      .SetHost(host)
+      .SetFlipCanvasOnPaintForRTLUI(true)
+      .AddChildren(
+          {views::Builder<views::Textfield>()
+               .CopyAddressTo(&find_text_)
+               .SetAccessibleName(l10n_util::GetStringUTF16(IDS_ACCNAME_FIND))
+               .SetBorder(views::NullBorder())
+               .SetDefaultWidthInChars(30)
+               .SetID(VIEW_ID_FIND_IN_PAGE_TEXT_FIELD)
+               .SetMinimumWidthInChars(1)
+               .SetTextInputFlags(ui::TEXT_INPUT_FLAG_AUTOCORRECT_OFF)
+               .SetProperty(views::kMarginsKey,
+                            gfx::Insets(toast_control_vertical_margin +
+                                        horizontal_margin))
+               .SetController(this),
+           views::Builder<FindBarMatchCountLabel>()
+               .CopyAddressTo(&match_count_text_)
+               .SetCanProcessEventsWithinSubtree(false)
+               .SetProperty(views::kMarginsKey,
+                            gfx::Insets(toast_label_vertical_margin +
+                                        horizontal_margin)),
+           views::Builder<views::Separator>()
+               .CopyAddressTo(&separator_)
+               .SetCanProcessEventsWithinSubtree(false)
+               .SetProperty(views::kMarginsKey,
+                            gfx::Insets(toast_control_vertical_margin +
+                                        horizontal_margin)),
+           views::Builder<views::ImageButton>()
+               .CopyAddressTo(&find_previous_button_)
+               .SetAccessibleName(
+                   l10n_util::GetStringUTF16(IDS_ACCNAME_PREVIOUS))
+               .SetID(VIEW_ID_FIND_IN_PAGE_PREVIOUS_BUTTON)
+               .SetTooltipText(
+                   l10n_util::GetStringUTF16(IDS_FIND_IN_PAGE_PREVIOUS_TOOLTIP))
+               .SetCallback(base::BindRepeating(&FindBarView::FindNext,
+                                                base::Unretained(this), true))
+               .SetProperty(views::kMarginsKey, image_button_margins),
+           views::Builder<views::ImageButton>()
+               .CopyAddressTo(&find_next_button_)
+               .SetAccessibleName(l10n_util::GetStringUTF16(IDS_ACCNAME_NEXT))
+               .SetID(VIEW_ID_FIND_IN_PAGE_NEXT_BUTTON)
+               .SetTooltipText(
+                   l10n_util::GetStringUTF16(IDS_FIND_IN_PAGE_NEXT_TOOLTIP))
+               .SetCallback(base::BindRepeating(&FindBarView::FindNext,
+                                                base::Unretained(this), false))
+               .SetProperty(views::kMarginsKey, image_button_margins),
+           views::Builder<views::ImageButton>()
+               .CopyAddressTo(&close_button_)
+               .SetID(VIEW_ID_FIND_IN_PAGE_CLOSE_BUTTON)
+               .SetTooltipText(
+                   l10n_util::GetStringUTF16(IDS_FIND_IN_PAGE_CLOSE_TOOLTIP))
+               .SetAnimationDuration(base::TimeDelta())
+               .SetCallback(base::BindRepeating(&FindBarView::EndFindSession,
+                                                base::Unretained(this)))
+               .SetProperty(views::kMarginsKey, image_button_margins)})
+      .BuildChildren();
+
+  SetFlexForView(find_text_, 1, true);
+  SetCommonButtonAttributes(find_previous_button_);
+  SetCommonButtonAttributes(find_next_button_);
+  SetCommonButtonAttributes(close_button_);
 }
 
 FindBarView::~FindBarView() {

chrome/browser/ui/views/find_bar_view.h

@@ -14,8 +14,9 @@
 #include "chrome/browser/ui/views/dropdown_bar_host_delegate.h"
 #include "ui/views/controls/button/button.h"
 #include "ui/views/controls/button/image_button.h"
+#include "ui/views/controls/textfield/textfield.h"
 #include "ui/views/controls/textfield/textfield_controller.h"
-#include "ui/views/view.h"
+#include "ui/views/layout/box_layout_view.h"
 
 class FindBarHost;
 class FindBarMatchCountLabel;
@@ -41,7 +42,7 @@ class Textfield;
 // button. It communicates the user search words to the FindBarHost.
 //
 ////////////////////////////////////////////////////////////////////////////////
-class FindBarView : public views::View,
+class FindBarView : public views::BoxLayoutView,
                     public DropdownBarHostDelegate,
                     public views::TextfieldController {
  public:
@@ -122,7 +123,7 @@ class FindBarView : public views::View,
   DISALLOW_COPY_AND_ASSIGN(FindBarView);
 };
 
-BEGIN_VIEW_BUILDER(/* no export */, FindBarView, views::View)
+BEGIN_VIEW_BUILDER(/* no export */, FindBarView, views::BoxLayoutView)
 VIEW_BUILDER_PROPERTY(FindBarHost*, Host)
 END_VIEW_BUILDER(/* no export */, FindBarView)
 

ui/views/controls/textfield/textfield.cc

@@ -383,6 +383,10 @@ void Textfield::SetAssociatedLabel(View* labelling_view) {
       node_data.GetString16Attribute(ax::mojom::StringAttribute::kName));
 }
 
+void Textfield::SetController(TextfieldController* controller) {
+  controller_ = controller;
+}
+
 bool Textfield::GetReadOnly() const {
   return read_only_;
 }

ui/views/controls/textfield/textfield.h

@@ -91,6 +91,8 @@ class VIEWS_EXPORT Textfield : public View,
   static const gfx::FontList& GetDefaultFontList();
 
   Textfield();
+  Textfield(const Textfield&) = delete;
+  Textfield& operator=(const Textfield&) = delete;
   ~Textfield() override;
 
   // Set the controller for this textfield.
@@ -98,6 +100,9 @@ class VIEWS_EXPORT Textfield : public View,
     controller_ = controller;
   }
 
+  // TODD (kylixrd): Remove set_controller and refactor codebase.
+  void SetController(TextfieldController* controller);
+
   // Gets/Sets whether or not the Textfield is read-only.
   bool GetReadOnly() const;
   void SetReadOnly(bool read_only);
@@ -734,13 +739,12 @@ class VIEWS_EXPORT Textfield : public View,
 
   // Used to bind callback functions to this object.
   base::WeakPtrFactory<Textfield> weak_ptr_factory_{this};
-
-  DISALLOW_COPY_AND_ASSIGN(Textfield);
 };
 
 BEGIN_VIEW_BUILDER(VIEWS_EXPORT, Textfield, View)
 VIEW_BUILDER_PROPERTY(base::string16, AccessibleName)
 VIEW_BUILDER_PROPERTY(SkColor, BackgroundColor)
+VIEW_BUILDER_PROPERTY(TextfieldController*, Controller)
 VIEW_BUILDER_PROPERTY(bool, CursorEnabled)
 VIEW_BUILDER_PROPERTY(int, DefaultWidthInChars)
 VIEW_BUILDER_PROPERTY(gfx::HorizontalAlignment, HorizontalAlignment)

ui/views/metadata/view_factory.h

@@ -23,12 +23,13 @@ class BaseViewBuilderT : public internal::ViewBuilderCore {
  public:
   using ViewClass_ = typename internal::ViewClassTrait<Builder>::ViewClass_;
   BaseViewBuilderT() { view_ = std::make_unique<ViewClass_>(); }
+  explicit BaseViewBuilderT(ViewClass_* root_view) : root_view_(root_view) {}
   BaseViewBuilderT(BaseViewBuilderT&&) = default;
   BaseViewBuilderT& operator=(BaseViewBuilderT&&) = default;
   ~BaseViewBuilderT() override = default;
 
   Builder& CopyAddressTo(ViewClass_** view_address) {
-    *view_address = view_.get();
+    *view_address = view_ ? view_.get() : root_view_;
     return *static_cast<Builder*>(this);
   }
 
@@ -46,16 +47,41 @@ class BaseViewBuilderT : public internal::ViewBuilderCore {
   }
 
   std::unique_ptr<ViewClass_> Build() {
+    DCHECK(view_);
     SetProperties(view_.get());
     CreateChildren(view_.get());
     return std::move(view_);
   }
 
+  void BuildChildren() {
+    DCHECK(root_view_);
+    SetProperties(root_view_);
+    CreateChildren(root_view_);
+  }
+
   template <typename T>
   Builder& SetProperty(const ui::ClassProperty<T>* property,
                        metadata::ArgType<T> value) {
     auto setter =
-        std::make_unique<internal::ClassPropertySetter<ViewClass_, T>>(
+        std::make_unique<internal::ClassPropertyValueSetter<ViewClass_, T>>(
+            property, value);
+    internal::ViewBuilderCore::AddPropertySetter(std::move(setter));
+    return *static_cast<Builder*>(this);
+  }
+
+  template <typename T>
+  Builder& SetProperty(const ui::ClassProperty<T*>* property, const T& value) {
+    auto setter =
+        std::make_unique<internal::ClassPropertyMoveSetter<ViewClass_, T>>(
+            property, value);
+    internal::ViewBuilderCore::AddPropertySetter(std::move(setter));
+    return *static_cast<Builder*>(this);
+  }
+
+  template <typename T>
+  Builder& SetProperty(const ui::ClassProperty<T*>* property, T&& value) {
+    auto setter =
+        std::make_unique<internal::ClassPropertyMoveSetter<ViewClass_, T>>(
             property, std::move(value));
     internal::ViewBuilderCore::AddPropertySetter(std::move(setter));
     return *static_cast<Builder*>(this);
@@ -67,6 +93,10 @@ class BaseViewBuilderT : public internal::ViewBuilderCore {
   // Owned and meaningful during the Builder building process. Its
   // ownership will be transferred out upon Build() call.
   std::unique_ptr<ViewClass_> view_;
+
+  // Unowned root view. Used for creating a builder with an existing root
+  // instance.
+  ViewClass_* root_view_ = nullptr;
 };
 
 // Example of builder class generated by the following macros.
@@ -115,8 +145,8 @@ class BaseViewBuilderT : public internal::ViewBuilderCore {
 //   }
 // };
 //
-// class VIEWS_EXPORT LabelButtonBuilderTest
-//     : public LabelButtonBuilderT<LabelButtonBuilderTest, LabelButton> {};
+// class VIEWS_EXPORT LabelButtonBuilder
+//     : public LabelButtonBuilderT<LabelButtonBuilder, LabelButton> {};
 
 #define BEGIN_VIEW_BUILDER(export, view_class, ancestor)                    \
   template <typename BuilderT>                                              \
@@ -125,6 +155,10 @@ class BaseViewBuilderT : public internal::ViewBuilderCore {
     using ViewClass_ = view_class;                                          \
                                                                             \
     view_class##BuilderT() = default;                                       \
+    explicit view_class##BuilderT(                                          \
+        typename ::views::internal::ViewClassTrait<BuilderT>::ViewClass_*   \
+            root_view)                                                      \
+        : ancestor##BuilderT<BuilderT>(root_view) {}                        \
     view_class##BuilderT(view_class##BuilderT&&) = default;                 \
     view_class##BuilderT& operator=(view_class##BuilderT&&) = default;      \
     ~view_class##BuilderT() override = default;
@@ -183,12 +217,20 @@ class BaseViewBuilderT : public internal::ViewBuilderCore {
 // semi-colon on a separate line.
 // clang-format off
 
-#define END_VIEW_BUILDER(export, view_class) \
-  };                                         \
-                                             \
-  template <>                                \
-  class export views::Builder<view_class>    \
-      : public view_class##BuilderT<views::Builder<view_class>>{};
+#define END_VIEW_BUILDER(export, view_class)                             \
+  };                                                                     \
+                                                                         \
+  template <>                                                            \
+  class export views::Builder<view_class>                                \
+      : public view_class##BuilderT<views::Builder<view_class>> {        \
+   public:                                                               \
+    Builder<view_class>() = default;                                     \
+    explicit Builder<view_class>(ViewClass_* root_view)                  \
+        : view_class##BuilderT<views::Builder<view_class>>(root_view) {} \
+    Builder<view_class>(Builder&&) = default;                            \
+    Builder<view_class>& operator=(Builder<view_class>&&) = default;     \
+    ~Builder<view_class>() = default;                                    \
+  };
 
 // clang-format on
 

ui/views/metadata/view_factory_internal.h

@@ -64,21 +64,43 @@ class PropertySetter : public PropertySetterBase {
 };
 
 template <typename TClass, typename TValue>
-class ClassPropertySetter : public PropertySetterBase {
+class ClassPropertyValueSetter : public PropertySetterBase {
  public:
-  ClassPropertySetter(const ui::ClassProperty<TValue>* property,
-                      metadata::ArgType<TValue> value)
+  ClassPropertyValueSetter(const ui::ClassProperty<TValue>* property,
+                           TValue value)
+      : property_(property), value_(value) {}
+  ClassPropertyValueSetter(const ClassPropertyValueSetter&) = delete;
+  ClassPropertyValueSetter& operator=(const ClassPropertyValueSetter&) = delete;
+  ~ClassPropertyValueSetter() override = default;
+
+  void SetProperty(void* obj) override {
+    static_cast<TClass*>(obj)->SetProperty(property_, value_);
+  }
+
+ private:
+  const ui::ClassProperty<TValue>* property_;
+  TValue value_;
+};
+
+template <typename TClass, typename TValue>
+class ClassPropertyMoveSetter : public PropertySetterBase {
+ public:
+  ClassPropertyMoveSetter(const ui::ClassProperty<TValue*>* property,
+                          const TValue& value)
+      : property_(property), value_(value) {}
+  ClassPropertyMoveSetter(const ui::ClassProperty<TValue*>* property,
+                          TValue&& value)
       : property_(property), value_(std::move(value)) {}
-  ClassPropertySetter(const ClassPropertySetter&) = delete;
-  ClassPropertySetter& operator=(const ClassPropertySetter&) = delete;
-  ~ClassPropertySetter() override = default;
+  ClassPropertyMoveSetter(const ClassPropertyMoveSetter&) = delete;
+  ClassPropertyMoveSetter& operator=(const ClassPropertyMoveSetter&) = delete;
+  ~ClassPropertyMoveSetter() override = default;
 
   void SetProperty(void* obj) override {
     static_cast<TClass*>(obj)->SetProperty(property_, std::move(value_));
   }
 
  private:
-  const ui::ClassProperty<TValue>* property_;
+  const ui::ClassProperty<TValue*>* property_;
   TValue value_;
 };