Commit 76e330d6 authored by Takashi Sakamoto's avatar Takashi Sakamoto Committed by Commit Bot

Revert "Clamshell <-> tablet mode transition"

This reverts commit 87d6424d.

Reason for revert: suspect causing ash_unittests failure on Linux Chromium OS ASan LSan Tests (1).
https://ci.chromium.org/p/chromium/builders/ci/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/34147
deterministic failures:
TabletModeWindowManagerWithClamshellSplitViewTest.ClamshellTabletTransitionTest

==23210==ERROR: AddressSanitizer: container-overflow on address 0x6020000e12b8 at pc 0x55b059128cb9 bp 0x7ffdb6f1b8d0 sp 0x7ffdb6f1b8c8
READ of size 8 at 0x6020000e12b8 thread T0
    #0 0x55b059128cb8 in ash::SplitViewDivider::~SplitViewDivider() ./../../ash/wm/splitview/split_view_divider.cc:253:19
    #1 0x55b059128f4d in ash::SplitViewDivider::~SplitViewDivider() ./../../ash/wm/splitview/split_view_divider.cc:249:39
    #2 0x55b0591499fc in ash::TabletModeController::SetTabletModeEnabledInternal(bool) ./../../ash/wm/tablet_mode/tablet_mode_controller.cc:622:16
    #3 0x55b052f19119 in ash::TabletModeWindowManagerTest::DestroyTabletModeWindowManager() ./../../ash/wm/tablet_mode/tablet_mode_window_manager_unittest.cc:138:45
    #4 0x55b052f8060b in ash::TabletModeWindowManagerWithClamshellSplitViewTest_ClamshellTabletTransitionTest_Test::TestBody() ./../../ash/wm/tablet_mode/tablet_mode_window_manager_unittest.cc:1974:3

https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8909703871929620480/+/steps/ash_unittests/0/logs/Deterministic_failure:_TabletModeWindowManagerWithClamshellSplitViewTest.ClamshellTabletTransitionTest__status_CRASH_/0

Original change's description:
> Clamshell <-> tablet mode transition
> 
> There are 3 cases in clamshell to tablet mode transition:
> 1) overview is active but splitview is not: keep overview active during
>    transition.
> 2) overview and splitview are both active: keep them both active during
>    transition.
> 3) overview and splitview are both inactive: keep the current behavior.
>    a) if the top window is a snapped window, put it in splitview
>    b) if the second top window is also a snapped window that snaps to the
>       opposite side of the screen, put it in splitview as well
>    c) if the top window is not a snapped window, maximize or fullscreen
>       all windows when entering tablet mode.
> 
> There are 4 cases in tablet to clamshell mode transition:
> 1) overview is active but splitview is not: keep overview active during
>    transition.
> 2) overview and splitview are both active: keep them both active during
>    transition.
> 3) overview is inactive but splitview is active (i.e., two snapped windows),
>    splitview will be no longer active after transition. But the two snapped
>    window will still keep snapped in clamshell mode.
> 4) overview and splitview are both inactive: keep the current behavior,
>    i.e., restore all windows to its old window state before entering tablet
>    mode.
> 
> 
> Bug: 890029
> Change-Id: I7769021f74c3e9d591b19258be2821992d780451
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1658955
> Commit-Queue: Xiaoqian Dai <xdai@chromium.org>
> Reviewed-by: Mitsuru Oshima <oshima@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#670678}

TBR=oshima@chromium.org,xdai@chromium.org

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

