Skip to content

Commit

Permalink
Reland "capture-selfie-cam: Show resize button conditionally"
Browse files Browse the repository at this point in the history
This reverts commit 5255bb4.

Reason for revert: This revert was reverted by mistake. The actual culprit is https://crbug.com/1313907 and https://chromium-review.googlesource.com/c/chromium/src/+/3560979/comments/8a2df609_af8457fb.

Original change's description:
> Revert "capture-selfie-cam: Show resize button conditionally"
>
> This reverts commit 187b3b7.
>
> Reason for revert: Suspected test failures in Linux ChromiumOS MSan Tests (https://ci.chromium.org/ui/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/27254/test-results)
>
> Original change's description:
> > capture-selfie-cam: Show resize button conditionally
> >
> > Per UX specs mentioned in crbug.com/1308919, the resize button should be
> > hidden by default and only show on mouse hover or tap. When the mouse
> > exits the preview widget or after tapping on the preview widget, the
> > resize button will stay visible for a predefined duration i.e. 4.5s and
> > gets hidden. Animation is added in the appear and disappear process to
> > show the fading effects.
> > Demo video:
> > https://bugs.chromium.org/p/chromium/issues/detail?id=1308919#c5
> >
> > Follow-up CL:
> > The resize button should always show when switch access is enabled.
> > (Need to get PM and UX's feedback)
> >
> > Bug: 1308919
> > Test: Added tests + manually test
> >
> > Change-Id: Ib38e114696a1b20aeaf9d82afd636cbc5ec5b55b
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3561708
> > Reviewed-by: Min Chen <minch@chromium.org>
> > Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
> > Commit-Queue: Michele Fan <michelefan@chromium.org>
> > Cr-Commit-Position: refs/heads/main@{#989619}
>
> Bug: 1308919
> Change-Id: Ibabcde5c8754dd04c575753a09ff597a1fea2956
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3576536
> Owners-Override: Kristi Park <kristipark@google.com>
> Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
> Commit-Queue: Kristi Park <kristipark@google.com>
> Cr-Commit-Position: refs/heads/main@{#989989}

Bug: 1308919
Change-Id: Ibced1788efcf4e39370d2e925037c6b14e59a37a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3576539
Auto-Submit: Ahmed Fakhry <afakhry@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Min Chen <minch@chromium.org>
Commit-Queue: Min Chen <minch@chromium.org>
Cr-Commit-Position: refs/heads/main@{#990039}
  • Loading branch information
Ahmed Fakhry authored and Chromium LUCI CQ committed Apr 7, 2022
1 parent b1f727b commit 4df42ba
Show file tree
Hide file tree
Showing 4 changed files with 214 additions and 5 deletions.
76 changes: 75 additions & 1 deletion ash/capture_mode/capture_mode_camera_preview_view.cc
Expand Up @@ -11,13 +11,15 @@
#include "ash/strings/grit/ash_strings.h"
#include "ash/style/ash_color_provider.h"
#include "base/bind.h"
#include "base/check.h"
#include "ui/aura/window_tree_host.h"
#include "ui/base/l10n/l10n_util.h"
#include "ui/base/metadata/metadata_impl_macros.h"
#include "ui/compositor/layer.h"
#include "ui/events/event.h"
#include "ui/gfx/geometry/point_f.h"
#include "ui/gfx/paint_vector_icon.h"
#include "ui/views/animation/animation_builder.h"
#include "ui/views/background.h"
#include "ui/views/controls/native/native_view_host.h"
#include "ui/views/widget/widget.h"
Expand All @@ -26,6 +28,12 @@ namespace ash {

namespace {

// The duration for the resize button fading in process.
constexpr base::TimeDelta kResizeButtonFadeInDuration = base::Milliseconds(150);

// The duration for the reize button fading out process.
constexpr base::TimeDelta kResizeButtonFadeOutDuration = base::Milliseconds(50);

gfx::PointF GetEventScreenLocation(const ui::LocatedEvent& event) {
return event.target()->GetScreenLocationF(event);
}
Expand Down Expand Up @@ -62,6 +70,14 @@ CameraPreviewView::CameraPreviewView(
AshColorProvider::Get()->GetBaseLayerColor(
AshColorProvider::BaseLayerType::kTransparent80),
resize_button_->GetPreferredSize().height() / 2.f));

// Ensure that when `FadeInResizeButton` was called first time, it animates
// from 0 to 1.
resize_button_->layer()->SetOpacity(0);

// The resize button should be hidden by default so that it doesn't hanld
// events.
resize_button_->SetVisible(false);
UpdateResizeButtonTooltip();
}

Expand Down Expand Up @@ -113,9 +129,15 @@ void CameraPreviewView::OnGestureEvent(ui::GestureEvent* event) {
/*is_touch=*/true);
break;
case ui::ET_GESTURE_END:
if (camera_controller_->is_drag_in_progress())
if (camera_controller_->is_drag_in_progress()) {
camera_controller_->EndDraggingPreview(screen_location,
/*is_touch=*/true);
}
break;
case ui::ET_GESTURE_TAP:
resize_button_hide_timer_.Stop();
FadeInResizeButton();
ScheduleRefreshResizeButtonVisibility();
break;
default:
break;
Expand All @@ -125,6 +147,16 @@ void CameraPreviewView::OnGestureEvent(ui::GestureEvent* event) {
event->SetHandled();
}

void CameraPreviewView::OnMouseEntered(const ui::MouseEvent& event) {
resize_button_hide_timer_.Stop();
FadeInResizeButton();
}

void CameraPreviewView::OnMouseExited(const ui::MouseEvent& event) {
if (!resize_button_->IsMouseHovered())
ScheduleRefreshResizeButtonVisibility();
}

void CameraPreviewView::Layout() {
const gfx::Size resize_button_size = resize_button_->GetPreferredSize();
const gfx::Rect bounds(
Expand Down Expand Up @@ -192,6 +224,48 @@ void CameraPreviewView::DisableEventHandlingInCameraVideoHostHierarchy() {
}
}

void CameraPreviewView::RefreshResizeButtonVisibility() {
if (IsMouseHovered() || resize_button_->IsMouseHovered()) {
DCHECK(resize_button_->GetVisible());
DCHECK_EQ(1.0f, resize_button_->layer()->GetTargetOpacity());
return;
}

FadeOutResizeButton();
}

void CameraPreviewView::FadeInResizeButton() {
resize_button_->SetVisible(true);

views::AnimationBuilder()
.SetPreemptionStrategy(
ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET)
.Once()
.SetDuration(kResizeButtonFadeInDuration)
.SetOpacity(resize_button_->layer(), 1.0f);
}

void CameraPreviewView::FadeOutResizeButton() {
views::AnimationBuilder()
.OnEnded(base::BindOnce(
[](base::WeakPtr<CameraPreviewView> camera_preview_view) {
if (camera_preview_view)
camera_preview_view->resize_button()->SetVisible(false);
},
weak_ptr_factory_.GetWeakPtr()))
.SetPreemptionStrategy(
ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET)
.Once()
.SetDuration(kResizeButtonFadeOutDuration)
.SetOpacity(resize_button_->layer(), 0.0f);
}

void CameraPreviewView::ScheduleRefreshResizeButtonVisibility() {
resize_button_hide_timer_.Start(
FROM_HERE, capture_mode::kResizeButtonShowDuration, this,
&CameraPreviewView::RefreshResizeButtonVisibility);
}

BEGIN_METADATA(CameraPreviewView, views::View)
END_METADATA

Expand Down
28 changes: 27 additions & 1 deletion ash/capture_mode/capture_mode_camera_preview_view.h
Expand Up @@ -8,6 +8,7 @@
#include "ash/capture_mode/camera_video_frame_renderer.h"
#include "ash/capture_mode/capture_mode_camera_controller.h"
#include "ash/capture_mode/capture_mode_session_focus_cycler.h"
#include "base/memory/weak_ptr.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "ui/base/metadata/metadata_header_macros.h"
#include "ui/gfx/geometry/size.h"
Expand Down Expand Up @@ -49,19 +50,24 @@ class CameraPreviewView
~CameraPreviewView() override;

const CameraId& camera_id() const { return camera_id_; }
CaptureModeButton* resize_button() const { return resize_button_; }

// views::View:
void AddedToWidget() override;
bool OnMousePressed(const ui::MouseEvent& event) override;
bool OnMouseDragged(const ui::MouseEvent& event) override;
void OnMouseReleased(const ui::MouseEvent& event) override;
void OnGestureEvent(ui::GestureEvent* event) override;
void OnMouseEntered(const ui::MouseEvent& event) override;
void OnMouseExited(const ui::MouseEvent& event) override;
void Layout() override;

// CaptureModeSessionFocusCycler::HighlightableView:
views::View* GetView() override;

CaptureModeButton* resize_button_for_test() const { return resize_button_; }
base::OneShotTimer* resize_button_hide_timer_for_test() {
return &resize_button_hide_timer_;
}

private:
friend class CaptureModeTestApi;
Expand All @@ -84,6 +90,19 @@ class CameraPreviewView
// `camera_video_host_view_` and all the native windows it is hosting.
void DisableEventHandlingInCameraVideoHostHierarchy();

// Gets called by the `resize_button_hide_timer_` to refresh the visibility of
// the `resize_button_` when necessary.
void RefreshResizeButtonVisibility();

// Fades in or out the `resize_button_` and updates its visibility
// accordingly.
void FadeInResizeButton();
void FadeOutResizeButton();

// Called when the mouse exits the camera preview or after the latest tap
// inside the camera preview to start the `resize_button_hide_timer_`.
void ScheduleRefreshResizeButtonVisibility();

CaptureModeCameraController* const camera_controller_;

// The ID of the camera for which this preview was created.
Expand All @@ -97,6 +116,13 @@ class CameraPreviewView
views::NativeViewHost* const camera_video_host_view_;

CaptureModeButton* const resize_button_;

// Started when the mouse exits the camera preview or after the latest tap
// inside the camera preview. Runs RefreshResizeButtonVisibility() to fade out
// the resize button if possible.
base::OneShotTimer resize_button_hide_timer_;

base::WeakPtrFactory<CameraPreviewView> weak_ptr_factory_{this};
};

} // namespace ash
Expand Down
110 changes: 107 additions & 3 deletions ash/capture_mode/capture_mode_camera_unittests.cc
Expand Up @@ -34,6 +34,7 @@
#include "base/system/system_monitor.h"
#include "base/test/bind.h"
#include "base/test/scoped_feature_list.h"
#include "base/timer/timer.h"
#include "cc/paint/skia_paint_canvas.h"
#include "media/base/video_frame.h"
#include "media/renderers/paint_canvas_video_renderer.h"
Expand All @@ -46,6 +47,8 @@
#include "ui/gfx/geometry/vector2d.h"
#include "ui/gfx/image/image_unittest_util.h"
#include "ui/gfx/paint_vector_icon.h"
#include "ui/views/view.h"
#include "ui/views/view_observer.h"
#include "ui/views/widget/widget.h"
#include "ui/wm/core/coordinate_conversion.h"

Expand Down Expand Up @@ -123,6 +126,28 @@ class CameraDevicesChangeWaiter : public CaptureModeCameraController::Observer {
int selected_camera_change_event_count_ = 0;
};

// Defines a waiter to observe the visibility change of the view.
class ViewVisibilityChangeWaiter : public views::ViewObserver {
public:
explicit ViewVisibilityChangeWaiter(views::View* view) : view_(view) {
view_->AddObserver(this);
}

~ViewVisibilityChangeWaiter() override { view_->RemoveObserver(this); }

void Wait() { wait_loop_.Run(); }

// views::ViewObserver:
void OnViewVisibilityChanged(views::View* observed_view,
views::View* starting_view) override {
wait_loop_.Quit();
}

private:
views::View* const view_;
base::RunLoop wait_loop_;
};

} // namespace

class CaptureModeCameraTest : public AshTestBase {
Expand Down Expand Up @@ -233,9 +258,7 @@ class CaptureModeCameraTest : public AshTestBase {
}

CaptureModeButton* GetPreviewResizeButton() const {
return GetCameraController()
->camera_preview_view()
->resize_button_for_test();
return GetCameraController()->camera_preview_view()->resize_button();
}

// Verifies that the camera preview is placed on the correct position based on
Expand Down Expand Up @@ -2189,6 +2212,87 @@ TEST_P(CaptureModeCameraPreviewTest, MultiDisplayResize) {
VerifyPreviewAlignment(GetCaptureBoundsInScreen());
}

// Tests the visibility of the resize button on mouse events.
TEST_P(CaptureModeCameraPreviewTest, ResizeButtonVisibilityOnMouseEvents) {
StartCaptureSessionWithParam();
CaptureModeCameraController* camera_controller = GetCameraController();
AddDefaultCamera();
camera_controller->SetSelectedCamera(CameraId(kDefaultCameraModelId, 1));
views::Widget* preview_widget = camera_controller->camera_preview_widget();
DCHECK(preview_widget);
const gfx::Rect default_preview_bounds =
preview_widget->GetWindowBoundsInScreen();

CaptureModeButton* resize_button = GetPreviewResizeButton();
auto* event_generator = GetEventGenerator();

// Tests that the resize button is hidden by default.
EXPECT_FALSE(resize_button->GetVisible());

// Tests that the resize button will show up when the mouse is entering the
// bounds of the preview widget.
event_generator->MoveMouseTo(default_preview_bounds.CenterPoint());
EXPECT_TRUE(resize_button->GetVisible());

// Tests that the resize button will stay visible while moving the mouse
// within the bounds of the preview widget.
event_generator->MoveMouseTo(default_preview_bounds.top_center());
EXPECT_TRUE(resize_button->GetVisible());

// Tests that when the mouse is exiting the bounds of the preview widget, the
// resize button will disappear after the predefined duration.
auto outside_point = default_preview_bounds.origin();
outside_point.Offset(-1, -1);
event_generator->MoveMouseTo(outside_point);

base::OneShotTimer* timer = camera_controller->camera_preview_view()
->resize_button_hide_timer_for_test();
EXPECT_TRUE(timer->IsRunning());
EXPECT_EQ(timer->GetCurrentDelay(), capture_mode::kResizeButtonShowDuration);

{
ViewVisibilityChangeWaiter waiter(resize_button);
EXPECT_TRUE(resize_button->GetVisible());
timer->FireNow();
waiter.Wait();
EXPECT_FALSE(resize_button->GetVisible());
}
}

// Tests the visibility of the resize button on tap events.
TEST_P(CaptureModeCameraPreviewTest, ResizeButtonVisibilityOnTapEvents) {
StartCaptureSessionWithParam();
CaptureModeCameraController* camera_controller = GetCameraController();
AddDefaultCamera();
camera_controller->SetSelectedCamera(CameraId(kDefaultCameraModelId, 1));
views::Widget* preview_widget = camera_controller->camera_preview_widget();
DCHECK(preview_widget);
const gfx::Rect default_preview_bounds =
preview_widget->GetWindowBoundsInScreen();

CaptureModeButton* resize_button = GetPreviewResizeButton();
auto* event_generator = GetEventGenerator();

// Tests that the resize button is hidden by default.
EXPECT_FALSE(resize_button->GetVisible());

// Tests that resize button shows up when tapping within the bounds of the
// preview widget and will fade out after the predefined duration.
event_generator->GestureTapAt(default_preview_bounds.CenterPoint());
EXPECT_TRUE(resize_button->GetVisible());
base::OneShotTimer* timer = camera_controller->camera_preview_view()
->resize_button_hide_timer_for_test();
EXPECT_TRUE(timer->IsRunning());
EXPECT_EQ(timer->GetCurrentDelay(), capture_mode::kResizeButtonShowDuration);

{
ViewVisibilityChangeWaiter waiter(resize_button);
timer->FireNow();
waiter.Wait();
EXPECT_FALSE(resize_button->GetVisible());
}
}

INSTANTIATE_TEST_SUITE_P(All,
CaptureModeCameraPreviewTest,
testing::Values(CaptureModeSource::kFullscreen,
Expand Down
5 changes: 5 additions & 0 deletions ash/capture_mode/capture_mode_constants.h
Expand Up @@ -5,6 +5,7 @@
#ifndef ASH_CAPTURE_MODE_CAPTURE_MODE_CONSTANTS_H_
#define ASH_CAPTURE_MODE_CAPTURE_MODE_CONSTANTS_H_

#include "base/time/time.h"
#include "third_party/skia/include/core/SkColor.h"
#include "ui/gfx/geometry/insets.h"
#include "ui/gfx/geometry/size.h"
Expand Down Expand Up @@ -47,6 +48,10 @@ constexpr int kSpaceBetweenCameraPreviewAndEdges = 16;
// of the camera preview.
constexpr int kSpaceBetweenResizeButtonAndCameraPreview = 12;

// The duration to continue showing resize button since the mouse exiting the
// preview bounds or the last tap on the preview widget.
constexpr base::TimeDelta kResizeButtonShowDuration = base::Milliseconds(4500);

} // namespace capture_mode

} // namespace ash
Expand Down

0 comments on commit 4df42ba

Please sign in to comment.