wm: Refactor WorkspaceWindowResizer.

Some things encountered while working on making workspace drag
work with maximized windows.

* remove unused members/variables
* remove base::NumberToString in tests
* combined two anon namespaces
* small miscallenous updates

Test: none
Bug: 1066159
Change-Id: I2dfd1cd7f416d9e93b034147d408227f16a39d13
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2142777Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758059}

ash/wm/drag_details.cc

@@ -70,7 +70,6 @@ DragDetails::DragDetails(aura::Window* window,
       // When drag starts, we might be in the middle of a window opacity
       // animation, on drag completion we must set the opacity to the target
       // opacity rather than the current opacity (crbug.com/687003).
-      initial_opacity(window->layer()->GetTargetOpacity()),
       window_component(window_component),
       bounds_change(
           WindowResizer::GetBoundsChangeForWindowComponent(window_component)),

ash/wm/drag_details.h

@@ -37,9 +37,6 @@ struct ASH_EXPORT DragDetails {
   // Location passed to the constructor, in |window->parent()|'s coordinates.
   const gfx::PointF initial_location_in_parent;
 
-  // Initial opacity of the window.
-  const float initial_opacity;
-
   // The component the user pressed on.
   const int window_component;
 

ash/wm/workspace/workspace_window_resizer.h

@@ -34,22 +34,9 @@ class WindowState;
 // attempt to restore the old height.
 class ASH_EXPORT WorkspaceWindowResizer : public WindowResizer {
  public:
-  // When dragging an attached window this is the min size we'll make sure is
-  // visible. In the vertical direction we take the max of this and that from
-  // the delegate.
-  static const int kMinOnscreenSize;
-
   // Min height we'll force on screen when dragging the caption.
   // TODO: this should come from a property on the window.
-  static const int kMinOnscreenHeight;
-
-  // Snap region when dragging close to the edges. That is, as the window gets
-  // this close to an edge of the screen it snaps to the edge.
-  static const int kScreenEdgeInset;
-
-  // Distance in pixels that the cursor must move past an edge for a window
-  // to move or resize beyond that edge.
-  static const int kStickyDistancePixels;
+  static constexpr int kMinOnscreenHeight = 32;
 
   ~WorkspaceWindowResizer() override;
 
@@ -179,41 +166,36 @@ class ASH_EXPORT WorkspaceWindowResizer : public WindowResizer {
   // Returns the currently used instance for test.
   static WorkspaceWindowResizer* GetInstanceForTest();
 
-  bool did_lock_cursor_;
+  bool did_lock_cursor_ = false;
 
   // Set to true once Drag() is invoked and the bounds of the window change.
-  bool did_move_or_resize_;
+  bool did_move_or_resize_ = false;
 
   // True if the window initially had |bounds_changed_by_user_| set in state.
-  bool initial_bounds_changed_by_user_;
+  const bool initial_bounds_changed_by_user_;
 
   // The initial size of each of the windows in |attached_windows_| along the
   // primary axis.
   std::vector<int> initial_size_;
 
   // Sum of the minimum sizes of the attached windows.
-  int total_min_;
+  int total_min_ = 0;
 
   // Sum of the sizes in |initial_size_|.
-  int total_initial_size_;
+  int total_initial_size_ = 0;
 
   // Gives a previews of where the the window will end up. Only used if there
   // is a grid and the caption is being dragged.
   std::unique_ptr<PhantomWindowController> snap_phantom_window_controller_;
 
   // The edge to which the window should be snapped to at the end of the drag.
-  SnapType snap_type_;
-
-  // Number of mouse moves since the last bounds change. Only used for phantom
-  // placement to track when the mouse is moved while pushed against the edge of
-  // the screen.
-  int num_mouse_moves_since_bounds_change_;
+  SnapType snap_type_ = SNAP_NONE;
 
   // The mouse location passed to Drag().
   gfx::PointF last_mouse_location_;
 
   // Window the drag has magnetically attached to.
-  aura::Window* magnetism_window_;
+  aura::Window* magnetism_window_ = nullptr;
 
   // Used to verify |magnetism_window_| is still valid.
   aura::WindowTracker window_tracker_;

ash/wm/workspace/workspace_window_resizer_unittest.cc

@@ -17,7 +17,6 @@
 #include "ash/wm/workspace/phantom_window_controller.h"
 #include "ash/wm/workspace_controller.h"
 #include "base/command_line.h"
-#include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
 #include "base/test/metrics/histogram_tester.h"
 #include "base/time/time.h"
@@ -158,15 +157,13 @@ class WorkspaceWindowResizerTest : public AshTestBase {
   // Returns a string identifying the z-order of each of the known child windows
   // of |parent|.  The returned string constains the id of the known windows and
   // is ordered from topmost to bottomost windows.
-  std::string WindowOrderAsString(aura::Window* parent) const {
-    std::string result;
+  std::vector<int> WindowOrderAsIntVector(aura::Window* parent) const {
+    std::vector<int> result;
     const aura::Window::Windows& windows = parent->children();
     for (aura::Window::Windows::const_reverse_iterator i = windows.rbegin();
          i != windows.rend(); ++i) {
       if (*i == window_.get() || *i == window2_.get() || *i == window3_.get()) {
-        if (!result.empty())
-          result += " ";
-        result += base::NumberToString((*i)->id());
+        result.push_back((*i)->id());
       }
     }
     return result;
@@ -633,10 +630,9 @@ TEST_F(WorkspaceWindowResizerTest, Edge) {
     EXPECT_EQ(root_windows[0], window_->GetRootWindow());
     resizer->CompleteDrag();
     EXPECT_EQ(root_windows[1], window_->GetRootWindow());
-    EXPECT_EQ("0,0 250x" + base::NumberToString(bottom),
-              window_->bounds().ToString());
-    EXPECT_EQ("820,30 400x60",
-              window_state->GetRestoreBoundsInScreen().ToString());
+    EXPECT_EQ(gfx::Rect(250, bottom), window_->bounds());
+    EXPECT_EQ(gfx::Rect(820, 30, 400, 60),
+              window_state->GetRestoreBoundsInScreen());
   }
 
   // Restore the window to clear snapped state.
@@ -658,10 +654,9 @@ TEST_F(WorkspaceWindowResizerTest, Edge) {
         screen_util::GetDisplayWorkAreaBoundsInParent(window_.get()).bottom();
     resizer->CompleteDrag();
     // TODO(varkha): Insets are updated because of http://crbug.com/292238
-    EXPECT_EQ("250,0 250x" + base::NumberToString(bottom),
-              window_->bounds().ToString());
-    EXPECT_EQ("820,30 400x60",
-              window_state->GetRestoreBoundsInScreen().ToString());
+    EXPECT_EQ(gfx::Rect(250, 0, 250, bottom), window_->bounds());
+    EXPECT_EQ(gfx::Rect(820, 30, 400, 60),
+              window_state->GetRestoreBoundsInScreen());
   }
 }
 
@@ -812,7 +807,8 @@ TEST_F(WorkspaceWindowResizerTest, RestackAttached) {
     resizer->Drag(CalculateDragPoint(*resizer, 100, -10), 0);
 
     // 2 should be topmost since it's initially the highest in the stack.
-    EXPECT_EQ("2 1 3", WindowOrderAsString(window_->parent()));
+    const std::vector<int> expected_order = {2, 1, 3};
+    EXPECT_EQ(expected_order, WindowOrderAsIntVector(window_->parent()));
   }
 
   {
@@ -826,7 +822,8 @@ TEST_F(WorkspaceWindowResizerTest, RestackAttached) {
     resizer->Drag(CalculateDragPoint(*resizer, 100, -10), 0);
 
     // 2 should be topmost since it's initially the highest in the stack.
-    EXPECT_EQ("2 3 1", WindowOrderAsString(window_->parent()));
+    const std::vector<int> expected_order = {2, 3, 1};
+    EXPECT_EQ(expected_order, WindowOrderAsIntVector(window_->parent()));
   }
 }
 
@@ -844,8 +841,7 @@ TEST_F(WorkspaceWindowResizerTest, DontDragOffBottom) {
   resizer->Drag(CalculateDragPoint(*resizer, 0, 600), 0);
   int expected_y =
       kRootHeight - WorkspaceWindowResizer::kMinOnscreenHeight - 10;
-  EXPECT_EQ("100," + base::NumberToString(expected_y) + " 300x400",
-            window_->bounds().ToString());
+  EXPECT_EQ(gfx::Rect(100, expected_y, 300, 400), window_->bounds());
 }
 
 // Makes sure we don't allow dragging on the work area with multidisplay.
@@ -874,8 +870,7 @@ TEST_F(WorkspaceWindowResizerTest, DontDragOffBottomWithMultiDisplay) {
     // When the mouse cursor is in the primary display, the window cannot move
     // on non-work area but can get all the way towards the bottom,
     // restricted only by the window height.
-    EXPECT_EQ("100," + base::NumberToString(expected_y) + " 300x20",
-              window_->bounds().ToString());
+    EXPECT_EQ(gfx::Rect(100, expected_y, 300, 20), window_->bounds());
     // Revert the drag in order to not remember the restore bounds.
     resizer->RevertDrag();
   }
@@ -893,8 +888,7 @@ TEST_F(WorkspaceWindowResizerTest, DontDragOffBottomWithMultiDisplay) {
         kRootHeight - WorkspaceWindowResizer::kMinOnscreenHeight - 10;
     // When the mouse cursor is in the primary display, the window cannot move
     // on non-work area with kMinOnscreenHeight margin.
-    EXPECT_EQ("100," + base::NumberToString(expected_y) + " 300x400",
-              window_->bounds().ToString());
+    EXPECT_EQ(gfx::Rect(100, expected_y, 300, 400), window_->bounds());
     resizer->CompleteDrag();
   }
 
@@ -949,9 +943,8 @@ TEST_F(WorkspaceWindowResizerTest, ResizeWindowOutsideLeftWorkArea) {
       window_.get(), gfx::Point(pixels_to_left_border, 0), HTRIGHT));
   ASSERT_TRUE(resizer.get());
   resizer->Drag(CalculateDragPoint(*resizer, -window_width, 0), 0);
-  EXPECT_EQ(base::NumberToString(window_x) + ",100 " +
-                base::NumberToString(kMinimumOnScreenArea - window_x) + "x380",
-            window_->bounds().ToString());
+  EXPECT_EQ(gfx::Rect(window_x, 100, kMinimumOnScreenArea - window_x, 380),
+            window_->bounds());
 }
 
 TEST_F(WorkspaceWindowResizerTest, ResizeWindowOutsideRightWorkArea) {
@@ -967,11 +960,11 @@ TEST_F(WorkspaceWindowResizerTest, ResizeWindowOutsideRightWorkArea) {
       CreateResizerForTest(window_.get(), gfx::Point(window_x, 0), HTLEFT));
   ASSERT_TRUE(resizer.get());
   resizer->Drag(CalculateDragPoint(*resizer, window_width, 0), 0);
-  EXPECT_EQ(base::NumberToString(right - kMinimumOnScreenArea) + ",100 " +
-                base::NumberToString(window_width - pixels_to_right_border +
-                                     kMinimumOnScreenArea) +
-                "x380",
-            window_->bounds().ToString());
+  EXPECT_EQ(
+      gfx::Rect(right - kMinimumOnScreenArea, 100,
+                window_width - pixels_to_right_border + kMinimumOnScreenArea,
+                380),
+      window_->bounds());
 }
 
 TEST_F(WorkspaceWindowResizerTest, ResizeWindowOutsideBottomWorkArea) {
@@ -986,11 +979,9 @@ TEST_F(WorkspaceWindowResizerTest, ResizeWindowOutsideBottomWorkArea) {
       window_.get(), gfx::Point(0, bottom - delta_to_bottom), HTTOP));
   ASSERT_TRUE(resizer.get());
   resizer->Drag(CalculateDragPoint(*resizer, 0, bottom), 0);
-  EXPECT_EQ("100," + base::NumberToString(bottom - kMinimumOnScreenArea) +
-                " 300x" +
-                base::NumberToString(height -
-                                     (delta_to_bottom - kMinimumOnScreenArea)),
-            window_->bounds().ToString());
+  EXPECT_EQ(gfx::Rect(100, bottom - kMinimumOnScreenArea, 300,
+                      height - (delta_to_bottom - kMinimumOnScreenArea)),
+            window_->bounds());
 }
 
 // Verifies that 'outside' check of the resizer take into account the extended
@@ -1010,9 +1001,8 @@ TEST_F(WorkspaceWindowResizerTest, DragWindowOutsideRightToSecondaryDisplay) {
       CreateResizerForTest(window_.get(), gfx::Point(window_x, 0), HTCAPTION));
   ASSERT_TRUE(resizer.get());
   resizer->Drag(CalculateDragPoint(*resizer, window_width, 0), 0);
-  EXPECT_EQ(base::NumberToString(right - kMinimumOnScreenArea) + ",100 " +
-                base::NumberToString(window_width) + "x380",
-            window_->bounds().ToString());
+  EXPECT_EQ(gfx::Rect(right - kMinimumOnScreenArea, 100, window_width, 380),
+            window_->bounds());
 
   // With secondary display.  Operation itself is same but doesn't change
   // the position because the window is still within the secondary display.
@@ -1021,9 +1011,8 @@ TEST_F(WorkspaceWindowResizerTest, DragWindowOutsideRightToSecondaryDisplay) {
                                          gfx::Insets(0, 0, 50, 0));
   window_->SetBounds(gfx::Rect(window_x, 100, window_width, 380));
   resizer->Drag(CalculateDragPoint(*resizer, window_width, 0), 0);
-  EXPECT_EQ(base::NumberToString(window_x + window_width) + ",100 " +
-                base::NumberToString(window_width) + "x380",
-            window_->bounds().ToString());
+  EXPECT_EQ(gfx::Rect(window_x + window_width, 100, window_width, 380),
+            window_->bounds());
 }
 
 // Verifies snapping to edges works.