Commit cb7a6cbf authored by Meilin Wang's avatar Meilin Wang Committed by Commit Bot

Reland "ambient: Show lockscreen contents on detecting user interaction."

This reverts commit b3f1a19a.

Reason for revert: The original CL broke the test because the expected
behavior has been updated for that test case. The try bot didn't alert
this failure as the failed test was flaky. Patchset 3 fixed this
temporarily by deleting the obsolete test case. I will add more test
cases to guard the updated behavior and deflake the original test in a
follow-up CL.

Original change's description:
> Revert "ambient: Show lockscreen contents on detecting user interaction."
>
> This reverts commit 660c9cef.
>
> Reason for revert: AmbientContainerViewTest.MouseClickClosesWidgetAndStopsTimer failing deterministically on several linux-chromeos bots. Please see https://crbug.com/1068745 for more information.
>
> Original change's description:
> > ambient: Show lockscreen contents on detecting user interaction.
> >
> > In ambient mode, when user interacting with the background image by
> > mouse or gesture, lock screen contents (login pod and media control
> > view) need be shown on top for users to either do re-authentication,
> > or media control.
> >
> > This CL implements a prototype of this view transaction by:
> > 1. Show the lock screen contents, and
> > 2. Fade-out the current shown image.
> >
> > Bug: b/149246238
> > Test: manually.
> > Change-Id: Ie31c666cdfaddefc863ff8173dc185d73ef19178
> > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2110689
> > Commit-Queue: Meilin Wang <meilinw@chromium.org>
> > Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
> > Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#757120}
>
> Change-Id: I395a7c02c6aa292f66ca5d9b4aefa9156e057773
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: b/149246238, 1068745
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2139369
> Reviewed-by: Darwin Huang <huangdarwin@chromium.org>
> Commit-Queue: Darwin Huang <huangdarwin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#757223}

