mash: Migrate ShelfTooltipManager to wm common types.

ash/common/shelf/shelf_tooltip_manager.cc

Index: ash/common/shelf/shelf_tooltip_manager.cc
diff --git a/ash/shelf/shelf_tooltip_manager.cc b/ash/common/shelf/shelf_tooltip_manager.cc
similarity index 65%
rename from ash/shelf/shelf_tooltip_manager.cc
rename to ash/common/shelf/shelf_tooltip_manager.cc
index 35a83a23d6808631583798d291725a3777f2df40..f7bd7c650506044f19e9dd7950439b9156e37c4c 100644
--- a/ash/shelf/shelf_tooltip_manager.cc
+++ b/ash/common/shelf/shelf_tooltip_manager.cc
@@ -2,19 +2,19 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "ash/shelf/shelf_tooltip_manager.h"
+#include "ash/common/shelf/shelf_tooltip_manager.h"
 
+#include "ash/common/shelf/wm_shelf.h"
 #include "ash/common/shell_window_ids.h"
-#include "ash/shelf/shelf.h"
-#include "ash/shelf/shelf_layout_manager.h"
+#include "ash/common/wm_lookup.h"
+#include "ash/common/wm_root_window_controller.h"
+#include "ash/common/wm_shell.h"
+#include "ash/common/wm_window.h"
 #include "ash/shelf/shelf_view.h"
-#include "ash/shell.h"
-#include "ash/wm/window_animations.h"
 #include "base/bind.h"
 #include "base/strings/string16.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
-#include "ui/aura/window.h"
 #include "ui/events/event.h"
 #include "ui/events/event_constants.h"
 #include "ui/gfx/geometry/insets.h"
@@ -22,7 +22,6 @@
 #include "ui/views/controls/label.h"
 #include "ui/views/layout/fill_layout.h"
 #include "ui/views/widget/widget.h"
-#include "ui/wm/core/window_animations.h"
 
 namespace ash {
 namespace {
@@ -53,8 +52,7 @@ class ShelfTooltipManager::ShelfTooltipBubble
                      views::BubbleBorder::Arrow arrow,
                      const base::string16& text)
       : views::BubbleDialogDelegateView(anchor, arrow) {
-    gfx::Insets insets =
-        gfx::Insets(kArrowOffsetTopBottom, kArrowOffsetLeftRight);
+    gfx::Insets insets(kArrowOffsetTopBottom, kArrowOffsetLeftRight);

[James Cook, 2016/07/26 16:31:14]

Just FYI, vakha has a lgtm'd patch that touches this line (and others in this
file), so you've got a rebase ahead of you.
https://codereview.chromium.org/2174643002/

[msw, 2016/07/26 17:43:25]

Acknowledged.

     // Adjust the anchor location for asymmetrical borders of shelf item.
     if (anchor->border())
       insets += anchor->border()->GetInsets();
@@ -66,12 +64,6 @@ class ShelfTooltipManager::ShelfTooltipBubble
     set_margins(gfx::Insets(kTooltipTopBottomMargin, kTooltipLeftRightMargin));
     set_shadow(views::BubbleBorder::SMALL_SHADOW);
     SetLayoutManager(new views::FillLayout());
-    // The anchor may not have the widget in tests.
-    if (anchor->GetWidget() && anchor->GetWidget()->GetNativeWindow()) {
-      set_parent_window(ash::Shell::GetContainer(
-          anchor->GetWidget()->GetNativeWindow()->GetRootWindow(),
-          ash::kShellWindowId_SettingBubbleContainer));
-    }
     views::Label* label = new views::Label(text);
     label->SetHorizontalAlignment(gfx::ALIGN_LEFT);
     label->SetEnabledColor(kTooltipTextColor);
@@ -87,6 +79,16 @@ class ShelfTooltipManager::ShelfTooltipBubble
                      std::max(size.height(), kTooltipMinHeight));
   }
 
+  void OnBeforeBubbleWidgetInit(views::Widget::InitParams* params,
+                                views::Widget* bubble_widget) const override {
+    // Place the bubble in the same display as the anchor.
+    WmLookup::Get()
+        ->GetWindowForWidget(anchor_widget())
+        ->GetRootWindowController()
+        ->ConfigureWidgetInitParamsForContainer(
+            bubble_widget, kShellWindowId_SettingBubbleContainer, params);
+  }
+
   int GetDialogButtons() const override { return ui::DIALOG_BUTTON_NONE; }
 
   DISALLOW_COPY_AND_ASSIGN(ShelfTooltipBubble);
@@ -95,32 +97,26 @@ class ShelfTooltipManager::ShelfTooltipBubble
 ShelfTooltipManager::ShelfTooltipManager(ShelfView* shelf_view)
     : timer_delay_(kTooltipAppearanceDelay),
       shelf_view_(shelf_view),
