capture_mode: Position capture button and add an ink drop.

Change the capture session widgets to be POPUP. If they are window,
they need to be clicked first to gain capture. For example, press and
drag of capture button. Then to close, you have to first click the bar
and then click the exit button. This happens before this patch, but it
is more noticeable with the inkdrop visuals.

Change ShouldCaptureLabelEvent so that it can get events directed at it
that have no location (ENTER, CAPTURELOST, EXIT).

The capture button is now centered in the region if it can be with a
minimum amount of padding on both sides. Otherwise it will go on either
slightly above or below the region, based on its proximity to the root
bottom edge.

Test: ash_unittests CaptureModeTest.CaptureRegionCaptureButtonLocation
Bug: 1131211
Change-Id: I732a1871120ffa9b9eee4fbadd5361fc9aec0ac0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2427865Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Commit-Queue: Sammie Quon <sammiequon@chromium.org>
Cr-Commit-Position: refs/heads/master@{#815883}

ash/capture_mode/capture_label_view.cc

@@ -13,6 +13,7 @@
 #include "base/i18n/number_formatting.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/paint_vector_icon.h"
+#include "ui/gfx/text_constants.h"
 #include "ui/views/background.h"
 #include "ui/views/controls/button/label_button.h"
 #include "ui/views/controls/label.h"
@@ -52,6 +53,14 @@ CaptureLabelView::CaptureLabelView(CaptureModeSession* capture_mode_session)
   label_button_->SetPaintToLayer();
   label_button_->layer()->SetFillsBoundsOpaquely(false);
   label_button_->SetEnabledTextColors(text_color);
+  label_button_->SetHorizontalAlignment(gfx::ALIGN_CENTER);
+  label_button_->SetNotifyEnterExitOnChild(true);
+
+  label_button_->SetInkDropMode(views::InkDropHostView::InkDropMode::ON);
+  const auto ripple_attributes =
+      color_provider->GetRippleAttributes(background_color);
+  label_button_->SetInkDropVisibleOpacity(ripple_attributes.inkdrop_opacity);
+  label_button_->SetInkDropBaseColor(ripple_attributes.base_color);
 
   label_ = AddChildView(std::make_unique<views::Label>(base::string16()));
   label_->SetPaintToLayer();
@@ -157,9 +166,7 @@ void CaptureLabelView::StartCountDown(
 }
 
 void CaptureLabelView::Layout() {
-  gfx::Rect label_button_bounds = GetLocalBounds();
-  label_button_bounds.ClampToCenteredSize(label_button_->GetPreferredSize());
-  label_button_->SetBoundsRect(label_button_bounds);
+  label_button_->SetBoundsRect(GetLocalBounds());
 
   gfx::Rect label_bounds = GetLocalBounds();
   label_bounds.ClampToCenteredSize(label_->GetPreferredSize());
@@ -203,4 +210,7 @@ void CaptureLabelView::CountDown() {
   label_->SetText(base::FormatNumber(timeout_count_down_--));
 }
 
+BEGIN_METADATA(CaptureLabelView, views::View)
+END_METADATA
+
 }  // namespace ash

ash/capture_mode/capture_label_view.h

@@ -24,6 +24,8 @@ class CaptureModeSession;
 class ASH_EXPORT CaptureLabelView : public views::View,
                                     public views::ButtonListener {
  public:
+  METADATA_HEADER(CaptureLabelView);
+
   explicit CaptureLabelView(CaptureModeSession* capture_mode_session);
   CaptureLabelView(const CaptureLabelView&) = delete;
   CaptureLabelView& operator=(const CaptureLabelView&) = delete;

ash/capture_mode/capture_mode_session.cc

@@ -71,6 +71,11 @@ constexpr gfx::ShadowValue kRegionAffordanceCircleShadow2(
     6,
     SkColorSetARGB(38, 0, 0, 0));
 
+// The minimum padding on each side of the capture region. If the capture button
+// cannot be placed in the center of the capture region and maintain this
+// padding, it will be placed below or above the capture region.
+constexpr int kCaptureRegionMinimumPaddingDp = 16;
+
 // Mouse cursor warping is disabled when the capture source is a custom region.
 // Sets the mouse warp status to |enable| and return the original value.
 bool SetMouseWarpEnabled(bool enable) {
@@ -137,8 +142,9 @@ gfx::Rect GetRectEnclosingPoints(const std::vector<gfx::Point>& points) {
 views::Widget::InitParams CreateWidgetParams(aura::Window* parent,
                                              const gfx::Rect& bounds,
                                              const std::string& name) {
-  views::Widget::InitParams params(
-      views::Widget::InitParams::TYPE_WINDOW_FRAMELESS);
+  // Use a popup widget to get transient properties, such as not needing to
+  // click on the widget first to get capture before receiving events.
+  views::Widget::InitParams params(views::Widget::InitParams::TYPE_POPUP);
   params.ownership = views::Widget::InitParams::WIDGET_OWNS_NATIVE_WIDGET;
   params.opacity = views::Widget::InitParams::WindowOpacity::kTranslucent;
   params.parent = parent;
@@ -363,8 +369,8 @@ void CaptureModeSession::OnLocatedEvent(ui::LocatedEvent* event,
     return;
 
   gfx::Point location = event->location();
-  aura::Window* source = static_cast<aura::Window*>(event->target());
-  aura::Window::ConvertPointToTarget(source, current_root_, &location);
+  aura::Window* event_target = static_cast<aura::Window*>(event->target());
+  aura::Window::ConvertPointToTarget(event_target, current_root_, &location);
   const bool is_event_on_capture_bar =
       CaptureModeBarView::GetBounds(current_root_).Contains(location);
 
@@ -381,7 +387,7 @@ void CaptureModeSession::OnLocatedEvent(ui::LocatedEvent* event,
       case ui::ET_TOUCH_PRESSED:
       case ui::ET_TOUCH_MOVED: {
         gfx::Point screen_location(event->location());
-        ::wm::ConvertPointToScreen(source, &screen_location);
+        ::wm::ConvertPointToScreen(event_target, &screen_location);
         capture_window_observer_->UpdateSelectedWindowAtPosition(
             screen_location);
         break;
@@ -398,7 +404,7 @@ void CaptureModeSession::OnLocatedEvent(ui::LocatedEvent* event,
   }
 
   // Let the capture button handle any events it can handle first.
-  if (ShouldCaptureLabelHandleEvent(location))
+  if (ShouldCaptureLabelHandleEvent(event_target))
     return;
 
   // Allow events that are located on the capture mode bar to pass through so we
@@ -687,24 +693,57 @@ void CaptureModeSession::UpdateCaptureLabelWidgetBounds() {
   // For fullscreen and window capture mode, the capture label is placed in the
   // middle of the screen. For region capture mode, if it's in select phase, the
   // capture label is also placed in the middle of the screen, and if it's in
-  // fine tune phase, the capture label is placed in middle of the capture
-  // region.
+  // fine tune phase, the capture label is ideally placed in the middle of the
+  // capture region. If it cannot fit, then it will be placed slightly above or
+  // below the capture region.
   gfx::Rect bounds(current_root_->bounds());
   const gfx::Rect capture_region = controller_->user_capture_region();
+  const gfx::Size preferred_size =
+      capture_label_widget_->GetContentsView()->GetPreferredSize();
   if (controller_->source() == CaptureModeSource::kRegion &&
       !is_selecting_region_ && !capture_region.IsEmpty()) {
     bounds = capture_region;
+
+    // The capture region must be at least the size of |preferred_size| plus
+    // some padding for the capture label to be centered inside it.
+    gfx::Size capture_region_min_size = preferred_size;
+    capture_region_min_size.Enlarge(kCaptureRegionMinimumPaddingDp,
+                                    kCaptureRegionMinimumPaddingDp);
+    if (bounds.width() > capture_region_min_size.width() &&
+        bounds.height() > capture_region_min_size.height()) {
+      bounds.ClampToCenteredSize(preferred_size);
+    } else {
+      // The capture region is too small for the capture label to be inside it.
+      // Align |bounds| so that its horizontal centerpoint aligns with the
+      // capture regions centerpoint.
+      bounds.set_size(preferred_size);
+      bounds.set_x(capture_region.CenterPoint().x() -
+                   preferred_size.width() / 2);
+
+      // Try to put the capture label slightly below the capture region. If it
+      // does not fully fit in the root window bounds, place the capture label
+      // slightly above.
+      const int under_region_label_y =
+          capture_region.bottom() + kCaptureButtonDistanceFromRegionDp;
+      if (under_region_label_y + preferred_size.height() <
+          current_root_->bounds().bottom()) {
+        bounds.set_y(under_region_label_y);
+      } else {
+        bounds.set_y(capture_region.y() - kCaptureButtonDistanceFromRegionDp -
+                     preferred_size.height());
+      }
+    }
+  } else {
+    bounds.ClampToCenteredSize(preferred_size);
   }
-  bounds.ClampToCenteredSize(
-      capture_label_widget_->GetContentsView()->GetPreferredSize());
+
   capture_label_widget_->SetBounds(bounds);
 }
 
 bool CaptureModeSession::ShouldCaptureLabelHandleEvent(
-    const gfx::Point& location_in_root) {
+    aura::Window* event_target) {
   if (!capture_label_widget_ ||
-      !capture_label_widget_->GetNativeWindow()->bounds().Contains(
-          location_in_root)) {
+      capture_label_widget_->GetNativeWindow() != event_target) {
     return false;
   }
 

ash/capture_mode/capture_mode_session.h

@@ -48,6 +48,10 @@ class ASH_EXPORT CaptureModeSession : public ui::LayerOwner,
   // The vertical distance from the size label to the custom capture region.
   static constexpr int kSizeLabelYDistanceFromRegionDp = 8;
 
+  // The vertical distance of the capture button from the capture region, if the
+  // capture button does not fit inside the capture region.
+  static constexpr int kCaptureButtonDistanceFromRegionDp = 24;
+
   aura::Window* current_root() const { return current_root_; }
   CaptureModeBarView* capture_mode_bar_view() const {
     return capture_mode_bar_view_;
@@ -82,6 +86,10 @@ class ASH_EXPORT CaptureModeSession : public ui::LayerOwner,
   void OnTabletModeStarted() override;
   void OnTabletModeEnded() override;
 
+  views::Widget* capture_label_widget_for_testing() const {
+    return capture_label_widget_.get();
+  }
+
  private:
   // Gets the bounds of current window selected for |kWindow| capture source.
   gfx::Rect GetSelectedWindowBounds() const;
@@ -132,8 +140,12 @@ class ASH_EXPORT CaptureModeSession : public ui::LayerOwner,
   // Updates the capture label widget.
   void UpdateCaptureLabelWidget();
   void UpdateCaptureLabelWidgetBounds();
-  // Returns true if the capture label should handle the event.
-  bool ShouldCaptureLabelHandleEvent(const gfx::Point& location_in_root);
+
+  // Returns true if the capture label should handle the event. |event_target|
+  // is the window which is receiving the event. The capture label should handle
+  // the event if its associated window is |event_target| and its capture button
+  // child is visible.
+  bool ShouldCaptureLabelHandleEvent(aura::Window* event_target);
 
   CaptureModeController* const controller_;
 

ash/capture_mode/capture_mode_unittests.cc

@@ -536,6 +536,44 @@ TEST_F(CaptureModeTest, DimensionsLabelLocation) {
   EXPECT_EQ(nullptr, GetDimensionsLabelWindow());
 }
 
+TEST_F(CaptureModeTest, CaptureRegionCaptureButtonLocation) {
+  UpdateDisplay("800x800");
+
+  auto* controller = StartImageRegionCapture();
+
+  // Select a large region. Verify that the capture button widget is centered.
+  SelectRegion(gfx::Rect(100, 100, 600, 600));
+
+  views::Widget* capture_button_widget =
+      controller->capture_mode_session()->capture_label_widget_for_testing();
+  ASSERT_TRUE(capture_button_widget);
+  aura::Window* capture_button_window =
+      capture_button_widget->GetNativeWindow();
+  EXPECT_EQ(gfx::Point(400, 400),
+            capture_button_window->bounds().CenterPoint());
+
+  // Drag the bottom corner so that the region is too small to fit the capture
+  // button. Verify that the button is aligned horizontally and placed below the
+  // region.
+  auto* event_generator = GetEventGenerator();
+  event_generator->DragMouseTo(gfx::Point(120, 120));
+  EXPECT_EQ(gfx::Rect(100, 100, 20, 20), controller->user_capture_region());
+  EXPECT_EQ(110, capture_button_window->bounds().CenterPoint().x());
+  const int distance_from_region =
+      CaptureModeSession::kCaptureButtonDistanceFromRegionDp;
+  EXPECT_EQ(120 + distance_from_region, capture_button_window->bounds().y());
+
+  // Click inside the region to drag the entire region to the bottom of the
+  // screen. Verify that the button is aligned horizontally and placed above the
+  // region.
+  event_generator->set_current_screen_location(gfx::Point(110, 110));
+  event_generator->DragMouseTo(gfx::Point(110, 790));
+  EXPECT_EQ(gfx::Rect(100, 780, 20, 20), controller->user_capture_region());
+  EXPECT_EQ(110, capture_button_window->bounds().CenterPoint().x());
+  EXPECT_EQ(780 - distance_from_region,
+            capture_button_window->bounds().bottom());
+}
+
 TEST_F(CaptureModeTest, WindowCapture) {
   // Create 2 windows that overlap with each other.
   const gfx::Rect bounds1(0, 0, 200, 200);