capture_mode: Make dimensions label show/hide properly.

In Capture Mode, when a user makes their own capture region the
dimensions label shows always.

This CL makes it so the label only shows when the region is non-empty
and the user is resizing the region, matching the spec.

Test: manual + modified existing test
Bug: 1136324
Change-Id: I56578d7f735f0c09ace6298f5b3172cbeedb5f47
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2462177
Commit-Queue: Jeremy Chinsen <chinsenj@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Sammie Quon <sammiequon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815827}

ash/capture_mode/capture_mode_session.cc

@@ -202,7 +202,7 @@ void CaptureModeSession::OnCaptureSourceChanged(CaptureModeSource new_source) {
 
   capture_mode_bar_view_->OnCaptureSourceChanged(new_source);
   SetMouseWarpEnabled(new_source != CaptureModeSource::kRegion);
-  UpdateCaptureRegionWidgets();
+  UpdateDimensionsLabelWidget(/*is_resizing=*/false);
   layer()->SchedulePaint(layer()->bounds());
   UpdateCaptureLabelWidget();
 }
@@ -468,7 +468,7 @@ void CaptureModeSession::OnLocatedEventPressed(
     } else if (!CaptureModeBarView::GetBounds(current_root_)
                     .Contains(location_in_root)) {
       is_selecting_region_ = true;
-      UpdateCaptureRegion(gfx::Rect());
+      UpdateCaptureRegion(gfx::Rect(), /*is_resizing=*/true);
     }
     return;
   }
@@ -485,7 +485,8 @@ void CaptureModeSession::OnLocatedEventDragged(
   // press location and the current location.
   if (is_selecting_region_) {
     UpdateCaptureRegion(
-        GetRectEnclosingPoints({initial_location_in_root_, location_in_root}));
+        GetRectEnclosingPoints({initial_location_in_root_, location_in_root}),
+        /*is_resizing=*/true);
     return;
   }
 
@@ -499,7 +500,7 @@ void CaptureModeSession::OnLocatedEventDragged(
     gfx::Rect new_capture_region = controller_->user_capture_region();
     new_capture_region.Offset(location_in_root - previous_location_in_root);
     new_capture_region.AdjustToFit(current_root_->bounds());
-    UpdateCaptureRegion(new_capture_region);
+    UpdateCaptureRegion(new_capture_region, /*is_resizing=*/false);
     return;
   }
 