Bug: 890029
Change-Id: I518170a62a0cf82aff14748679e3a5f12f054f72
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1675924Reviewed-by: default avatarTakashi Sakamoto <tasak@google.com>
Commit-Queue: Takashi Sakamoto <tasak@google.com>
Cr-Commit-Position: refs/heads/master@{#671997}
parent 0f73b636
......@@ -768,10 +768,10 @@ void SplitViewController::OnWindowBoundsChanged(
const gfx::Rect& new_bounds,
ui::PropertyChangeReason reason) {
if (split_view_type_ != SplitViewType::kClamshellType ||
reason == ui::PropertyChangeReason::FROM_ANIMATION ||
!InSplitViewMode()) {
reason == ui::PropertyChangeReason::FROM_ANIMATION) {
return;
}
DCHECK(InSplitViewMode());
wm::WindowState* window_state = wm::GetWindowState(window);
const bool is_window_moved = window_state->is_dragged() &&
......@@ -1109,37 +1109,14 @@ void SplitViewController::OnDisplayMetricsChanged(
UpdateSnappedWindowsAndDividerBounds();
}
void SplitViewController::OnTabletModeStarting() {
void SplitViewController::OnTabletModeStarted() {
split_view_type_ = SplitViewType::kTabletType;
// If splitview is active when tablet mode is starting, do the clamshell mode
// splitview to tablet mode splitview transition by adding the split view
// divider bar.
if (InSplitViewMode()) {
divider_position_ = GetClosestFixedDividerPosition();
split_view_divider_ = std::make_unique<SplitViewDivider>(
this, GetDefaultSnappedWindow()->GetRootWindow());
UpdateSnappedWindowsAndDividerBounds();
NotifyDividerPositionChanged();
}
}
void SplitViewController::OnTabletModeEnding() {
if (IsClamshellSplitViewModeEnabled()) {
if (IsClamshellSplitViewModeEnabled())
split_view_type_ = SplitViewType::kClamshellType;
// If splitview is active when tablet mode is ending, simply destroy the
// split view divider bar as we don't have the bar in clamshell split view
// mode.
if (InSplitViewMode())
split_view_divider_.reset();
} else if (InSplitViewMode()) {
// If clamshell splitview mode is not enabled, fall back to the old
// behavior: end splitview and overivew and all windows will return to its
// old window state before entering tablet mode.
EndSplitView();
EndOverview();
}
EndSplitView();
}
void SplitViewController::OnTabletControllerDestroyed() {
......
......@@ -59,17 +59,18 @@ class ASH_EXPORT SplitViewController : public SplitViewNotifier,
// top of the screen.
enum SnapPosition { NONE, LEFT, RIGHT };
// Why splitview was ended.
// Why splitview was ended. For now, all reasons will be kNormal except when
// the home launcher button is pressed, an unsnappable window just got
// activated, the active user session changed, or the window dragging
// started.
enum class EndReason {
kNormal = 0,
kHomeLauncherPressed,
kUnsnappableWindowActivated,
kActiveUserChanged,
kWindowDragStarted,
// TODO(edcourtney): Consider not ending Split-View on PIP expand.
// See crbug.com/950827.
// TODO(950827): Consider not ending Split-View on PIP expand.
kPipExpanded,
kExitTabletMode,
};
// The behaviors of split view are very different when in tablet mode and in
......@@ -191,7 +192,7 @@ class ASH_EXPORT SplitViewController : public SplitViewNotifier,
uint32_t metrics) override;
// TabletModeObserver:
void OnTabletModeStarting() override;
void OnTabletModeStarted() override;
void OnTabletModeEnding() override;
void OnTabletControllerDestroyed() override;
......
......@@ -251,7 +251,8 @@ SplitViewDivider::~SplitViewDivider() {
divider_widget_->Close();
split_view_window_targeter_.reset();
for (auto* iter : observed_windows_)
RemoveObservedWindow(iter);
iter->RemoveObserver(this);
observed_windows_.clear();
}
// static
......
......@@ -906,8 +906,6 @@ void TabletModeController::ResetPauser() {
}
void TabletModeController::FinishInitTabletMode() {
for (auto& observer : tablet_mode_observers_)
observer.OnTabletModeStarting();
tablet_mode_window_manager_ = std::make_unique<TabletModeWindowManager>();
tablet_mode_window_manager_->Init();
......
......@@ -13,9 +13,6 @@ namespace ash {
// NOTE: Code in chrome should use TabletModeClientObserver.
class ASH_EXPORT TabletModeObserver {
public:
// Called when the tablet mode is about to start.
virtual void OnTabletModeStarting() {}
// Called when the tablet mode has started. Windows might still be animating
// though.
virtual void OnTabletModeStarted() {}
......
......@@ -124,10 +124,9 @@ class ASH_EXPORT TabletModeWindowManager : public aura::WindowObserver,
void ArrangeWindowsForTabletMode();
// Revert all windows to how they were arranged before tablet mode.
// |windows_in_splitview| contains the windows that were in splitview before
// entering clamshell mode.
void ArrangeWindowsForClamshellMode(
base::flat_map<aura::Window*, WindowStateType> windows_in_splitview);
// |was_in_overview| indicates whether it was in overview before entering
// desktop mode.
void ArrangeWindowsForDesktopMode(bool was_in_overview = false);
// If the given window should be handled by us, this function will add it to
// the list of known windows (remembering the initial show state).
......@@ -138,10 +137,13 @@ class ASH_EXPORT TabletModeWindowManager : public aura::WindowObserver,
bool snap = false,
bool animate_bounds_on_attach = true);
// Remove a window from our tracking list. If the window is going to be
// destroyed, do not restore its old previous window state object as it will
// send unnecessary window state change event.
void ForgetWindow(aura::Window* window, bool destroyed);
// Remove a window from our tracking list. |was_in_overview| used when
// |destroyed| is false to help handle leaving tablet mode. If the window is
// going to be destroyed, do not restore its old previous window state object
// as it will send unnecessary window state change event.
void ForgetWindow(aura::Window* window,
bool destroyed,
bool was_in_overview = false);
// Returns true when the given window should be modified in any way by us.
bool ShouldHandleWindow(aura::Window* window);
......
......@@ -197,19 +197,18 @@ TabletModeWindowState::~TabletModeWindowState() {
creator_->WindowStateDestroyed(window_);
}
void TabletModeWindowState::LeaveTabletMode(wm::WindowState* window_state) {
// Only do bounds change animation if the window is the top window or a window
// showing in splitview, and the window has changed its state. Otherwise,
// restore its bounds immediately.
EnterAnimationType animation_type =
window_state->IsSnapped() || IsTopWindow(window_state->window())
? DEFAULT
: IMMEDIATE;
void TabletModeWindowState::LeaveTabletMode(wm::WindowState* window_state,
bool was_in_overview) {
// TODO(minch): Keep the current animation if leaving tablet mode from
// overview. Need more investigation for windows' transform animation and
// updates bounds animation when overview is active.
bool use_default = was_in_overview || window_state->IsSnapped() ||
IsTopWindow(window_state->window());
if (old_state_->GetType() == window_state->GetStateType() &&
!window_state->IsNormalStateType()) {
animation_type = IMMEDIATE;
use_default = false;
}
old_state_->set_enter_animation_type(animation_type);
old_state_->set_enter_animation_type(use_default ? DEFAULT : IMMEDIATE);
// Note: When we return we will destroy ourselves with the |our_reference|.
std::unique_ptr<wm::WindowState::State> our_reference =
window_state->SetStateObject(std::move(old_state_));
......
......@@ -41,7 +41,7 @@ class TabletModeWindowState : public wm::WindowState::State {
void set_ignore_wm_events(bool ignore) { ignore_wm_events_ = ignore; }
// Leaves the tablet mode by reverting to previous state object.
void LeaveTabletMode(wm::WindowState* window_state);
void LeaveTabletMode(wm::WindowState* window_state, bool was_in_overview);
// WindowState::State overrides:
void OnWMEvent(wm::WindowState* window_state,
......
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