Removed set_owned_by_client() from TooltipAura

Currently TooltipAura creates a single TooltipView for its tooltips
and shares this single instance between transient instances of Widget
which are created as needed.

This patch changes the behavior such that a new TooltipView is
created for each new TooltipAura Widget. This ensures that each
Widget hierarchy can assert ownership over its View tree in alignment
with current Views best practices.

Bug: 1044687
Change-Id: I8441ec4264c012d00bd68fd5c7a1d15113e02246
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2212800Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Thomas Lukaszewicz <tluk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771937}

ui/views/corewm/tooltip_aura.cc

@@ -53,40 +53,14 @@ bool CanUseTranslucentTooltipWidget() {
 #endif
 }
 
-// Creates a widget of type TYPE_TOOLTIP
-views::Widget* CreateTooltipWidget(aura::Window* tooltip_window) {
-  views::Widget* widget = new views::Widget;
-  views::Widget::InitParams params;
-  // For aura, since we set the type to TYPE_TOOLTIP, the widget will get
-  // auto-parented to the right container.
-  params.type = views::Widget::InitParams::TYPE_TOOLTIP;
-  params.context = tooltip_window;
-  DCHECK(params.context);
-  params.z_order = ui::ZOrderLevel::kFloatingUIElement;
-  params.accept_events = false;
-  if (CanUseTranslucentTooltipWidget())
-    params.opacity = views::Widget::InitParams::WindowOpacity::kTranslucent;
-  params.shadow_type = views::Widget::InitParams::ShadowType::kNone;
-  // Use software compositing to avoid using unnecessary hardware resources
-  // which just amount to overkill for this UI.
-  params.force_software_compositing = true;
-  widget->Init(std::move(params));
-  return widget;
-}
-
-}  // namespace
-
-namespace views {
-namespace corewm {
-
 // TODO(oshima): Consider to use views::Label.
-class TooltipAura::TooltipView : public views::View {
+class TooltipView : public views::View {
  public:
   TooltipView() : render_text_(gfx::RenderText::CreateRenderText()) {
-    SetBorder(CreateEmptyBorder(kVerticalPaddingTop, kHorizontalPadding,
-                                kVerticalPaddingBottom, kHorizontalPadding));
+    SetBorder(views::CreateEmptyBorder(kVerticalPaddingTop, kHorizontalPadding,
+                                       kVerticalPaddingBottom,
+                                       kHorizontalPadding));
 
-    set_owned_by_client();
     render_text_->SetWordWrapBehavior(gfx::WRAP_LONG_WORDS);
     render_text_->SetMultiline(true);
 
@@ -177,18 +151,38 @@ class TooltipAura::TooltipView : public views::View {
   DISALLOW_COPY_AND_ASSIGN(TooltipView);
 };
 
-TooltipAura::TooltipAura() : tooltip_view_(new TooltipView) {}
+}  // namespace
+
+namespace views {
+namespace corewm {
 
 TooltipAura::~TooltipAura() {
   DestroyWidget();
 }
 
+class TooltipAura::TooltipWidget : public Widget {
+ public:
+  TooltipWidget() = default;
+  ~TooltipWidget() override = default;
+
+  TooltipView* GetTooltipView() { return tooltip_view_; }
+
+  void SetTooltipView(std::unique_ptr<TooltipView> tooltip_view) {
+    tooltip_view_ = SetContentsView(std::move(tooltip_view));
+  }
+
+ private:
+  TooltipView* tooltip_view_ = nullptr;
+};
+
 gfx::RenderText* TooltipAura::GetRenderTextForTest() {
-  return tooltip_view_->render_text_for_test();
+  DCHECK(widget_);
+  return widget_->GetTooltipView()->render_text_for_test();
 }
 
 void TooltipAura::GetAccessibleNodeDataForTest(ui::AXNodeData* node_data) {
-  tooltip_view_->GetAccessibleNodeData(node_data);
+  DCHECK(widget_);
+  widget_->GetTooltipView()->GetAccessibleNodeData(node_data);
 }
 
 gfx::Rect TooltipAura::GetTooltipBounds(const gfx::Point& mouse_pos,
@@ -214,6 +208,28 @@ gfx::Rect TooltipAura::GetTooltipBounds(const gfx::Point& mouse_pos,
   return tooltip_rect;
 }
 
+void TooltipAura::CreateTooltipWidget() {
+  DCHECK(!widget_);
+  DCHECK(tooltip_window_);
+  widget_ = new TooltipWidget;
+  views::Widget::InitParams params;
+  // For aura, since we set the type to TYPE_TOOLTIP, the widget will get
+  // auto-parented to the right container.
+  params.type = views::Widget::InitParams::TYPE_TOOLTIP;
+  params.context = tooltip_window_;
+  DCHECK(params.context);
+  params.z_order = ui::ZOrderLevel::kFloatingUIElement;
+  params.accept_events = false;
+  if (CanUseTranslucentTooltipWidget())
+    params.opacity = views::Widget::InitParams::WindowOpacity::kTranslucent;
+  params.shadow_type = views::Widget::InitParams::ShadowType::kNone;
+  // Use software compositing to avoid using unnecessary hardware resources
+  // which just amount to overkill for this UI.
+  params.force_software_compositing = true;
+  widget_->Init(std::move(params));
+  widget_->SetTooltipView(std::make_unique<TooltipView>());
+}
+
 void TooltipAura::DestroyWidget() {
   if (widget_) {
     widget_->RemoveObserver(this);
@@ -232,15 +248,17 @@ void TooltipAura::SetText(aura::Window* window,
                           const base::string16& tooltip_text,
                           const gfx::Point& location) {
   tooltip_window_ = window;
-  tooltip_view_->SetMaxWidth(GetMaxWidth(location));
-  tooltip_view_->SetText(tooltip_text);
 
   if (!widget_) {
-    widget_ = CreateTooltipWidget(tooltip_window_);
-    widget_->SetContentsView(tooltip_view_.get());
+    CreateTooltipWidget();
     widget_->AddObserver(this);
   }
 
+  TooltipView* tooltip_view = widget_->GetTooltipView();
+
+  tooltip_view->SetMaxWidth(GetMaxWidth(location));
+  tooltip_view->SetText(tooltip_text);
+
   ui::NativeTheme* native_theme = widget_->GetNativeTheme();
   auto background_color =
       native_theme->GetSystemColor(ui::NativeTheme::kColorId_TooltipBackground);
@@ -254,8 +272,8 @@ void TooltipAura::SetText(aura::Window* window,
   if (!CanUseTranslucentTooltipWidget())
     foreground_color =
         color_utils::GetResultingPaintColor(foreground_color, background_color);
-  tooltip_view_->SetBackgroundColor(background_color, foreground_color);
-  tooltip_view_->SetForegroundColor(foreground_color);
+  tooltip_view->SetBackgroundColor(background_color, foreground_color);
+  tooltip_view->SetForegroundColor(foreground_color);
 
   // Calculate the tooltip preferred size after all tooltip attributes are
   // updated - tooltip updates (for example setting text color) may invalidate
@@ -265,7 +283,7 @@ void TooltipAura::SetText(aura::Window* window,
   // GetPreferredSize() will generate fresh render text layout, even if the
   // actual tooltip text hasn't changed).
   const gfx::Rect adjusted_bounds =
-      GetTooltipBounds(location, tooltip_view_->GetPreferredSize());
+      GetTooltipBounds(location, tooltip_view->GetPreferredSize());
   widget_->SetBounds(adjusted_bounds);
 }
 
@@ -273,8 +291,8 @@ void TooltipAura::Show() {
   if (widget_) {
     widget_->Show();
     widget_->StackAtTop();
-    tooltip_view_->NotifyAccessibilityEvent(ax::mojom::Event::kTooltipOpened,
-                                            true);
+    widget_->GetTooltipView()->NotifyAccessibilityEvent(
+        ax::mojom::Event::kTooltipOpened, true);
   }
 }
 
@@ -288,9 +306,9 @@ void TooltipAura::Hide() {
     // guarantees we never show outdated information.
     // TODO(http://crbug.com/998280): Figure out why the old content is
     // displayed despite the size change.
+    widget_->GetTooltipView()->NotifyAccessibilityEvent(
+        ax::mojom::Event::kTooltipClosed, true);
     DestroyWidget();
-    tooltip_view_->NotifyAccessibilityEvent(ax::mojom::Event::kTooltipClosed,
-                                            true);
   }
 }
 

ui/views/corewm/tooltip_aura.h

@@ -32,11 +32,11 @@ class TooltipAuraTestApi;
 // Implementation of Tooltip that shows the tooltip using a Widget and Label.
 class VIEWS_EXPORT TooltipAura : public Tooltip, public WidgetObserver {
  public:
-  TooltipAura();
+  TooltipAura() = default;
   ~TooltipAura() override;
 
  private:
-  class TooltipView;
+  class TooltipWidget;
 
   friend class test::TooltipAuraTestApi;
   gfx::RenderText* GetRenderTextForTest();
@@ -47,6 +47,9 @@ class VIEWS_EXPORT TooltipAura : public Tooltip, public WidgetObserver {
   gfx::Rect GetTooltipBounds(const gfx::Point& mouse_pos,
                              const gfx::Size& tooltip_size);
 
+  // Sets |widget_| to a new instance of TooltipWidget.
+  void CreateTooltipWidget();
+
   // Destroys |widget_|.
   void DestroyWidget();
 
@@ -62,11 +65,8 @@ class VIEWS_EXPORT TooltipAura : public Tooltip, public WidgetObserver {
   // WidgetObserver:
   void OnWidgetDestroying(Widget* widget) override;
 
-  // The view showing the tooltip.
-  std::unique_ptr<TooltipView> tooltip_view_;
-
   // The widget containing the tooltip. May be NULL.
-  Widget* widget_ = nullptr;
+  TooltipWidget* widget_ = nullptr;
 
   // The window we're showing the tooltip for. Never NULL and valid while
   // showing.