Bug: b/149246238, 1068745
Change-Id: I196c60cb21917dd84f9e7018950d5bf1b788b1ff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2141067
Commit-Queue: Meilin Wang <meilinw@chromium.org>
Reviewed-by: default avatarXiaohui Chen <xiaohuic@chromium.org>
Reviewed-by: default avatarXiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#760196}
parent 3103a579
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "ash/ambient/ui/ambient_container_view.h" #include "ash/ambient/ui/ambient_container_view.h"
#include "ash/ambient/util/ambient_util.h" #include "ash/ambient/util/ambient_util.h"
#include "ash/assistant/assistant_controller_impl.h" #include "ash/assistant/assistant_controller_impl.h"
#include "ash/assistant/util/animation_util.h"
#include "ash/login/ui/lock_screen.h" #include "ash/login/ui/lock_screen.h"
#include "ash/public/cpp/ambient/ambient_mode_state.h" #include "ash/public/cpp/ambient/ambient_mode_state.h"
#include "ash/public/cpp/ambient/ambient_prefs.h" #include "ash/public/cpp/ambient/ambient_prefs.h"
...@@ -93,20 +94,17 @@ void AmbientController::OnAmbientModeEnabled(bool enabled) { ...@@ -93,20 +94,17 @@ void AmbientController::OnAmbientModeEnabled(bool enabled) {
} }
void AmbientController::OnLockStateChanged(bool locked) { void AmbientController::OnLockStateChanged(bool locked) {
if (!locked) { if (locked) {
// We should already exit ambient mode at this time, as the ambient // Show ambient mode when entering lock screen.
// container needs to be closed to uncover the login port for
// re-authentication.
DCHECK(!container_view_); DCHECK(!container_view_);
return; Show();
} else {
// Destroy ambient mode after user re-login.
Destroy();
} }
// Show the ambient container on top of the lock screen.
DCHECK(!container_view_);
Start();
} }
void AmbientController::Start() { void AmbientController::Show() {
if (!CanStartAmbientMode()) { if (!CanStartAmbientMode()) {
// TODO(wutao): Show a toast to indicate that Ambient mode is not ready. // TODO(wutao): Show a toast to indicate that Ambient mode is not ready.
return; return;
...@@ -121,14 +119,34 @@ void AmbientController::Start() { ...@@ -121,14 +119,34 @@ void AmbientController::Start() {
ambient_state_.SetAmbientModeEnabled(true); ambient_state_.SetAmbientModeEnabled(true);
} }
void AmbientController::Stop() { void AmbientController::Destroy() {
ambient_state_.SetAmbientModeEnabled(false); ambient_state_.SetAmbientModeEnabled(false);
} }
void AmbientController::Toggle() { void AmbientController::Toggle() {
if (container_view_) if (container_view_)
Stop(); Destroy();
else else
Start(); Show();
}
void AmbientController::OnBackgroundPhotoEvents() {
refresh_timer_.Stop();
// Move the |AmbientModeContainer| beneath the |LockScreenWidget| to show the
// lock screen contents on top before the fade-out animation.
auto* ambient_window = container_view_->GetWidget()->GetNativeWindow();
ambient_window->parent()->StackChildAtBottom(ambient_window);
// Start fading out the current background photo.
StartFadeOutAnimation();
}
void AmbientController::StartFadeOutAnimation() {
// We fade out the |PhotoView| on its own layer instead of using the general
// layer of the widget, otherwise it will reveal the color of the lockscreen
// wallpaper beneath.
container_view_->FadeOutPhotoView();
} }
void AmbientController::CreateContainerView() { void AmbientController::CreateContainerView() {
...@@ -146,9 +164,6 @@ void AmbientController::DestroyContainerView() { ...@@ -146,9 +164,6 @@ void AmbientController::DestroyContainerView() {
} }
void AmbientController::RefreshImage() { void AmbientController::RefreshImage() {
if (!PhotoController::Get())
return;
if (photo_model_.ShouldFetchImmediately()) { if (photo_model_.ShouldFetchImmediately()) {
// TODO(b/140032139): Defer downloading image if it is animating. // TODO(b/140032139): Defer downloading image if it is animating.
base::SequencedTaskRunnerHandle::Get()->PostDelayedTask( base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
......
...@@ -45,12 +45,21 @@ class ASH_EXPORT AmbientController : public views::WidgetObserver, ...@@ -45,12 +45,21 @@ class ASH_EXPORT AmbientController : public views::WidgetObserver,
// SessionObserver: // SessionObserver:
void OnLockStateChanged(bool locked) override; void OnLockStateChanged(bool locked) override;
void Start(); // Creates and displays the ambient mode screen on top of the lock screen.
void Stop(); void Show();
// Destroys the ambient mode screen widget.
void Destroy();
// Toggle between show and destroy the ambient mode screen.
// Should be removed once we delete the shortcut entry point.
void Toggle(); void Toggle();
PhotoModel* photo_model() { return &photo_model_; } PhotoModel* photo_model() { return &photo_model_; }
// Handles user interactions on the background photo. For now the behavior
// is showing lock screen contents (login pod and media control view) on top
// while fading-out the current shown image.
void OnBackgroundPhotoEvents();
AmbientContainerView* get_container_view_for_testing() { AmbientContainerView* get_container_view_for_testing() {
return container_view_; return container_view_;
} }
...@@ -69,6 +78,8 @@ class ASH_EXPORT AmbientController : public views::WidgetObserver, ...@@ -69,6 +78,8 @@ class ASH_EXPORT AmbientController : public views::WidgetObserver,
void GetNextImage(); void GetNextImage();
void OnPhotoDownloaded(bool success, const gfx::ImageSkia& image); void OnPhotoDownloaded(bool success, const gfx::ImageSkia& image);
void StartFadeOutAnimation();
AmbientViewDelegateImpl delegate_{this}; AmbientViewDelegateImpl delegate_{this};
AmbientContainerView* container_view_ = nullptr; // Owned by view hierarchy. AmbientContainerView* container_view_ = nullptr; // Owned by view hierarchy.
PhotoModel photo_model_; PhotoModel photo_model_;
......
...@@ -21,18 +21,7 @@ PhotoModel* AmbientViewDelegateImpl::GetPhotoModel() { ...@@ -21,18 +21,7 @@ PhotoModel* AmbientViewDelegateImpl::GetPhotoModel() {
} }
void AmbientViewDelegateImpl::OnBackgroundPhotoEvents() { void AmbientViewDelegateImpl::OnBackgroundPhotoEvents() {
// Exit ambient mode by closing the widget when user interacts with the ambient_controller_->OnBackgroundPhotoEvents();
// background photo using mouse or gestures. We do this asynchronously to
// ensure that for a mouse moved event, the widget will be destroyed *after*
// its cursor has been updated in |RootView::OnMouseMoved|.
base::SequencedTaskRunnerHandle::Get()->PostTask(
FROM_HERE,
base::BindOnce(
[](const base::WeakPtr<AmbientViewDelegateImpl>& weak_ptr) {
if (weak_ptr)
weak_ptr->ambient_controller_->Stop();
},
weak_factory_.GetWeakPtr()));
} }
} // namespace ash } // namespace ash
...@@ -11,6 +11,7 @@ ...@@ -11,6 +11,7 @@
#include "ash/ambient/ui/ambient_view_delegate.h" #include "ash/ambient/ui/ambient_view_delegate.h"
#include "ash/ambient/ui/photo_view.h" #include "ash/ambient/ui/photo_view.h"
#include "ash/ambient/util/ambient_util.h" #include "ash/ambient/util/ambient_util.h"
#include "ash/assistant/util/animation_util.h"
#include "ash/login/ui/lock_screen.h" #include "ash/login/ui/lock_screen.h"
#include "ash/public/cpp/shell_window_ids.h" #include "ash/public/cpp/shell_window_ids.h"
#include "ash/shell.h" #include "ash/shell.h"
...@@ -26,6 +27,12 @@ namespace { ...@@ -26,6 +27,12 @@ namespace {
// Ambient Assistant container view appearance. // Ambient Assistant container view appearance.
constexpr int kAmbientAssistantContainerViewPreferredHeightDip = 128; constexpr int kAmbientAssistantContainerViewPreferredHeightDip = 128;
// TODO(meilinw): temporary values for dev purpose, need to be updated with the
// final spec.
constexpr float kBackgroundPhotoOpacity = 0.5f;
constexpr base::TimeDelta kBackgroundPhotoFadeOutAnimationDuration =
base::TimeDelta::FromMilliseconds(500);
aura::Window* GetContainer() { aura::Window* GetContainer() {
aura::Window* container = nullptr; aura::Window* container = nullptr;
if (ambient::util::IsShowing(LockScreen::ScreenType::kLock)) if (ambient::util::IsShowing(LockScreen::ScreenType::kLock))
...@@ -40,7 +47,7 @@ void CreateWidget(AmbientContainerView* view) { ...@@ -40,7 +47,7 @@ void CreateWidget(AmbientContainerView* view) {
params.parent = GetContainer(); params.parent = GetContainer();
params.type = views::Widget::InitParams::TYPE_WINDOW_FRAMELESS; params.type = views::Widget::InitParams::TYPE_WINDOW_FRAMELESS;
params.delegate = view; params.delegate = view;
params.name = view->GetClassName(); params.name = "AmbientModeContainer";
views::Widget* widget = new views::Widget; views::Widget* widget = new views::Widget;
widget->Init(std::move(params)); widget->Init(std::move(params));
...@@ -75,6 +82,16 @@ void AmbientContainerView::Layout() { ...@@ -75,6 +82,16 @@ void AmbientContainerView::Layout() {
kAmbientAssistantContainerViewPreferredHeightDip)); kAmbientAssistantContainerViewPreferredHeightDip));
} }
void AmbientContainerView::FadeOutPhotoView() {
DCHECK(photo_view_);
photo_view_->layer()->GetAnimator()->StartAnimation(
assistant::util::CreateLayerAnimationSequence(
assistant::util::CreateOpacityElement(
kBackgroundPhotoOpacity,
kBackgroundPhotoFadeOutAnimationDuration)));
}
void AmbientContainerView::Init() { void AmbientContainerView::Init() {
CreateWidget(this); CreateWidget(this);
// TODO(b/139954108): Choose a better dark mode theme color. // TODO(b/139954108): Choose a better dark mode theme color.
......
...@@ -26,6 +26,9 @@ class ASH_EXPORT AmbientContainerView : public views::WidgetDelegateView { ...@@ -26,6 +26,9 @@ class ASH_EXPORT AmbientContainerView : public views::WidgetDelegateView {
gfx::Size CalculatePreferredSize() const override; gfx::Size CalculatePreferredSize() const override;
void Layout() override; void Layout() override;
// Fade out the background photo.
void FadeOutPhotoView();
private: private:
void Init(); void Init();
......
...@@ -112,25 +112,4 @@ TEST_F(AmbientContainerViewTest, TimerRunningWhenShowing) { ...@@ -112,25 +112,4 @@ TEST_F(AmbientContainerViewTest, TimerRunningWhenShowing) {
EXPECT_FALSE(GetTimer().IsRunning()); EXPECT_FALSE(GetTimer().IsRunning());
} }
// Tests that a mouse click closes the widget and stops the timer.
TEST_F(AmbientContainerViewTest, MouseClickClosesWidgetAndStopsTimer) {
// Show the widget.
Toggle();
EXPECT_TRUE(GetView());
// Download |kImageBufferLength| / 2 + 1 images to fill buffer in PhotoModel,
// in order to return false in |ShouldFetchImmediately()| and start timer.
const int num_image_to_load = kImageBufferLength / 2 + 1;
task_environment()->FastForwardBy(kAnimationDuration * num_image_to_load);
EXPECT_TRUE(GetTimer().IsRunning());
// Simulate mouse click to close the widget.
ui::test::EventGenerator generator(
GetView()->GetWidget()->GetNativeWindow()->GetRootWindow());
generator.PressLeftButton();
base::RunLoop().RunUntilIdle();
EXPECT_FALSE(GetView());
EXPECT_FALSE(GetTimer().IsRunning());
}
} // namespace ash } // namespace ash
...@@ -17,7 +17,7 @@ class ASH_EXPORT AmbientViewDelegate { ...@@ -17,7 +17,7 @@ class ASH_EXPORT AmbientViewDelegate {
virtual PhotoModel* GetPhotoModel() = 0; virtual PhotoModel* GetPhotoModel() = 0;
// Invoked when user interacts with the background photo using mouse, // Invoked when user interacting with the background photo using mouse,
// touchpad, or touchscreen. // touchpad, or touchscreen.
virtual void OnBackgroundPhotoEvents() = 0; virtual void OnBackgroundPhotoEvents() = 0;
}; };
......
...@@ -15,8 +15,7 @@ namespace ash { ...@@ -15,8 +15,7 @@ namespace ash {
TestPhotoController::TestPhotoController() = default; TestPhotoController::TestPhotoController() = default;
TestPhotoController::~TestPhotoController() = default; TestPhotoController::~TestPhotoController() = default;
void TestPhotoController::GetNextImage( void TestPhotoController::GetNextImage(PhotoDownloadCallback callback) {
PhotoController::PhotoDownloadCallback callback) {
gfx::ImageSkia image = gfx::ImageSkia image =
gfx::test::CreateImageSkia(/*width=*/10, /*height=*/10); gfx::test::CreateImageSkia(/*width=*/10, /*height=*/10);
std::move(callback).Run(/*success=*/true, image); std::move(callback).Run(/*success=*/true, image);
......
...@@ -18,7 +18,7 @@ class ASH_PUBLIC_EXPORT TestPhotoController : public PhotoController { ...@@ -18,7 +18,7 @@ class ASH_PUBLIC_EXPORT TestPhotoController : public PhotoController {
~TestPhotoController() override; ~TestPhotoController() override;
// PhotoController: // PhotoController:
void GetNextImage(PhotoController::PhotoDownloadCallback callback) override; void GetNextImage(PhotoDownloadCallback callback) override;
void GetSettings(GetSettingsCallback callback) override; void GetSettings(GetSettingsCallback callback) override;
void UpdateSettings(int topic_source, void UpdateSettings(int topic_source,
UpdateSettingsCallback callback) override; UpdateSettingsCallback callback) override;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment