Skip to content

Commit

Permalink
snap-group: Add SplitViewOverviewSession exit point metrics
Browse files Browse the repository at this point in the history
This cl adds two uma metrics for the `SplitViewOverviewSession`
exit points:
1. Ash.SplitViewOverviewSession.WindowLayoutCompleteOnSessionExit;
2. Ash.SplitViewOverviewSession.SplitViewOverviewSessionExitPoint.

Metic #1 focuses on the user-initiated action in partial overview
i.e. activate a window to complete the layout or skip the pairing.
This metric will give us quick and direct access to the key success
metric of `kFasterSplitScreenSetup`.

Metric #2 provides a breakdown of the user action to trigger the
exit of split overview session, which can be user-initiated or other
reasons such as system shutdown.

This cl also fixes an inaccurate enum recorded in `OverviewEndAction`
by adding an entry of `kWindowDeactivating`.

This cl will be combiend with crrev.com/c/4953989 to complete the
entire key user metrics for the ``kFasterSplitScreenSetup` dogfood.

Fixed: b/297096670
Test: Added unit test + run existing
Change-Id: Ibc4042ffa4d15262b8bb36d0a687d637ed01b3c5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4957463
Reviewed-by: Xiaoqian Dai <xdai@chromium.org>
Commit-Queue: Michele Fan <michelefan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1213034}
  • Loading branch information
Michele Fan authored and Chromium LUCI CQ committed Oct 21, 2023
1 parent 0fd6dcd commit 459aa77
Show file tree
Hide file tree
Showing 15 changed files with 339 additions and 44 deletions.
3 changes: 1 addition & 2 deletions ash/capture_mode/capture_window_observer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ CaptureWindowObserver::CaptureWindowObserver(

CaptureWindowObserver::~CaptureWindowObserver() {
DesksController::Get()->RemoveObserver(this);
auto* shell = Shell::Get();
shell->activation_client()->RemoveObserver(this);
Shell::Get()->activation_client()->RemoveObserver(this);
StopObserving();
}

Expand Down
11 changes: 8 additions & 3 deletions ash/root_window_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,7 @@ void RootWindowController::Shutdown() {

touch_exploration_manager_.reset();
wallpaper_widget_controller_.reset();
EndSplitViewOverviewSession();
EndSplitViewOverviewSession(SplitViewOverviewSessionExitPoint::kShutdown);
CloseAmbientWidget(/*immediately=*/true);

CloseChildWindows();
Expand Down Expand Up @@ -1009,7 +1009,12 @@ void RootWindowController::StartSplitViewOverviewSession(
split_view_overview_session_->Init(action, type);
}

void RootWindowController::EndSplitViewOverviewSession() {
void RootWindowController::EndSplitViewOverviewSession(
SplitViewOverviewSessionExitPoint exit_point) {
if (!is_shutting_down_ && split_view_overview_session_) {
split_view_overview_session_
->RecordSplitViewOverviewSessionExitPointMetrics(exit_point);
}
split_view_overview_session_.reset();
}

Expand Down Expand Up @@ -1086,7 +1091,7 @@ void RootWindowController::Init(RootWindowType root_window_type) {
// Explicitly update the desks controller before notifying the ShellObservers.
// This is to make sure the desks' states are correct before clients are
// updated.
Shell::Get()->desks_controller()->OnRootWindowAdded(root_window);
shell->desks_controller()->OnRootWindowAdded(root_window);

if (root_window_type == RootWindowType::PRIMARY) {
shell->keyboard_controller()->RebuildKeyboardIfEnabled();
Expand Down
7 changes: 6 additions & 1 deletion ash/root_window_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "ash/style/ash_color_provider_source.h"
#include "ash/wm/overview/overview_metrics.h"
#include "ash/wm/overview/overview_types.h"
#include "ash/wm/splitview/split_view_overview_session.h"
#include "ash/wm/workspace/workspace_types.h"
#include "base/gtest_prod_util.h"
#include "base/memory/raw_ptr.h"
Expand Down Expand Up @@ -274,7 +275,11 @@ class ASH_EXPORT RootWindowController {
aura::Window* window,
absl::optional<OverviewStartAction> action,
absl::optional<OverviewEnterExitType> type);
void EndSplitViewOverviewSession();

// Ends the split view overview session and reports the uma metrics if it is
// active.
void EndSplitViewOverviewSession(
SplitViewOverviewSessionExitPoint exit_point);

void SetScreenRotationAnimatorForTest(
std::unique_ptr<ScreenRotationAnimator> animator);
Expand Down
3 changes: 2 additions & 1 deletion ash/wm/overview/overview_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ bool OverviewController::EndOverview(OverviewEndAction action,
if (!CanEndOverview(type))
return false;

overview_session_->set_overview_end_action(action);
ToggleOverview(type);
RecordOverviewEndAction(action);

Expand Down Expand Up @@ -395,7 +396,7 @@ void OverviewController::ToggleOverview(OverviewEnterExitType type) {
window_util::MinimizeAndHideWithoutAnimation(windows_to_minimize);
}

// Do not show mask and show during overview shutdown.
// Do not show rounded corners or shadow during overview shutdown.
overview_session_->UpdateRoundedCornersAndShadow();

for (auto& observer : observers_)
Expand Down
55 changes: 41 additions & 14 deletions ash/wm/overview/overview_grid.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,12 +49,15 @@
#include "ash/wm/overview/overview_item.h"
#include "ash/wm/overview/overview_item_base.h"
#include "ash/wm/overview/overview_item_view.h"
#include "ash/wm/overview/overview_metrics.h"
#include "ash/wm/overview/overview_session.h"
#include "ash/wm/overview/overview_types.h"
#include "ash/wm/overview/overview_utils.h"
#include "ash/wm/overview/overview_window_drag_controller.h"
#include "ash/wm/overview/scoped_overview_animation_settings.h"
#include "ash/wm/splitview/split_view_constants.h"
#include "ash/wm/splitview/split_view_divider.h"
#include "ash/wm/splitview/split_view_overview_session.h"
#include "ash/wm/splitview/split_view_utils.h"
#include "ash/wm/tablet_mode/tablet_mode_controller.h"
#include "ash/wm/window_properties.h"
Expand Down Expand Up @@ -492,6 +495,24 @@ int GetTopViewInset(const aura::Window::Windows& windows) {
return inset;
}

// Returns the corresponding `SplitViewOverviewSessionExitPoint` with the
// overview end action deduced from the given `overview_session`.
SplitViewOverviewSessionExitPoint GetSplitViewOverviewSessionExitPoint(
OverviewSession* overview_session) {
OverviewEndAction overview_end_action =
overview_session->overview_end_action();
if (overview_end_action == OverviewEndAction::kWindowActivating) {
return SplitViewOverviewSessionExitPoint::kCompleteByActivating;
} else if (overview_end_action == OverviewEndAction::kKeyEscapeOrBack ||
overview_end_action ==
OverviewEndAction::kClickingOutsideWindowsInOverview) {
return SplitViewOverviewSessionExitPoint::kSkip;
} else if (overview_end_action == OverviewEndAction::kShuttingDown) {
return SplitViewOverviewSessionExitPoint::kShutdown;
}
return SplitViewOverviewSessionExitPoint::kUnspecified;
}

} // namespace

OverviewGrid::OverviewGrid(aura::Window* root_window,
Expand Down Expand Up @@ -537,12 +558,15 @@ void OverviewGrid::Shutdown(OverviewEnterExitType exit_type) {
EndNudge();

auto* root_controller = RootWindowController::ForWindow(root_window_);
root_controller->EndSplitViewOverviewSession();
root_controller->EndSplitViewOverviewSession(
GetSplitViewOverviewSessionExitPoint(overview_session_));
SplitViewController::Get(root_window_)->RemoveObserver(this);
if (auto* animator = root_controller->GetScreenRotationAnimator()) {
animator->RemoveObserver(this);
}
Shell::Get()->wallpaper_controller()->RemoveObserver(this);

Shell* shell = Shell::Get();
shell->wallpaper_controller()->RemoveObserver(this);
grid_event_handler_.reset();

if (IsShowingSavedDeskLibrary())
Expand All @@ -559,16 +583,17 @@ void OverviewGrid::Shutdown(OverviewEnterExitType exit_type) {
}
window->Shutdown();
}
bool single_animation_in_clamshell =
(animate_count == 1 && !has_non_cover_animating) &&
!Shell::Get()->tablet_mode_controller()->InTabletMode();

const bool in_split_view =
SplitViewController::Get(root_window_)->InSplitViewMode();
// OverviewGrid in splitscreen does not include the window to be activated.
if (!window_list_.empty() || in_split_view) {
bool minimized_in_tablet = overview_session_->enter_exit_overview_type() ==
OverviewEnterExitType::kFadeOutExit;
const bool minimized_in_tablet =
overview_session_->enter_exit_overview_type() ==
OverviewEnterExitType::kFadeOutExit;
const bool single_animation_in_clamshell =
(animate_count == 1 && !has_non_cover_animating) &&
!shell->tablet_mode_controller()->InTabletMode();
// The following instance self-destructs when shutdown animation ends.
new ShutdownAnimationMetricsTrackerObserver(
root_window_->layer()->GetCompositor(), in_split_view,
Expand Down Expand Up @@ -2278,17 +2303,19 @@ SavedDeskSaveDeskButtonContainer* OverviewGrid::GetSaveDeskButtonContainer()
void OverviewGrid::OnSplitViewStateChanged(
SplitViewController::State previous_state,
SplitViewController::State state) {
if (features::IsFasterSplitScreenSetupEnabled()) {
// When an activated is auto snapped, it will send a state change and try to
// end overview here. Ignore split view state when `kFasterSplitScreenSetup`
// is enabled.
// Do nothing if overview is being shutdown.
OverviewController* overview_controller = OverviewController::Get();
if (!overview_controller->InOverviewSession()) {
return;
}

// Do nothing if overview is being shutdown.
OverviewController* overview_controller = OverviewController::Get();
if (!overview_controller->InOverviewSession())
if (window_util::IsFasterSplitScreenOrSnapGroupArm1Enabled()) {
// When an activated is auto snapped, it will send a state change and try to
// end overview here. Ignore split view state when `kFasterSplitScreenSetup`
// or `kSnapGroup` arm1 is enabled.
RefreshGridBounds(/*animate=*/false);
return;
}

SplitViewController* split_view_controller =
SplitViewController::Get(root_window_);
Expand Down
3 changes: 2 additions & 1 deletion ash/wm/overview/overview_metrics.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ enum class OverviewEndAction {
kDevTools,
kTests,
kShowGlanceables_DEPRECATED,
kMaxValue = kShowGlanceables_DEPRECATED,
kWindowDeactivating,
kMaxValue = kWindowDeactivating,
};
void RecordOverviewEndAction(OverviewEndAction type);

Expand Down
2 changes: 1 addition & 1 deletion ash/wm/overview/overview_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -890,7 +890,7 @@ void OverviewSession::OnWindowActivating(
// Cancel overview session and do not restore activation when active window
// is set to nullptr. This happens when removing a display.
RestoreWindowActivation(false);
EndOverview(OverviewEndAction::kWindowActivating);
EndOverview(OverviewEndAction::kWindowDeactivating);
return;
}

Expand Down
10 changes: 10 additions & 0 deletions ash/wm/overview/overview_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -378,10 +378,17 @@ class ASH_EXPORT OverviewSession : public display::DisplayObserver,
OverviewEnterExitType enter_exit_overview_type() const {
return enter_exit_overview_type_;
}

void set_enter_exit_overview_type(OverviewEnterExitType val) {
enter_exit_overview_type_ = val;
}

OverviewEndAction overview_end_action() const { return overview_end_action_; }

void set_overview_end_action(OverviewEndAction overview_end_action) {
overview_end_action_ = overview_end_action;
}

OverviewWindowDragController* window_drag_controller() {
return window_drag_controller_.get();
}
Expand Down Expand Up @@ -488,6 +495,9 @@ class ASH_EXPORT OverviewSession : public display::DisplayObserver,
OverviewEnterExitType enter_exit_overview_type_ =
OverviewEnterExitType::kNormal;

// Stores the action that ends the overview mode.
OverviewEndAction overview_end_action_ = OverviewEndAction::kMaxValue;

// The selected item when exiting overview mode. nullptr if no window
// selected.
raw_ptr<OverviewItemBase, DanglingUntriaged | ExperimentalAsh>
Expand Down

0 comments on commit 459aa77

Please sign in to comment.