@@ -508,7 +509,7 @@ void CaptureModeSession::OnLocatedEventDragged(
   std::vector<gfx::Point> points = anchor_points_;
   DCHECK(!points.empty());
   points.push_back(location_in_root);
-  UpdateCaptureRegion(GetRectEnclosingPoints(points));
+  UpdateCaptureRegion(GetRectEnclosingPoints(points), /*is_resizing=*/true);
 }
 
 void CaptureModeSession::OnLocatedEventReleased(
@@ -523,17 +524,19 @@ void CaptureModeSession::OnLocatedEventReleased(
       gfx::Insets(-kAffordanceCircleRadiusDp - kCaptureRegionBorderStrokePx));
   layer()->SchedulePaint(damage_region);
 
+  UpdateDimensionsLabelWidget(/*is_resizing=*/false);
+
   if (!is_selecting_region_)
     return;
 
   // After first release event, we advance to the next phase.
   is_selecting_region_ = false;
-  UpdateCaptureRegionWidgets();
   UpdateCaptureLabelWidget();
 }
 
 void CaptureModeSession::UpdateCaptureRegion(
-    const gfx::Rect& new_capture_region) {
+    const gfx::Rect& new_capture_region,
+    bool is_resizing) {
   const gfx::Rect old_capture_region = controller_->user_capture_region();
   if (old_capture_region == new_capture_region)
     return;
@@ -548,25 +551,19 @@ void CaptureModeSession::UpdateCaptureRegion(
   layer()->SchedulePaint(damage_region);
 
   controller_->set_user_capture_region(new_capture_region);
-  UpdateCaptureRegionWidgets();
+  UpdateDimensionsLabelWidget(is_resizing);
   UpdateCaptureLabelWidget();
 }
 
-void CaptureModeSession::UpdateCaptureRegionWidgets() {
-  // TODO(chinsenj): The dimensons label is always shown and the capture
-  // button label is always shown in the fine tune stage. Update this to match
-  // the specs.
-  const bool show = controller_->source() == CaptureModeSource::kRegion;
-  if (!show) {
+void CaptureModeSession::UpdateDimensionsLabelWidget(bool is_resizing) {
+  const bool should_not_show =
+      !is_resizing || controller_->source() != CaptureModeSource::kRegion ||
+      controller_->user_capture_region().IsEmpty();
+  if (should_not_show) {
     dimensions_label_widget_.reset();
     return;
   }
 
-  MaybeCreateAndUpdateDimensionsLabelWidget();
-  UpdateDimensionsLabelBounds();
-}
-
-void CaptureModeSession::MaybeCreateAndUpdateDimensionsLabelWidget() {
   if (!dimensions_label_widget_) {
     auto* parent = GetParentContainer(current_root_);
     dimensions_label_widget_ = std::make_unique<views::Widget>();
@@ -595,6 +592,8 @@ void CaptureModeSession::MaybeCreateAndUpdateDimensionsLabelWidget() {
   const gfx::Rect capture_region = controller_->user_capture_region();
   size_label->SetText(base::UTF8ToUTF16(base::StringPrintf(
       "%d x %d", capture_region.width(), capture_region.height())));
+
+  UpdateDimensionsLabelBounds();
 }
 
 void CaptureModeSession::UpdateDimensionsLabelBounds() {

ash/capture_mode/capture_mode_session.h

@@ -106,18 +106,16 @@ class ASH_EXPORT CaptureModeSession : public ui::LayerOwner,
   void OnLocatedEventDragged(const gfx::Point& location_in_root);
   void OnLocatedEventReleased(const gfx::Point& location_in_root);
 
-  // Updates the capture region and the capture region widgets.
-  void UpdateCaptureRegion(const gfx::Rect& new_capture_region);
-
-  // Updates the widgets that are used to display text/icons while selecting a
-  // capture region. They are not visible during fullscreen or window capture,
-  // and some are only visible during certain phases of region capture. This
-  // will create or destroy the widgets as needed.
-  void UpdateCaptureRegionWidgets();
-
-  // Creates |dimensions_label_widget_| if it does not exist and then set its
-  // content view to the size label view.
-  void MaybeCreateAndUpdateDimensionsLabelWidget();
+  // Updates the capture region and the capture region widgets depending on the
+  // value of |is_resizing|.
+  void UpdateCaptureRegion(const gfx::Rect& new_capture_region,
+                           bool is_resizing);
+
+  // Updates the dimensions label widget shown during a region capture session.
+  // If not |is_resizing|, not a region capture session or the capture region is
+  // empty, clear existing |dimensions_label_widget_|. Otherwise, create and
+  // update the dimensions label.
+  void UpdateDimensionsLabelWidget(bool is_resizing);
 
   // Updates the bounds of |dimensions_label_widget_| relative to the current
   // capture region. Both |dimensions_label_widget_| and its content view must

ash/capture_mode/capture_mode_unittests.cc

@@ -136,22 +136,25 @@ class CaptureModeTest : public AshTestBase {
   }
 
   // Select a region by pressing and dragging the mouse.
-  void SelectRegion(const gfx::Rect& region) {
+  void SelectRegion(const gfx::Rect& region, bool release_mouse = true) {
     auto* controller = CaptureModeController::Get();
     ASSERT_TRUE(controller->IsActive());
     ASSERT_EQ(CaptureModeSource::kRegion, controller->source());
     auto* event_generator = GetEventGenerator();
     event_generator->set_current_screen_location(region.origin());
-    event_generator->DragMouseTo(region.bottom_right());
+    event_generator->PressLeftButton();
+    event_generator->MoveMouseTo(region.bottom_right());
+    if (release_mouse)
+      event_generator->ReleaseLeftButton();
     EXPECT_EQ(region, controller->user_capture_region());
   }
 
   aura::Window* GetDimensionsLabelWindow() const {
     auto* controller = CaptureModeController::Get();
     DCHECK(controller->IsActive());
-    return controller->capture_mode_session()
-        ->dimensions_label_widget()
-        ->GetNativeWindow();
+    auto* widget =
+        controller->capture_mode_session()->dimensions_label_widget();
+    return widget ? widget->GetNativeWindow() : nullptr;
   }
 
   void WaitForCountDownToFinish() {
@@ -480,40 +483,57 @@ TEST_F(CaptureModeTest, DimensionsLabelLocation) {
   // Start Capture Mode in a region in image mode.
   StartImageRegionCapture();
 
+  // Press down and don't move the mouse. Label shouldn't display for empty
+  // capture regions.
+  auto* generator = GetEventGenerator();
+  generator->set_current_screen_location(gfx::Point(0, 0));
+  generator->PressLeftButton();
+  auto* controller = CaptureModeController::Get();
+  EXPECT_TRUE(controller->IsActive());
+  EXPECT_TRUE(controller->user_capture_region().IsEmpty());
+  EXPECT_EQ(nullptr, GetDimensionsLabelWindow());
+  generator->ReleaseLeftButton();
+
   // Press down and drag to select a large region. Verify that the dimensions
   // label is centered and that the label is below the capture region.
   gfx::Rect capture_region{100, 100, 600, 200};
-  SelectRegion(capture_region);
-
-  aura::Window* dimensions_label_window = GetDimensionsLabelWindow();
+  SelectRegion(capture_region, /*release_mouse=*/false);
   EXPECT_EQ(capture_region.CenterPoint().x(),
-            dimensions_label_window->bounds().CenterPoint().x());
+            GetDimensionsLabelWindow()->bounds().CenterPoint().x());
   EXPECT_EQ(capture_region.bottom() +
                 CaptureModeSession::kSizeLabelYDistanceFromRegionDp,
-            dimensions_label_window->bounds().y());
+            GetDimensionsLabelWindow()->bounds().y());
+  generator->ReleaseLeftButton();
+  EXPECT_EQ(nullptr, GetDimensionsLabelWindow());
 
   // Create a new capture region close to the left side of the screen such that
   // if the label was centered it would extend out of the screen.
   // The x value of the label should be the left edge of the screen (0).
   capture_region.SetRect(2, 100, 2, 100);
-  SelectRegion(capture_region);
-  EXPECT_EQ(0, dimensions_label_window->bounds().x());
+  SelectRegion(capture_region, /*release_mouse=*/false);
+  EXPECT_EQ(0, GetDimensionsLabelWindow()->bounds().x());
+  generator->ReleaseLeftButton();
+  EXPECT_EQ(nullptr, GetDimensionsLabelWindow());
 
   // Create a new capture region close to the right side of the screen such that
   // if the label was centered it would extend out of the screen.
   // The right (x + width) of the label should be the right edge of the screen
   // (800).
   capture_region.SetRect(796, 100, 2, 100);
-  SelectRegion(capture_region);
-  EXPECT_EQ(800, dimensions_label_window->bounds().right());
+  SelectRegion(capture_region, /*release_mouse=*/false);
+  EXPECT_EQ(800, GetDimensionsLabelWindow()->bounds().right());
+  generator->ReleaseLeftButton();
+  EXPECT_EQ(nullptr, GetDimensionsLabelWindow());
 
   // Create a new capture region close to the bottom side of the screen.
   // The label should now appear inside the capture region, just above the
   // bottom edge. It should be above the bottom of the screen as well.
   capture_region.SetRect(100, 700, 600, 790);
-  SelectRegion(capture_region);
+  SelectRegion(capture_region, /*release_mouse=*/false);
   EXPECT_EQ(800 - CaptureModeSession::kSizeLabelYDistanceFromRegionDp,
-            dimensions_label_window->bounds().bottom());
+            GetDimensionsLabelWindow()->bounds().bottom());
+  generator->ReleaseLeftButton();
+  EXPECT_EQ(nullptr, GetDimensionsLabelWindow());
 }
 
 TEST_F(CaptureModeTest, WindowCapture) {