Move shelf item tooltip flag from delegate to item.

ash/wm/panels/panel_window_resizer_unittest.cc

Index: ash/wm/panels/panel_window_resizer_unittest.cc
diff --git a/ash/wm/panels/panel_window_resizer_unittest.cc b/ash/wm/panels/panel_window_resizer_unittest.cc
index 18e41a1d63a1213a3e21f54904ec1df9d660b8e6..5ac07d8dd7d96a907e1cc7725eea419dcd1ea561 100644
--- a/ash/wm/panels/panel_window_resizer_unittest.cc
+++ b/ash/wm/panels/panel_window_resizer_unittest.cc
@@ -21,6 +21,7 @@
 #include "ash/test/ash_test_base.h"
 #include "ash/test/cursor_manager_test_api.h"
 #include "ash/wm/drag_window_resizer.h"
+#include "ash/wm/window_properties.h"
 #include "ash/wm/window_state_aura.h"
 #include "base/i18n/rtl.h"
 #include "base/win/windows_version.h"
@@ -90,8 +91,7 @@ class PanelWindowResizerTest : public test::AshTestBase {
   // Test dragging the panel slightly, then detaching, and then reattaching
   // dragging out by the vector (dx, dy).
   void DetachReattachTest(aura::Window* window, int dx, int dy) {
-    wm::WindowState* window_state = wm::GetWindowState(window);
-    EXPECT_TRUE(window_state->panel_attached());
+    EXPECT_TRUE(window->GetProperty(kPanelAttachedKey));
     aura::Window* root_window = window->GetRootWindow();
     EXPECT_EQ(kShellWindowId_PanelContainer, window->parent()->id());
     DragStart(window);
@@ -111,7 +111,7 @@ class PanelWindowResizerTest : public test::AshTestBase {
     // The panel should be detached when the drag completes.
     DragEnd();
 
-    EXPECT_FALSE(window_state->panel_attached());
+    EXPECT_FALSE(window->GetProperty(kPanelAttachedKey));
     EXPECT_EQ(kShellWindowId_DefaultContainer, window->parent()->id());
     EXPECT_EQ(root_window, window->GetRootWindow());
 
@@ -122,7 +122,7 @@ class PanelWindowResizerTest : public test::AshTestBase {
     DragEnd();
 
     // The panel should be reattached and have snapped to the launcher.
-    EXPECT_TRUE(window_state->panel_attached());
+    EXPECT_TRUE(window->GetProperty(kPanelAttachedKey));
     EXPECT_EQ(initial_bounds.x(), window->GetBoundsInScreen().x());
     EXPECT_EQ(initial_bounds.y(), window->GetBoundsInScreen().y());
     EXPECT_EQ(kShellWindowId_PanelContainer, window->parent()->id());
@@ -299,7 +299,7 @@ TEST_F(PanelWindowResizerTest, DetachThenDragAcrossDisplays) {
   EXPECT_EQ(root_windows[0], window->GetRootWindow());
   EXPECT_EQ(initial_bounds.x(), window->GetBoundsInScreen().x());
   EXPECT_EQ(initial_bounds.y() - 100, window->GetBoundsInScreen().y());
-  EXPECT_FALSE(wm::GetWindowState(window.get())->panel_attached());
+  EXPECT_FALSE(window->GetProperty(kPanelAttachedKey));
   EXPECT_EQ(kShellWindowId_DefaultContainer, window->parent()->id());
 
   DragStart(window.get());
@@ -308,7 +308,7 @@ TEST_F(PanelWindowResizerTest, DetachThenDragAcrossDisplays) {
   EXPECT_EQ(root_windows[1], window->GetRootWindow());
   EXPECT_EQ(initial_bounds.x() + 500, window->GetBoundsInScreen().x());
   EXPECT_EQ(initial_bounds.y() - 100, window->GetBoundsInScreen().y());
-  EXPECT_FALSE(wm::GetWindowState(window.get())->panel_attached());
+  EXPECT_FALSE(window->GetProperty(kPanelAttachedKey));
   EXPECT_EQ(kShellWindowId_DefaultContainer, window->parent()->id());
 }
 
@@ -327,7 +327,7 @@ TEST_F(PanelWindowResizerTest, DetachAcrossDisplays) {
   EXPECT_EQ(root_windows[1], window->GetRootWindow());
   EXPECT_EQ(initial_bounds.x() + 500, window->GetBoundsInScreen().x());
   EXPECT_EQ(initial_bounds.y() - 100, window->GetBoundsInScreen().y());
-  EXPECT_FALSE(wm::GetWindowState(window.get())->panel_attached());
+  EXPECT_FALSE(window->GetProperty(kPanelAttachedKey));

[sky, 2016/12/08 00:38:54]

I think Oshima explicitly wanted state information to be encapsulated in
WmState. It's ok to have WmState call out to get a property, but everyone but
WmState should go through WmState. I'll add Oshima to this review for his input
to see if I have that right.

[msw, 2016/12/08 20:31:36]

I'm eager to hear more about the reasoning for that. It seems more difficult to
propagate the whole wm::WindowState property for aura-mus (we'd need a matching
mojo struct and converter), versus using a handful of individual window
properties. It also seems more difficult to observe changes to wm::WindowState
members (there's no OnWindowStateChanged, WindowStateObserver looks rather
incomplete, and I wonder why we'd want to split window observation from state
observation, and where to draw that line, eg. would it make sense to call
WindowObserver::OnWindowStatechanged or
WindowStateObserver::OnPanelAttachedChanged anytime this flag changes?).

It looks like a lot of wm::WindowState mirrors what's on aura::Window, and
simultaneously, wm::WindowState doesn't seem to capture everything needed to
represent the state of a window (eg. title?), some of its purpose is lost on me.

Abstractly, this seems like the difference between keeping numerous simple
window properties versus keeping a single complex window property that
represents a wide array of aspects regarding window state... I can see that the
latter might help for certain things (storing and reverting a set of properties
that depend on the completion/cancellation of a drag), but in general, it seems
like keeping state information as window properties makes more sense to me.

Oshima, can you weigh in? Perhaps we don't need an overarching plan if moving
this one flag to a window property (for now) is alright.

[sky, 2016/12/08 20:52:42]

I'm going to leave Oshima to answer this one.

[msw, 2016/12/09 01:18:21]

Oshima and I chatted over im; he doesn't have any objections. He noted that
wm::WindowState is better for ash/wm-specific state, but that's not a strict
rule, and panels are going away soon anyway.

   EXPECT_EQ(kShellWindowId_DefaultContainer, window->parent()->id());
 }
 
@@ -346,7 +346,7 @@ TEST_F(PanelWindowResizerTest, DetachThenAttachToSecondDisplay) {
   DragMove(0, -100);
   DragEnd();
   EXPECT_EQ(root_windows[0], window->GetRootWindow());
-  EXPECT_FALSE(wm::GetWindowState(window.get())->panel_attached());
+  EXPECT_FALSE(window->GetProperty(kPanelAttachedKey));
 
   // Drag the window just above the other display's launcher.
   DragStart(window.get());
@@ -359,7 +359,7 @@ TEST_F(PanelWindowResizerTest, DetachThenAttachToSecondDisplay) {
 
   // When dropped should move to second display's panel container.
   EXPECT_EQ(root_windows[1], window->GetRootWindow());
-  EXPECT_TRUE(wm::GetWindowState(window.get())->panel_attached());
+  EXPECT_TRUE(window->GetProperty(kPanelAttachedKey));
   EXPECT_EQ(kShellWindowId_PanelContainer, window->parent()->id());
 }
 
@@ -384,7 +384,7 @@ TEST_F(PanelWindowResizerTest, AttachToSecondDisplay) {
 
   // When dropped should move to second display's panel container.
   EXPECT_EQ(root_windows[1], window->GetRootWindow());
-  EXPECT_TRUE(wm::GetWindowState(window.get())->panel_attached());
+  EXPECT_TRUE(window->GetProperty(kPanelAttachedKey));
   EXPECT_EQ(kShellWindowId_PanelContainer, window->parent()->id());
 }
 
@@ -415,7 +415,7 @@ TEST_F(PanelWindowResizerTest, AttachToSecondFullscreenDisplay) {
 
   // When dropped should move to second display's panel container.
   EXPECT_EQ(root_windows[1], window->GetRootWindow());
-  EXPECT_TRUE(wm::GetWindowState(window.get())->panel_attached());
+  EXPECT_TRUE(window->GetProperty(kPanelAttachedKey));
   EXPECT_EQ(kShellWindowId_PanelContainer, window->parent()->id());
   EXPECT_TRUE(window->IsVisible());
   EXPECT_TRUE(wm::GetWindowState(window.get())->IsActive());
@@ -424,19 +424,19 @@ TEST_F(PanelWindowResizerTest, AttachToSecondFullscreenDisplay) {
 
 TEST_F(PanelWindowResizerTest, RevertDragRestoresAttachment) {
   std::unique_ptr<aura::Window> window(CreatePanelWindow(gfx::Point(0, 0)));
-  EXPECT_TRUE(wm::GetWindowState(window.get())->panel_attached());
+  EXPECT_TRUE(window->GetProperty(kPanelAttachedKey));
   EXPECT_EQ(kShellWindowId_PanelContainer, window->parent()->id());
   DragStart(window.get());
   DragMove(0, -100);
   DragRevert();
-  EXPECT_TRUE(wm::GetWindowState(window.get())->panel_attached());
+  EXPECT_TRUE(window->GetProperty(kPanelAttachedKey));
   EXPECT_EQ(kShellWindowId_PanelContainer, window->parent()->id());
 
   // Detach panel.
   DragStart(window.get());
   DragMove(0, -100);
   DragEnd();
-  EXPECT_FALSE(wm::GetWindowState(window.get())->panel_attached());
+  EXPECT_FALSE(window->GetProperty(kPanelAttachedKey));
   EXPECT_EQ(kShellWindowId_DefaultContainer, window->parent()->id());
 
   // Drag back to launcher.
@@ -445,7 +445,7 @@ TEST_F(PanelWindowResizerTest, RevertDragRestoresAttachment) {
 
   // When the drag is reverted it should remain detached.
   DragRevert();
-  EXPECT_FALSE(wm::GetWindowState(window.get())->panel_attached());
+  EXPECT_FALSE(window->GetProperty(kPanelAttachedKey));
   EXPECT_EQ(kShellWindowId_DefaultContainer, window->parent()->id());
 }