-      root_window_(nullptr),
-      shelf_layout_manager_(nullptr),
       bubble_(nullptr),
-      weak_factory_(this) {}
+      weak_factory_(this) {
+  shelf_view_->wm_shelf()->AddObserver(this);
+  WmShell::Get()->AddPointerWatcher(this);
+}
 
 ShelfTooltipManager::~ShelfTooltipManager() {
-  WillDeleteShelfLayoutManager();
-
-  Shell::GetInstance()->RemovePointerWatcher(this);
-
-  if (root_window_) {
-    root_window_->RemoveObserver(this);
-    root_window_->RemovePreTargetHandler(this);
-    root_window_ = nullptr;
-  }
+  WmShell::Get()->RemovePointerWatcher(this);
+  shelf_view_->wm_shelf()->RemoveObserver(this);
+  WmWindow* window = nullptr;
+  if (shelf_view_->GetWidget())
+    window = WmLookup::Get()->GetWindowForWidget(shelf_view_->GetWidget());

[sky, 2016/07/26 13:25:20]

You have this in a number of places, is it worth a GetShelfWidgetWindow?

[msw, 2016/07/26 17:43:25]

Three is borderline, and only this needs to check the widget; left as-is.

+  if (window)
+    window->RemoveLimitedPreTargetHandler(this);
 }
 
 void ShelfTooltipManager::Init() {
-  shelf_layout_manager_ = shelf_view_->shelf()->shelf_layout_manager();
-  shelf_layout_manager_->AddObserver(this);
-
-  root_window_ = shelf_view_->GetWidget()->GetNativeWindow()->GetRootWindow();
-  root_window_->AddPreTargetHandler(this);
-  root_window_->AddObserver(this);
-
-  Shell::GetInstance()->AddPointerWatcher(this);
+  WmWindow* window =
+      WmLookup::Get()->GetWindowForWidget(shelf_view_->GetWidget());
+  window->AddLimitedPreTargetHandler(this);
 }
 
 void ShelfTooltipManager::Close() {
@@ -142,26 +138,35 @@ void ShelfTooltipManager::ShowTooltip(views::View* view) {
   timer_.Stop();
   if (bubble_) {
     // Cancel the hiding animation to hide the old bubble immediately.
-    gfx::NativeView native_view = bubble_->GetWidget()->GetNativeView();
-    ::wm::SetWindowVisibilityAnimationTransition(native_view,
-                                                 ::wm::ANIMATE_NONE);
+    WmLookup::Get()
+        ->GetWindowForWidget(bubble_->GetWidget())
+        ->SetVisibilityAnimationTransition(::wm::ANIMATE_NONE);
     Close();
   }
 
   if (!ShouldShowTooltipForView(view))
     return;
 
-  Shelf* shelf = shelf_view_->shelf();
-  views::BubbleBorder::Arrow arrow = shelf->SelectValueForShelfAlignment(
-      views::BubbleBorder::BOTTOM_CENTER, views::BubbleBorder::LEFT_CENTER,
-      views::BubbleBorder::RIGHT_CENTER);
+  views::BubbleBorder::Arrow arrow = views::BubbleBorder::Arrow::NONE;
+  switch (shelf_view_->wm_shelf()->GetAlignment()) {
+    case SHELF_ALIGNMENT_BOTTOM:
+    case SHELF_ALIGNMENT_BOTTOM_LOCKED:
+      arrow = views::BubbleBorder::BOTTOM_CENTER;
+      break;
+    case SHELF_ALIGNMENT_LEFT:
+      arrow = views::BubbleBorder::LEFT_CENTER;
+      break;
+    case SHELF_ALIGNMENT_RIGHT:
+      arrow = views::BubbleBorder::RIGHT_CENTER;
+      break;
+  }
 
   base::string16 text = shelf_view_->GetTitleForView(view);
   bubble_ = new ShelfTooltipBubble(view, arrow, text);
-  gfx::NativeView native_view = bubble_->GetWidget()->GetNativeView();
-  ::wm::SetWindowVisibilityAnimationType(
-      native_view, ::wm::WINDOW_VISIBILITY_ANIMATION_TYPE_VERTICAL);
-  ::wm::SetWindowVisibilityAnimationTransition(native_view, ::wm::ANIMATE_HIDE);
+  WmWindow* window = WmLookup::Get()->GetWindowForWidget(bubble_->GetWidget());
+  window->SetVisibilityAnimationType(
+      ::wm::WINDOW_VISIBILITY_ANIMATION_TYPE_VERTICAL);
+  window->SetVisibilityAnimationTransition(::wm::ANIMATE_HIDE);
   bubble_->GetWidget()->Show();
 }
 
@@ -187,24 +192,16 @@ void ShelfTooltipManager::OnTouchPressed(const ui::TouchEvent& event,
   Close();
 }
 
-void ShelfTooltipManager::OnEvent(ui::Event* event) {
-  // Mouse and touch press events are handled via views::PointerWatcher.
-  if (event->type() == ui::ET_MOUSE_PRESSED ||
-      event->type() == ui::ET_TOUCH_PRESSED) {
+void ShelfTooltipManager::OnMouseEvent(ui::MouseEvent* event) {
+  if (event->type() == ui::ET_MOUSE_EXITED) {
+    Close();
     return;
   }
 
-  if (event->type() == ui::ET_MOUSE_EXITED || !event->IsMouseEvent() ||
-      event->target() != shelf_view_->GetWidget()->GetNativeWindow()) {
-    if (!event->IsKeyEvent())
-      Close();
+  if (event->type() != ui::ET_MOUSE_MOVED)
     return;
-  }
 
-  gfx::Point point = static_cast<ui::LocatedEvent*>(event)->location();
-  aura::Window::ConvertPointToTarget(
-      static_cast<aura::Window*>(event->target()),
-      shelf_view_->GetWidget()->GetNativeWindow(), &point);
+  gfx::Point point = event->location();
   views::View::ConvertPointFromWidget(shelf_view_, &point);

[James Cook, 2016/07/26 16:31:14]

Just to double-check: Is this still needed? I'm not sure what coordinate system
is used for events matched by a pre-target handler at the window level.

[msw, 2016/07/26 17:43:25]

I tested removing this, and it breaks (reverses) the location in RTL UI, so I
left it here. I think the widget bounds and window bounds are equivalent in this
case, so given the event's location in window bounds, we can convert directly
from widget bounds to view bounds.

[James Cook, 2016/07/26 17:55:37]

Ah, RTL. Good to know.

   views::View* view = shelf_view_->GetTooltipHandlerForPoint(point);
   const bool should_show = ShouldShowTooltipForView(view);
@@ -212,27 +209,12 @@ void ShelfTooltipManager::OnEvent(ui::Event* event) {
   timer_.Stop();
   if (IsVisible() && should_show && bubble_->GetAnchorView() != view)
     ShowTooltip(view);
-  else if (!IsVisible() && should_show && event->type() == ui::ET_MOUSE_MOVED)
+  else if (!IsVisible() && should_show)
     ShowTooltipWithDelay(view);
   else if (IsVisible() && shelf_view_->ShouldHideTooltip(point))
     Close();
 }
 
-void ShelfTooltipManager::OnWindowDestroying(aura::Window* window) {

[sky, 2016/07/26 13:25:20]

Is this and the next no loner necessary because of destruction order?

[James Cook, 2016/07/26 16:31:14]

Same question. I suspect it's OK since you're no longer directly observing root
window and WmShelf is long lived, but it might be worth manually trying out the
external monitor connect/disconnect case. (You're probably familiar with this,
but if not, run chrome with --ash-debug-shortcuts and hit Ctrl-Shift-D to
add/remove.)

[msw, 2016/07/26 17:43:25]

Yeah, this is no longer an observer of the aura root window, nor an observer of
the shelf layout manager. As the instance is owned by ShelfView, teardown in the
destructor seems sufficient. The dtor accesses shelf_view_->wm_shelf() and
shelf_view_->GetWidget (which needed a null check for unit tests), but otherwise
this works in automated and manual testing, including external monitor
connect/disconnect, and overflow views. Let me know if you're concerned, I can
split Shutdown out the of the dtor and try to call it earlier (perhaps in
ShelfView::ViewHierarchyChanged or make this a WidgetRemovalsObserver and
teardown in OnWillRemoveView, not sure if these would be better/safer).

[James Cook, 2016/07/26 17:55:37]

I think it's fine as you have it.

-  if (window == root_window_) {
-    root_window_->RemoveObserver(this);
-    root_window_->RemovePreTargetHandler(this);
-    root_window_ = nullptr;
-  }
-}
-
-void ShelfTooltipManager::WillDeleteShelfLayoutManager() {
-  if (shelf_layout_manager_)
-    shelf_layout_manager_->RemoveObserver(this);
-  shelf_layout_manager_ = nullptr;
-  shelf_view_ = nullptr;
-}
-
 void ShelfTooltipManager::WillChangeVisibilityState(
     ShelfVisibilityState new_state) {
   if (new_state == SHELF_HIDDEN)
@@ -253,7 +235,7 @@ void ShelfTooltipManager::OnAutoHideStateChanged(ShelfAutoHideState new_state) {
 
 bool ShelfTooltipManager::ShouldShowTooltipForView(views::View* view) {
   return shelf_view_ && shelf_view_->ShouldShowTooltipForView(view) &&
-         shelf_layout_manager_ && shelf_layout_manager_->IsVisible();
+         shelf_view_->wm_shelf()->GetVisibilityState() == SHELF_VISIBLE;
 }
 
 }  // namespace ash