New issue
Advanced search Search tips

Issue 605998 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Closed: Apr 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

Restoring a window should only have one show state transition

Project Member Reported by varkha@chromium.org, Apr 22 2016

Issue description

Version: 51.0.2704.15
OS: Chrome OS

What steps will reproduce the problem?
(1) Trace current and next states in DefaultState::EnterToNextState.
(2) Minimize a window.
(3) Restore the window.

What is the expected output?
In step (2) Normal -> Minimized
In step (3) Minimized -> Normal

What do you see instead?
In step (2) Normal -> Minimized
In step (3):
 Minimized -> Normal
 Normal -> Minimized
 Minimized -> Normal

This bug also causes a extra minimizing animation that is masked for regular windows but visible when a previously docked window is restored.

This regressed after https://codereview.chromium.org/1130033003 that was fixing issue 473228.
 

Comment 2 by osh...@chromium.org, Apr 22 2016

Cc: sadrul@chromium.org
I once tried to cleanup the show state, including this problem (I think) https://codereview.chromium.org/1489843003.  Just FYI.

Comment 3 by varkha@chromium.org, Apr 22 2016

#2, I see. My change is much smaller scope - just trying to avoid unnecessary transitions.
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 25 2016

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

commit 696f45a78fb690a7f19bdbafbb8ba84be06ee056
Author: varkha <varkha@chromium.org>
Date: Mon Apr 25 17:05:23 2016

Corrects NativeWidgetAura state model to not minimize on every restore

CL https://codereview.chromium.org/1130033003 made it so that a call to Show() can make window (show state NORMAL) only to minimize it again. This makes the state change sequence unnecessarily complex and prone to having visible artifacts such as the docking restore that is described in the bug.

This CL keeps most of the changes from https://codereview.chromium.org/1130033003 but corrects Widget::Show() to only perform the extra call to Minimize() once after Widget::Init().

BUG= 605998 
TEST=Restore a previously minimized docked window.
     There should be only a fade-in animation.

Review URL: https://codereview.chromium.org/1905333004

Cr-Commit-Position: refs/heads/master@{#389506}

[modify] https://crrev.com/696f45a78fb690a7f19bdbafbb8ba84be06ee056/ui/views/widget/native_widget_aura_unittest.cc
[modify] https://crrev.com/696f45a78fb690a7f19bdbafbb8ba84be06ee056/ui/views/widget/widget.cc

Comment 5 by varkha@chromium.org, Apr 25 2016

Status: Fixed (was: Started)
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 25 2016

Labels: merge-merged-2716
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/696f45a78fb690a7f19bdbafbb8ba84be06ee056

commit 696f45a78fb690a7f19bdbafbb8ba84be06ee056
Author: varkha <varkha@chromium.org>
Date: Mon Apr 25 17:05:23 2016

Corrects NativeWidgetAura state model to not minimize on every restore

CL https://codereview.chromium.org/1130033003 made it so that a call to Show() can make window (show state NORMAL) only to minimize it again. This makes the state change sequence unnecessarily complex and prone to having visible artifacts such as the docking restore that is described in the bug.

This CL keeps most of the changes from https://codereview.chromium.org/1130033003 but corrects Widget::Show() to only perform the extra call to Minimize() once after Widget::Init().

BUG= 605998 
TEST=Restore a previously minimized docked window.
     There should be only a fade-in animation.

Review URL: https://codereview.chromium.org/1905333004

Cr-Commit-Position: refs/heads/master@{#389506}

[modify] https://crrev.com/696f45a78fb690a7f19bdbafbb8ba84be06ee056/ui/views/widget/native_widget_aura_unittest.cc
[modify] https://crrev.com/696f45a78fb690a7f19bdbafbb8ba84be06ee056/ui/views/widget/widget.cc

Bulk verified
Status: Verified (was: Fixed)
bulk verified

Sign in to add a comment