New issue
Advanced search Search tips

Issue 875098 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 30
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Crash happened if trying to snap the "files" app from the top of the display

Project Member Reported by minch@chromium.org, Aug 16

Issue description

Drag the "files" app from the top of the display to splitscreen when it is maximized or snapped already.

Crash happened.

Seems the problem of the SetIndicatorState
 
#0 0x7fa963239cbc base::debug::StackTrace::StackTrace()
#1 0x7fa95c882740 ash::SplitViewDragIndicators::~SplitViewDragIndicators()
#2 0x7fa95c88af96 ash::TabletModeWindowDragDelegate::~TabletModeWindowDragDelegate()
#3 0x7fa95c88694e ash::(anonymous namespace)::TabletModeAppWindowDragDelegate::~TabletModeAppWindowDragDelegate()
#4 0x7fa95c8864bf ash::TabletModeAppWindowDragController::~TabletModeAppWindowDragController()
#5 0x7fa95c85a6c7 ash::ImmersiveGestureHandlerClassic::~ImmersiveGestureHandlerClassic()
#6 0x7fa95c196989 ash::ImmersiveFullscreenController::EnableWindowObservers()
#7 0x7fa95c196b90 ash::ImmersiveFullscreenController::SetEnabled()
#8 0x561631fe620c (anonymous namespace)::NativeAppWindowStateDelegate::OnPostWindowStateTypeChange()
#9 0x7fa95c89a1ff ash::wm::WindowState::NotifyPostStateTypeChange()
#10 0x7fa95c88f0d5 ash::TabletModeWindowState::UpdateWindow()
#11 0x7fa95c899160 ash::wm::WindowState::OnWMEvent()
#12 0x7fa95c87abfd ash::SplitViewController::SnapWindow()
#13 0x7fa95c87d89e ash::SplitViewController::EndWindowDragImpl()


Above is part of the crash stack trace,
The crash happened because we disabled the immersive fullscreen if window exit fullscreen mode.
The |immersive_fullscreen_controller_| will be deleted and then the TabletModeAppWindowDragController, then TabletModeAppWindowDragDelegate, and it will crash when we try to set the indicator state inside the delegate here
https://cs.chromium.org/chromium/src/ash/wm/tablet_mode/tablet_mode_window_drag_delegate.cc?rcl=b83d07a07ce7d22d429d673cb3c4494537dac754&l=169

Since we trying to snap a window, which will trigger the window state change here
https://cs.chromium.org/chromium/src/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc?rcl=123a64cbf84a55ca88e6a77c1f84f999e96c55d4&l=124. It will disable the immersive fullscreen if window exit fullscreen but not minimized.
Status: Fixed (was: Started)
Tried to upload cl https://chromium-review.googlesource.com/c/chromium/src/+/1188460 to fix it.

But the cl merged before here https://chromium-review.googlesource.com/c/chromium/src/+/1194874 fixed this issue too.

No crash now. marked as fixed.
Status: Started (was: Fixed)
reopen since  https://chromium-review.googlesource.com/c/chromium/src/+/1194874 has been reverted again.
Will go to merge my own fix.
Project Member

Comment 5 by bugdroid1@chromium.org, Aug 30

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d2586387e0cfc4d773795c147c652765dcdf575b

commit d2586387e0cfc4d773795c147c652765dcdf575b
Author: Min Chen <minch@google.com>
Date: Thu Aug 30 01:43:40 2018

Do not disable immersive mode if exit fullscreen in tablet mode.

The previous cl
https://chromium-review.googlesource.com/c/chromium/src/+/1166254/5
disabled the immersive mode if window exits fullscreen.

This should only did for window in clamshell mode, since in tablet
mode, we still want to keep immersive mode enabled even window is
not in fullscreen mode. Like window is maximized or snapped, the
caption of the window should still be hidden and can be dragged
from top.

Bug:  875098 
Change-Id: I9727b15c19b6955467e8db89deb2586421167c80
Reviewed-on: https://chromium-review.googlesource.com/1188460
Commit-Queue: Min Chen <minch@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587400}
[modify] https://crrev.com/d2586387e0cfc4d773795c147c652765dcdf575b/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash.cc
[modify] https://crrev.com/d2586387e0cfc4d773795c147c652765dcdf575b/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura_ash_browsertest.cc

Status: Fixed (was: Started)
Issue 879398 has been merged into this issue.

Sign in to add a comment