Aura tooltips do not move on mouse move in case of many neighboring

views with the same tooltip label

BUG=Aura tooltips do not move on mouse move in case of many neighboring
views with the same tooltip label. This can be really uncomfortable
when working with lots of tabs with the same address on the
tab strip. Try creating ~30 startpage tabs and hover over one of them,
then move the cursor to others. The tooltip will just sit in the place
where it was first shown.

Review URL: https://codereview.chromium.org/213833018

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@277010 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 202 insertions, 3 deletions

diff --git a/ui/views/corewm/tooltip_controller.cc b/ui/views/corewm/tooltip_controller.cc
index 86e9fd6..b1ffc1c 100644
--- a/ui/views/corewm/tooltip_controller.cc
+++ b/ui/views/corewm/tooltip_controller.cc
@@ -115,6 +115,7 @@ aura::Window* GetTooltipTarget(const ui::MouseEvent& event,
 
 TooltipController::TooltipController(scoped_ptr<Tooltip> tooltip)
     : tooltip_window_(NULL),
+      tooltip_id_(NULL),
       tooltip_window_at_mouse_press_(NULL),
       tooltip_(tooltip.Pass()),
       tooltips_enabled_(true) {
@@ -282,12 +283,19 @@ void TooltipController::UpdateIfRequired() {
     tooltip_window_at_mouse_press_ = NULL;
   }
 
+  // If the uniqueness indicator is different from the previously encountered
+  // one, we should force tooltip update
+  const void* tooltip_id = aura::client::GetTooltipId(tooltip_window_);
+  bool ids_differ = false;
+  ids_differ = tooltip_id_ != tooltip_id;
+  tooltip_id_ = tooltip_id;
+
   // We add the !tooltip_->IsVisible() below because when we come here from
   // TooltipTimerFired(), the tooltip_text may not have changed but we still
   // want to update the tooltip because the timer has fired.
   // If we come here from UpdateTooltip(), we have already checked for tooltip
   // visibility and this check below will have no effect.
-  if (tooltip_text_ != tooltip_text || !tooltip_->IsVisible()) {
+  if (tooltip_text_ != tooltip_text || !tooltip_->IsVisible() || ids_differ) {
     tooltip_shown_timer_.Stop();
     tooltip_text_ = tooltip_text;
     base::string16 trimmed_text(tooltip_text_);
diff --git a/ui/views/corewm/tooltip_controller.h b/ui/views/corewm/tooltip_controller.h
index 9815df1..e045334 100644
--- a/ui/views/corewm/tooltip_controller.h
+++ b/ui/views/corewm/tooltip_controller.h
@@ -61,7 +61,7 @@ class VIEWS_EXPORT TooltipController : public aura::client::TooltipClient,
   void TooltipShownTimerFired();
 
   // Updates the tooltip if required (if there is any change in the tooltip
-  // text or the aura::Window.
+  // text, tooltip id or the aura::Window).
   void UpdateIfRequired();
 
   // Only used in tests.
@@ -81,6 +81,7 @@ class VIEWS_EXPORT TooltipController : public aura::client::TooltipClient,
 
   aura::Window* tooltip_window_;
   base::string16 tooltip_text_;
+  const void* tooltip_id_;
 
   // These fields are for tracking state when the user presses a mouse button.
   aura::Window* tooltip_window_at_mouse_press_;
diff --git a/ui/views/corewm/tooltip_controller_unittest.cc b/ui/views/corewm/tooltip_controller_unittest.cc
index f09a2a0..5460227 100644
--- a/ui/views/corewm/tooltip_controller_unittest.cc
+++ b/ui/views/corewm/tooltip_controller_unittest.cc
@@ -671,6 +671,7 @@ class TestTooltip : public Tooltip {
                        const base::string16& tooltip_text,
                        const gfx::Point& location) OVERRIDE {
     tooltip_text_ = tooltip_text;
+    location_ = location;
   }
   virtual void Show() OVERRIDE {
     is_visible_ = true;
@@ -681,10 +682,12 @@ class TestTooltip : public Tooltip {
   virtual bool IsVisible() OVERRIDE {
     return is_visible_;
   }
+  const gfx::Point& location() { return location_; }
 
  private:
   bool is_visible_;
   base::string16 tooltip_text_;
+  gfx::Point location_;
 
   DISALLOW_COPY_AND_ASSIGN(TestTooltip);
 };
@@ -766,6 +769,174 @@ TEST_F(TooltipControllerTest2, CloseOnCancelMode) {
   EXPECT_TRUE(helper_->GetTooltipWindow() == NULL);
 }
 
+// Use for tests that need both views and a TestTooltip.
+class TooltipControllerTest3 : public aura::test::AuraTestBase {
+ public:
+  TooltipControllerTest3() : test_tooltip_(new TestTooltip) {}
+  virtual ~TooltipControllerTest3() {}
+
+  virtual void SetUp() OVERRIDE {
+    wm_state_.reset(new wm::WMState);
+    aura::test::AuraTestBase::SetUp();
+    new wm::DefaultActivationClient(root_window());
+
+    widget_.reset(CreateWidget(root_window()));
+    widget_->SetContentsView(new View);
+    view_ = new TooltipTestView;
+    widget_->GetContentsView()->AddChildView(view_);
+    view_->SetBoundsRect(widget_->GetContentsView()->GetLocalBounds());
+
+    generator_.reset(new aura::test::EventGenerator(GetRootWindow()));
+    controller_.reset(new TooltipController(
+        scoped_ptr<views::corewm::Tooltip>(test_tooltip_)));
+    GetRootWindow()->RemovePreTargetHandler(
+        static_cast<TooltipController*>(aura::client::GetTooltipClient(
+            widget_->GetNativeWindow()->GetRootWindow())));
+    GetRootWindow()->AddPreTargetHandler(controller_.get());
+    helper_.reset(new TooltipControllerTestHelper(controller_.get()));
+    SetTooltipClient(GetRootWindow(), controller_.get());
+  }
+
+  virtual void TearDown() OVERRIDE {
+    GetRootWindow()->RemovePreTargetHandler(controller_.get());
+    aura::client::SetTooltipClient(GetRootWindow(), NULL);
+
+    controller_.reset();
+    generator_.reset();
+    helper_.reset();
+    widget_.reset();
+    aura::test::AuraTestBase::TearDown();
+    wm_state_.reset();
+  }
+
+  aura::Window* GetWindow() { return widget_->GetNativeWindow(); }
+
+ protected:
+  // Owned by |controller_|.
+  TestTooltip* test_tooltip_;
+  scoped_ptr<TooltipControllerTestHelper> helper_;
+  scoped_ptr<aura::test::EventGenerator> generator_;
+  scoped_ptr<views::Widget> widget_;
+  TooltipTestView* view_;
+
+ private:
+  scoped_ptr<TooltipController> controller_;
+  scoped_ptr<wm::WMState> wm_state_;
+
+#if defined(OS_WIN)
+  ui::ScopedOleInitializer ole_initializer_;
+#endif
+
+  aura::Window* GetRootWindow() { return GetWindow()->GetRootWindow(); }
+
+  DISALLOW_COPY_AND_ASSIGN(TooltipControllerTest3);
+};
+
+TEST_F(TooltipControllerTest3, TooltipPositionChangesOnTwoViewsWithSameLabel) {
+  // Owned by |view_|.
+  // These two views have the same tooltip text
+  TooltipTestView* v1 = new TooltipTestView;
+  TooltipTestView* v2 = new TooltipTestView;
+  // v1_1 is a view inside v1 that has an identical tooltip text to that of v1
+  // and v2
+  TooltipTestView* v1_1 = new TooltipTestView;
+  // v2_1 is a view inside v2 that has an identical tooltip text to that of v1
+  // and v2
+  TooltipTestView* v2_1 = new TooltipTestView;
+  // v2_2 is a view inside v2 with the tooltip text different from all the
+  // others
+  TooltipTestView* v2_2 = new TooltipTestView;
+
+  // Setup all the views' relations
+  view_->AddChildView(v1);
+  view_->AddChildView(v2);
+  v1->AddChildView(v1_1);
+  v2->AddChildView(v2_1);
+  v2->AddChildView(v2_2);
+  const base::string16 reference_string(
+      base::ASCIIToUTF16("Identical Tooltip Text"));
+  const base::string16 alternative_string(
+      base::ASCIIToUTF16("Another Shrubbery"));
+  v1->set_tooltip_text(reference_string);
+  v2->set_tooltip_text(reference_string);
+  v1_1->set_tooltip_text(reference_string);
+  v2_1->set_tooltip_text(reference_string);
+  v2_2->set_tooltip_text(alternative_string);
+
+  // Set views' bounds
+  gfx::Rect view_bounds(view_->GetLocalBounds());
+  view_bounds.set_height(view_bounds.height() / 2);
+  v1->SetBoundsRect(view_bounds);
+  v1_1->SetBounds(0, 0, 3, 3);
+  view_bounds.set_y(view_bounds.height());
+  v2->SetBoundsRect(view_bounds);
+  v2_2->SetBounds(view_bounds.width() - 3, view_bounds.height() - 3, 3, 3);
+  v2_1->SetBounds(0, 0, 3, 3);
+
+  // Test whether a toolbar appears on v1
+  gfx::Point center = v1->bounds().CenterPoint();
+  generator_->MoveMouseRelativeTo(GetWindow(), center);
+  helper_->FireTooltipTimer();
+  EXPECT_TRUE(helper_->IsTooltipVisible());
+  EXPECT_EQ(reference_string, helper_->GetTooltipText());
+  gfx::Point tooltip_bounds1 = test_tooltip_->location();
+
+  // Test whether the toolbar changes position on mouse over v2
+  center = v2->bounds().CenterPoint();
+  generator_->MoveMouseRelativeTo(GetWindow(), center);
+  helper_->FireTooltipTimer();
+  EXPECT_TRUE(helper_->IsTooltipVisible());
+  EXPECT_EQ(reference_string, helper_->GetTooltipText());
+  gfx::Point tooltip_bounds2 = test_tooltip_->location();
+
+  EXPECT_NE(tooltip_bounds1, gfx::Point());
+  EXPECT_NE(tooltip_bounds2, gfx::Point());
+  EXPECT_NE(tooltip_bounds1, tooltip_bounds2);
+
+  // Test if the toolbar does not change position on encountering a contained
+  // view with the same tooltip text
+  center = v2_1->GetLocalBounds().CenterPoint();
+  views::View::ConvertPointToTarget(v2_1, view_, &center);
+  generator_->MoveMouseRelativeTo(GetWindow(), center);
+  helper_->FireTooltipTimer();
+  gfx::Point tooltip_bounds2_1 = test_tooltip_->location();
+
+  EXPECT_NE(tooltip_bounds2, tooltip_bounds2_1);
+  EXPECT_TRUE(helper_->IsTooltipVisible());
+  EXPECT_EQ(reference_string, helper_->GetTooltipText());
+
+  // Test if the toolbar changes position on encountering a contained
+  // view with a different tooltip text
+  center = v2_2->GetLocalBounds().CenterPoint();
+  views::View::ConvertPointToTarget(v2_2, view_, &center);
+  generator_->MoveMouseRelativeTo(GetWindow(), center);
+  helper_->FireTooltipTimer();
+  gfx::Point tooltip_bounds2_2 = test_tooltip_->location();
+
+  EXPECT_NE(tooltip_bounds2_1, tooltip_bounds2_2);
+  EXPECT_TRUE(helper_->IsTooltipVisible());
+  EXPECT_EQ(alternative_string, helper_->GetTooltipText());
+
+  // Test if moving from a view that is contained by a larger view, both with
+  // the same tooltip text, does not change tooltip's position.
+  center = v1_1->GetLocalBounds().CenterPoint();
+  views::View::ConvertPointToTarget(v1_1, view_, &center);
+  generator_->MoveMouseRelativeTo(GetWindow(), center);
+  helper_->FireTooltipTimer();
+  gfx::Point tooltip_bounds1_1 = test_tooltip_->location();
+
+  EXPECT_TRUE(helper_->IsTooltipVisible());
+  EXPECT_EQ(reference_string, helper_->GetTooltipText());
+
+  center = v1->bounds().CenterPoint();
+  generator_->MoveMouseRelativeTo(GetWindow(), center);
+  helper_->FireTooltipTimer();
+  tooltip_bounds1 = test_tooltip_->location();
+
+  EXPECT_NE(tooltip_bounds1_1, tooltip_bounds1);
+  EXPECT_EQ(reference_string, helper_->GetTooltipText());
+}
+
 }  // namespace test
 }  // namespace corewm
 }  // namespace views
diff --git a/ui/views/widget/tooltip_manager_aura.cc b/ui/views/widget/tooltip_manager_aura.cc
index 986ea5d..99ae9e6 100644
--- a/ui/views/widget/tooltip_manager_aura.cc
+++ b/ui/views/widget/tooltip_manager_aura.cc
@@ -133,6 +133,9 @@ void TooltipManagerAura::UpdateTooltipForTarget(View* target,
   } else {
     tooltip_text_.clear();
   }
+
+  aura::client::SetTooltipId(GetWindow(), target);
+
   aura::client::GetTooltipClient(root_window)->UpdateTooltip(GetWindow());
 }
 
diff --git a/ui/wm/public/tooltip_client.cc b/ui/wm/public/tooltip_client.cc
index b473238..844cca5 100644
--- a/ui/wm/public/tooltip_client.cc
+++ b/ui/wm/public/tooltip_client.cc
@@ -9,6 +9,7 @@
 
 DECLARE_WINDOW_PROPERTY_TYPE(aura::client::TooltipClient*)
 DECLARE_WINDOW_PROPERTY_TYPE(base::string16*)
+DECLARE_WINDOW_PROPERTY_TYPE(void**)
 
 namespace aura {
 namespace client {
@@ -16,6 +17,7 @@ namespace client {
 DEFINE_LOCAL_WINDOW_PROPERTY_KEY(
     TooltipClient*, kRootWindowTooltipClientKey, NULL);
 DEFINE_LOCAL_WINDOW_PROPERTY_KEY(base::string16*, kTooltipTextKey, NULL);
+DEFINE_LOCAL_WINDOW_PROPERTY_KEY(void*, kTooltipIdKey, NULL);
 
 void SetTooltipClient(Window* root_window, TooltipClient* client) {
   DCHECK_EQ(root_window->GetRootWindow(), root_window);
@@ -33,10 +35,18 @@ void SetTooltipText(Window* window, base::string16* tooltip_text) {
   window->SetProperty(kTooltipTextKey, tooltip_text);
 }
 
+void SetTooltipId(Window* window, void* id) {
+  window->SetProperty(kTooltipIdKey, id);
+}
+
 const base::string16 GetTooltipText(Window* window) {
   base::string16* string_ptr = window->GetProperty(kTooltipTextKey);
   return string_ptr ? *string_ptr : base::string16();
 }
 
+const void* GetTooltipId(Window* window) {
+  return window->GetProperty(kTooltipIdKey);
+}
+
 }  // namespace client
 }  // namespace aura
diff --git a/ui/wm/public/tooltip_client.h b/ui/wm/public/tooltip_client.h
index f592b21..f2b97cd 100644
--- a/ui/wm/public/tooltip_client.h
+++ b/ui/wm/public/tooltip_client.h
@@ -36,8 +36,14 @@ AURA_EXPORT void SetTooltipClient(Window* root_window,
                                   TooltipClient* client);
 AURA_EXPORT TooltipClient* GetTooltipClient(Window* root_window);
 
-AURA_EXPORT void SetTooltipText(Window* window, base::string16* tooltip_text);
+// Sets the text for the tooltip. The id is used to determine uniqueness when
+// the text does not change. For example, if the tooltip text does not change,
+// but the id does then the position of the tooltip is updated.
+AURA_EXPORT void SetTooltipText(Window* window,
+                                base::string16* tooltip_text);
+AURA_EXPORT void SetTooltipId(Window* window, void* id);
 AURA_EXPORT const base::string16 GetTooltipText(Window* window);
+AURA_EXPORT const void* GetTooltipId(Window* window);
 
 }  // namespace client
 }  // namespace aura