Commit 008d7ab2 authored by David Black's avatar David Black Committed by Commit Bot

Hide Assistant when interacting outside bounds.

Previously we relied on deactivation of the Assistant widget to tell us
when we should hide Assistant UI. This did not work in all cases because
the user can interact with the Desktop our Settings tray without causing
inactivation of the widget.

Now, we register an EventMonitor to watch for events on our root window
while Assistant UI is visible. If a press event happens outside our
bounds, we hide Assistant UI.

Note that in testing this did not cause any unexpected behavior with
regards to metalayer.

This CL was adapted from logic implemented here:
https://cs.chromium.org/chromium/src/ash/system/tray/tray_event_filter.cc?type=cs&g=0&l=38-115

Bug: b:115923288
Change-Id: If708427467bca4ec6f0dd12a6aa876d47d033a4c
Reviewed-on: https://chromium-review.googlesource.com/c/1256221Reviewed-by: default avatarXiaohui Chen <xiaohuic@chromium.org>
Commit-Queue: David Black <dmblack@google.com>
Cr-Commit-Position: refs/heads/master@{#596061}
parent 1c4b8ffd
...@@ -73,13 +73,8 @@ void AssistantUiController::RemoveModelObserver( ...@@ -73,13 +73,8 @@ void AssistantUiController::RemoveModelObserver(
void AssistantUiController::OnWidgetActivationChanged(views::Widget* widget, void AssistantUiController::OnWidgetActivationChanged(views::Widget* widget,
bool active) { bool active) {
if (active) { if (active)
container_view_->RequestFocus(); container_view_->RequestFocus();
} else {
// When the widget is deactivated the UI should hide. Interacting with
// the metalayer does not cause widget deactivation.
HideUi(AssistantSource::kUnspecified);
}
} }
void AssistantUiController::OnWidgetVisibilityChanged(views::Widget* widget, void AssistantUiController::OnWidgetVisibilityChanged(views::Widget* widget,
...@@ -227,16 +222,39 @@ void AssistantUiController::OnUiVisibilityChanged( ...@@ -227,16 +222,39 @@ void AssistantUiController::OnUiVisibilityChanged(
? mojom::VoiceInteractionState::RUNNING ? mojom::VoiceInteractionState::RUNNING
: mojom::VoiceInteractionState::STOPPED); : mojom::VoiceInteractionState::STOPPED);
if (new_visibility == AssistantVisibility::kHidden) { switch (new_visibility) {
// When hiding the UI, start a timer to automatically close ourselves after case AssistantVisibility::kClosed:
// |kAutoCloseThreshold|. This is to give the user an opportunity to resume // When the UI is closed, we stop the auto close timer as it may be
// their previous session before it is automatically finished. // running and also stop monitoring events.
auto_close_timer_.Start(FROM_HERE, kAutoCloseThreshold, auto_close_timer_.Stop();
event_monitor_.reset();
break;
case AssistantVisibility::kHidden:
// When hiding the UI, we start a timer to automatically close ourselves
// after |kAutoCloseThreshold|. This is to give the user an opportunity to
// resume their previous session before it is automatically finished.
auto_close_timer_.Start(
FROM_HERE, kAutoCloseThreshold,
base::BindRepeating(&AssistantUiController::CloseUi, base::BindRepeating(&AssistantUiController::CloseUi,
weak_factory_.GetWeakPtr(), weak_factory_.GetWeakPtr(),
AssistantSource::kUnspecified)); AssistantSource::kUnspecified));
} else {
// Because the UI is not visible we needn't monitor events.
event_monitor_.reset();
break;
case AssistantVisibility::kVisible:
// Upon becoming visible, we stop the auto close timer.
auto_close_timer_.Stop(); auto_close_timer_.Stop();
// We need to monitor events for the root window while we're visible to
// give us an opportunity to dismiss Assistant UI when the user starts an
// interaction outside of our bounds. TODO(dmblack): Investigate how this
// behaves in a multi-display environment.
gfx::NativeWindow root_window =
container_view_->GetWidget()->GetNativeWindow()->GetRootWindow();
event_monitor_ =
views::EventMonitor::CreateWindowMonitor(this, root_window);
break;
} }
// Metalayer should not be sticky. Disable when the UI is no longer visible. // Metalayer should not be sticky. Disable when the UI is no longer visible.
...@@ -381,6 +399,34 @@ void AssistantUiController::OnDisplayMetricsChanged( ...@@ -381,6 +399,34 @@ void AssistantUiController::OnDisplayMetricsChanged(
} }
} }
void AssistantUiController::OnMouseEvent(ui::MouseEvent* event) {
if (event->type() == ui::ET_MOUSE_PRESSED)
OnPressedEvent(*event);
}
void AssistantUiController::OnTouchEvent(ui::TouchEvent* event) {
if (event->type() == ui::ET_TOUCH_PRESSED)
OnPressedEvent(*event);
}
void AssistantUiController::OnPressedEvent(const ui::LocatedEvent& event) {
DCHECK(event.type() == ui::ET_MOUSE_PRESSED ||
event.type() == ui::ET_TOUCH_PRESSED);
const gfx::Point screen_location =
event.target() ? event.target()->GetScreenLocation(event)
: event.root_location();
const gfx::Rect screen_bounds =
container_view_->GetWidget()->GetWindowBoundsInScreen();
// Pressed events outside our widget bounds should result in hiding of
// Assistant UI. This event does not fire during a Metalayer session so we
// needn't enforce logic to prevent hiding when using the stylus.
if (!screen_bounds.Contains(screen_location))
HideUi(AssistantSource::kUnspecified);
}
void AssistantUiController::UpdateUsableWorkArea(aura::Window* root_window) { void AssistantUiController::UpdateUsableWorkArea(aura::Window* root_window) {
gfx::Rect usable_work_area; gfx::Rect usable_work_area;
gfx::Rect screen_bounds = root_window->GetBoundsInScreen(); gfx::Rect screen_bounds = root_window->GetBoundsInScreen();
......
...@@ -21,8 +21,10 @@ ...@@ -21,8 +21,10 @@
#include "base/macros.h" #include "base/macros.h"
#include "base/timer/timer.h" #include "base/timer/timer.h"
#include "ui/display/display_observer.h" #include "ui/display/display_observer.h"
#include "ui/events/event_handler.h"
#include "ui/gfx/geometry/rect.h" #include "ui/gfx/geometry/rect.h"
#include "ui/keyboard/keyboard_controller_observer.h" #include "ui/keyboard/keyboard_controller_observer.h"
#include "ui/views/event_monitor.h"
#include "ui/views/widget/widget_observer.h" #include "ui/views/widget/widget_observer.h"
namespace chromeos { namespace chromeos {
...@@ -53,7 +55,8 @@ class ASH_EXPORT AssistantUiController ...@@ -53,7 +55,8 @@ class ASH_EXPORT AssistantUiController
public DialogPlateObserver, public DialogPlateObserver,
public HighlighterController::Observer, public HighlighterController::Observer,
public keyboard::KeyboardControllerObserver, public keyboard::KeyboardControllerObserver,
public display::DisplayObserver { public display::DisplayObserver,
public ui::EventHandler {
public: public:
explicit AssistantUiController(AssistantController* assistant_controller); explicit AssistantUiController(AssistantController* assistant_controller);
~AssistantUiController() override; ~AssistantUiController() override;
...@@ -115,6 +118,10 @@ class ASH_EXPORT AssistantUiController ...@@ -115,6 +118,10 @@ class ASH_EXPORT AssistantUiController
void OnDisplayMetricsChanged(const display::Display& display, void OnDisplayMetricsChanged(const display::Display& display,
uint32_t changed_metrics) override; uint32_t changed_metrics) override;
// ui::EventHandler:
void OnMouseEvent(ui::MouseEvent* event) override;
void OnTouchEvent(ui::TouchEvent* event) override;
void ShowUi(AssistantSource source); void ShowUi(AssistantSource source);
void HideUi(AssistantSource source); void HideUi(AssistantSource source);
void CloseUi(AssistantSource source); void CloseUi(AssistantSource source);
...@@ -123,6 +130,9 @@ class ASH_EXPORT AssistantUiController ...@@ -123,6 +130,9 @@ class ASH_EXPORT AssistantUiController
AssistantContainerView* GetViewForTest(); AssistantContainerView* GetViewForTest();
private: private:
// Invoked on either a mouse or touch pressed event.
void OnPressedEvent(const ui::LocatedEvent& event);
// Updates UI mode to |ui_mode| if specified. Otherwise UI mode is updated on // Updates UI mode to |ui_mode| if specified. Otherwise UI mode is updated on
// the basis of interaction/widget visibility state. // the basis of interaction/widget visibility state.
void UpdateUiMode(base::Optional<AssistantUiMode> ui_mode = base::nullopt); void UpdateUiMode(base::Optional<AssistantUiMode> ui_mode = base::nullopt);
...@@ -146,6 +156,8 @@ class ASH_EXPORT AssistantUiController ...@@ -146,6 +156,8 @@ class ASH_EXPORT AssistantUiController
AssistantContainerView* container_view_ = AssistantContainerView* container_view_ =
nullptr; // Owned by view hierarchy. nullptr; // Owned by view hierarchy.
std::unique_ptr<views::EventMonitor> event_monitor_;
gfx::Rect keyboard_workspace_occluded_bounds_; gfx::Rect keyboard_workspace_occluded_bounds_;
// When hidden, Assistant automatically closes itself to finish the previous // When hidden, Assistant automatically closes itself to finish the previous
......
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