0

Revert "ambient: Handle key press exit path"

This reverts commit 5e5acdd482.

Reason for revert: Some corner cases need to be handled.

Bug: b/175398984

Original change's description:
> ambient: Handle key press exit path
>
> Currently ambient mode container does not have focus. A key press will
> enter into the login pod.
> To be consistent with other platforms, the key press is consumed in
> ambient mode by making the ambient mode container activatable.
>
> Bug: b/173642446
> Test: new unittest
> Change-Id: I2331079635834972e292076e55bec54dffe6f507
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2577923
> Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
> Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
> Commit-Queue: Tao Wu <wutao@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#835380}

TBR=xiyuan@chromium.org,xiaohuic@chromium.org,wutao@chromium.org,chromium-scoped@luci-project-accounts.iam.gserviceaccount.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: b/173642446
Change-Id: I1789704605085aece066ad65d87241594d222619
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2593746
Commit-Queue: Tao Wu <wutao@chromium.org>
Reviewed-by: Tao Wu <wutao@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Xiaohui Chen <xiaohuic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#837320}
This commit is contained in:
Tao Wu
2020-12-15 23:30:40 +00:00
committed by Chromium LUCI CQ
parent 984f2f12f4
commit c0e676f626
11 changed files with 15 additions and 80 deletions

@@ -118,15 +118,6 @@ bool IsAmbientModeEnabled() {
pref_service->GetBoolean(ambient::prefs::kAmbientModeEnabled); pref_service->GetBoolean(ambient::prefs::kAmbientModeEnabled);
} }
bool IsAmbientWidgetVisible() {
for (auto* controller : RootWindowController::root_window_controllers()) {
auto* widget = controller->ambient_widget();
if (widget && widget->IsVisible())
return true;
}
return false;
}
class AmbientWidgetDelegate : public views::WidgetDelegate { class AmbientWidgetDelegate : public views::WidgetDelegate {
public: public:
AmbientWidgetDelegate() { SetCanMaximize(true); } AmbientWidgetDelegate() { SetCanMaximize(true); }
@@ -417,10 +408,6 @@ void AmbientController::OnAuthScanDone(
} }
void AmbientController::OnUserActivity(const ui::Event* event) { void AmbientController::OnUserActivity(const ui::Event* event) {
// If ambient widget is visible, it will handle key event by OnKeyPressed().
if (IsAmbientWidgetVisible() && event->IsKeyEvent())
return;
DismissUI(); DismissUI();
} }
@@ -476,10 +463,6 @@ bool AmbientController::IsShown() const {
return ambient_ui_model_.ui_visibility() == AmbientUiVisibility::kShown; return ambient_ui_model_.ui_visibility() == AmbientUiVisibility::kShown;
} }
void AmbientController::OnViewEvents() {
DismissUI();
}
void AmbientController::AcquireWakeLock() { void AmbientController::AcquireWakeLock() {
if (!wake_lock_) { if (!wake_lock_) {
mojo::Remote<device::mojom::WakeLockProvider> provider; mojo::Remote<device::mojom::WakeLockProvider> provider;

@@ -105,9 +105,6 @@ class ASH_EXPORT AmbientController
// constructed. // constructed.
bool IsShown() const; bool IsShown() const;
// Handles events on the ambient mode container view.
void OnViewEvents();
void RequestAccessToken( void RequestAccessToken(
AmbientAccessTokenController::AccessTokenCallback callback, AmbientAccessTokenController::AccessTokenCallback callback,
bool may_refresh_token_on_lock = false); bool may_refresh_token_on_lock = false);

@@ -474,6 +474,9 @@ TEST_F(AmbientControllerTest, ShouldDismissContainerViewOnEvents) {
gfx::Vector2d(), gfx::PointF(), gfx::PointF(), base::TimeTicks(), gfx::Vector2d(), gfx::PointF(), gfx::PointF(), base::TimeTicks(),
ui::EF_NONE, ui::EF_NONE)); ui::EF_NONE, ui::EF_NONE));
events.emplace_back(std::make_unique<ui::KeyEvent>(
ui::ET_KEY_PRESSED, ui::VKEY_SPACE, ui::EF_NONE));
events.emplace_back(std::make_unique<ui::ScrollEvent>( events.emplace_back(std::make_unique<ui::ScrollEvent>(
ui::ET_SCROLL, gfx::PointF(), gfx::PointF(), base::TimeTicks(), ui::ET_SCROLL, gfx::PointF(), gfx::PointF(), base::TimeTicks(),
ui::EF_NONE, /*x_offset=*/0.0f, ui::EF_NONE, /*x_offset=*/0.0f,
@@ -506,9 +509,9 @@ TEST_F(AmbientControllerTest, ShouldDismissAndThenComesBack) {
FastForwardTiny(); FastForwardTiny();
EXPECT_TRUE(WidgetsVisible()); EXPECT_TRUE(WidgetsVisible());
ui::MouseEvent mouse_event(ui::ET_MOUSE_PRESSED, gfx::Point(), gfx::Point(), ui::KeyEvent key_event(ui::ET_KEY_PRESSED, ui::KeyboardCode::VKEY_1,
base::TimeTicks(), ui::EF_NONE, ui::EF_NONE); ui::EF_NONE);
ambient_controller()->OnUserActivity(&mouse_event); ambient_controller()->OnUserActivity(&key_event);
FastForwardTiny(); FastForwardTiny();
EXPECT_TRUE(GetContainerViews().empty()); EXPECT_TRUE(GetContainerViews().empty());
@@ -517,40 +520,6 @@ TEST_F(AmbientControllerTest, ShouldDismissAndThenComesBack) {
EXPECT_TRUE(WidgetsVisible()); EXPECT_TRUE(WidgetsVisible());
} }
TEST_F(AmbientControllerTest, ShouldDismissContainerViewOnKeyEvent) {
// Without user interaction, should show ambient mode.
ambient_controller()->ShowUi();
EXPECT_FALSE(WidgetsVisible());
FastForwardTiny();
EXPECT_TRUE(WidgetsVisible());
CloseAmbientScreen();
// Before ambient widget is created, if there is a user interaction, will exit
// ambient mode.
ui::KeyEvent key_event(ui::ET_KEY_PRESSED, ui::KeyboardCode::VKEY_1,
ui::EF_NONE);
ambient_controller()->ShowUi();
EXPECT_FALSE(WidgetsVisible());
ambient_controller()->OnUserActivity(&key_event);
FastForwardTiny();
EXPECT_FALSE(WidgetsVisible());
// After ambient widget is created, OnUserActivity() should ignore key event.
ambient_controller()->ShowUi();
FastForwardTiny();
EXPECT_TRUE(WidgetsVisible());
ambient_controller()->OnUserActivity(&key_event);
FastForwardTiny();
EXPECT_TRUE(WidgetsVisible());
// General key press will exit ambient mode.
// Simulate key press to close the widget.
ui::test::EventGenerator* event_generator = GetEventGenerator();
event_generator->PressKey(ui::VKEY_A, /*flags=*/0);
EXPECT_FALSE(WidgetsVisible());
}
TEST_F(AmbientControllerTest, TEST_F(AmbientControllerTest,
ShouldShowAmbientScreenWithLockscreenWhenScreenIsDimmed) { ShouldShowAmbientScreenWithLockscreenWhenScreenIsDimmed) {
GetSessionControllerClient()->SetShouldLockScreenAutomatically(true); GetSessionControllerClient()->SetShouldLockScreenAutomatically(true);
@@ -737,7 +706,8 @@ TEST_F(AmbientControllerTest, ShowsOnMultipleDisplays) {
EXPECT_EQ(GetContainerViews().size(), 2u); EXPECT_EQ(GetContainerViews().size(), 2u);
// Check that each root controller has an ambient widget. // Check that each root controller has an ambient widget.
for (auto* ctrl : RootWindowController::root_window_controllers()) for (auto* ctrl : RootWindowController::root_window_controllers())
EXPECT_TRUE(ctrl->ambient_widget() && ctrl->ambient_widget()->IsVisible()); EXPECT_TRUE(ctrl->ambient_widget_for_testing() &&
ctrl->ambient_widget_for_testing()->IsVisible());
} }
TEST_F(AmbientControllerTest, RespondsToDisplayAdded) { TEST_F(AmbientControllerTest, RespondsToDisplayAdded) {
@@ -756,7 +726,8 @@ TEST_F(AmbientControllerTest, RespondsToDisplayAdded) {
EXPECT_EQ(screen->GetNumDisplays(), 2); EXPECT_EQ(screen->GetNumDisplays(), 2);
EXPECT_EQ(GetContainerViews().size(), 2u); EXPECT_EQ(GetContainerViews().size(), 2u);
for (auto* ctrl : RootWindowController::root_window_controllers()) for (auto* ctrl : RootWindowController::root_window_controllers())
EXPECT_TRUE(ctrl->ambient_widget() && ctrl->ambient_widget()->IsVisible()); EXPECT_TRUE(ctrl->ambient_widget_for_testing() &&
ctrl->ambient_widget_for_testing()->IsVisible());
} }
TEST_F(AmbientControllerTest, HandlesDisplayRemoved) { TEST_F(AmbientControllerTest, HandlesDisplayRemoved) {

@@ -31,10 +31,6 @@ AmbientBackendModel* AmbientViewDelegateImpl::GetAmbientBackendModel() {
return ambient_controller_->GetAmbientBackendModel(); return ambient_controller_->GetAmbientBackendModel();
} }
void AmbientViewDelegateImpl::OnViewEvents() {
ambient_controller_->OnViewEvents();
}
void AmbientViewDelegateImpl::OnPhotoTransitionAnimationCompleted() { void AmbientViewDelegateImpl::OnPhotoTransitionAnimationCompleted() {
for (auto& observer : view_delegate_observers_) for (auto& observer : view_delegate_observers_)
observer.OnPhotoTransitionAnimationCompleted(); observer.OnPhotoTransitionAnimationCompleted();

@@ -23,7 +23,6 @@ class AmbientViewDelegateImpl : public AmbientViewDelegate {
// AmbientViewDelegate: // AmbientViewDelegate:
AmbientBackendModel* GetAmbientBackendModel() override; AmbientBackendModel* GetAmbientBackendModel() override;
void OnViewEvents() override;
void OnPhotoTransitionAnimationCompleted() override; void OnPhotoTransitionAnimationCompleted() override;
void AddObserver(AmbientViewDelegateObserver* observer); void AddObserver(AmbientViewDelegateObserver* observer);

@@ -391,7 +391,7 @@ AmbientPhotoController* AmbientAshTestBase::photo_controller() {
std::vector<AmbientContainerView*> AmbientAshTestBase::GetContainerViews() { std::vector<AmbientContainerView*> AmbientAshTestBase::GetContainerViews() {
std::vector<AmbientContainerView*> result; std::vector<AmbientContainerView*> result;
for (auto* ctrl : RootWindowController::root_window_controllers()) { for (auto* ctrl : RootWindowController::root_window_controllers()) {
auto* widget = ctrl->ambient_widget(); auto* widget = ctrl->ambient_widget_for_testing();
if (widget) { if (widget) {
auto* view = widget->GetContentsView(); auto* view = widget->GetContentsView();
DCHECK(view && view->GetID() == kAmbientContainerView); DCHECK(view && view->GetID() == kAmbientContainerView);
@@ -402,7 +402,8 @@ std::vector<AmbientContainerView*> AmbientAshTestBase::GetContainerViews() {
} }
AmbientContainerView* AmbientAshTestBase::GetContainerView() { AmbientContainerView* AmbientAshTestBase::GetContainerView() {
auto* widget = Shell::GetPrimaryRootWindowController()->ambient_widget(); auto* widget =
Shell::GetPrimaryRootWindowController()->ambient_widget_for_testing();
if (widget) { if (widget) {
auto* container_view = widget->GetContentsView(); auto* container_view = widget->GetContentsView();

@@ -60,11 +60,6 @@ void AmbientContainerView::Layout() {
View::Layout(); View::Layout();
} }
bool AmbientContainerView::OnKeyPressed(const ui::KeyEvent& event) {
delegate_->OnViewEvents();
return true;
}
void AmbientContainerView::Init() { void AmbientContainerView::Init() {
// TODO(b/139954108): Choose a better dark mode theme color. // TODO(b/139954108): Choose a better dark mode theme color.
SetBackground(views::CreateSolidBackground(SK_ColorBLACK)); SetBackground(views::CreateSolidBackground(SK_ColorBLACK));

@@ -28,7 +28,6 @@ class ASH_EXPORT AmbientContainerView : public views::View {
const char* GetClassName() const override; const char* GetClassName() const override;
gfx::Size CalculatePreferredSize() const override; gfx::Size CalculatePreferredSize() const override;
void Layout() override; void Layout() override;
bool OnKeyPressed(const ui::KeyEvent& event) override;
private: private:
friend class AmbientAshTestBase; friend class AmbientAshTestBase;

@@ -27,9 +27,6 @@ class ASH_EXPORT AmbientViewDelegate {
// Ambient Mode. // Ambient Mode.
virtual AmbientBackendModel* GetAmbientBackendModel() = 0; virtual AmbientBackendModel* GetAmbientBackendModel() = 0;
// Invoked when user interacting with the ambient mode container view.
virtual void OnViewEvents() = 0;
// Invoked when the photo transition animation completed. // Invoked when the photo transition animation completed.
virtual void OnPhotoTransitionAnimationCompleted() = 0; virtual void OnPhotoTransitionAnimationCompleted() = 0;
}; };

@@ -33,7 +33,7 @@ constexpr std::array<int, 11> kPreDesksActivatableContainersIds = {
// List of IDs of the containers whose windows are actiavated *after* windows in // List of IDs of the containers whose windows are actiavated *after* windows in
// the desks containers. // the desks containers.
constexpr std::array<int, 5> kPostDesksActivatableContainersIds = { constexpr std::array<int, 4> kPostDesksActivatableContainersIds = {
kShellWindowId_HomeScreenContainer, kShellWindowId_HomeScreenContainer,
// Launcher and status are intentionally checked after other containers // Launcher and status are intentionally checked after other containers
@@ -42,9 +42,6 @@ constexpr std::array<int, 5> kPostDesksActivatableContainersIds = {
kShellWindowId_PipContainer, kShellWindowId_PipContainer,
kShellWindowId_ShelfContainer, kShellWindowId_ShelfContainer,
kShellWindowId_ShelfBubbleContainer, kShellWindowId_ShelfBubbleContainer,
// Ambient mode container is fullscreen and on top of other windows.
kShellWindowId_AmbientModeContainer,
}; };
} // namespace } // namespace

@@ -227,7 +227,7 @@ class ASH_EXPORT RootWindowController {
void CreateAmbientWidget(); void CreateAmbientWidget();
void CloseAmbientWidget(bool immediately); void CloseAmbientWidget(bool immediately);
views::Widget* ambient_widget() { return ambient_widget_.get(); } views::Widget* ambient_widget_for_testing() { return ambient_widget_.get(); }
// Returns accessibility panel layout manager for this root window. // Returns accessibility panel layout manager for this root window.
AccessibilityPanelLayoutManager* GetAccessibilityPanelLayoutManagerForTest(); AccessibilityPanelLayoutManager* GetAccessibilityPanelLayoutManagerForTest();