m50 has this issue as well. +mastash folks in case they have any clue.

I'm not sure, maybe Scott knows of some ash/wm or widget init changes?
Can we get a regression range?

Here's where the button calls into SystemTrayDelegateChromeOS::ShowSettings():
https://code.google.com/p/chromium/codesearch#chromium/src/ash/system/chromeos/settings/tray_settings.cc&l=85

It seems to just call into chrome::Navigate here, other similar calls must have the same defect:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/settings_window_manager.cc&rcl=1460459876&l=36

+stevenjb for Settings window

Fun. This would be a good starter bug for someone.
Doesn't seem like a P1 though.

This is apps related, but not settings. Calculator/FilesApp have the same issue.

+vollick for animation expert:

so this turns out that app saves the location after the hide animation is applied, and the hide animation leaves the transform in the window after layers are detached. So one of following CLs fixes the issue:

a) Do not save the position after the window is hidden
https://codereview.chromium.org/1947993002/

b) Reset the transform used for hide animation after detaching layer
https://codereview.chromium.org/1951913003/

I think a) is probably the right approach for this particular issue, but we may also want b), because hide animation is purely for visual effect, not to change the location of the window. vollick@, what do you think?

For approach a), this is also needed for avoiding settings window shifting
https://codereview.chromium.org/1944333003

Are you sure you need it? This is necessary only if it is called before hide animation is applied.

I think it is needed. The stacktrace shows when a window is closed, FocusController::WindowLostFocusFromDispositionChange will be called within hide animation, then Widget::OnNativeWidgetActivationChanged will be called, where there is a SaveWindowPlacement().

I can know what's your concern, maybe this updated change is more reasonable:
https://codereview.chromium.org/1944333003

 

This shouldn't be a stable blocker

I admit that I'm out of my depth here, but my understanding of the two CLs are:
 a) whenever we make a change that could update the transform for a layer, we first check to see if the layer is visible before storing the transform.
 b) when we reconstitute a window, we nuke the layer transform which leaves the window where it would appear naturally.

Is that right? If so, I agree that a) should address the issue. I don't understand the relationship between DetachAndRecreateLayers and hide animations for b), however.

a)
save the window position for restore only when the window is visible. In other words, ignore if the window position changed after it's hidden.

b)
When a window is closed, ash applies the animation effects, which moves the window location (by shrinking) as a side effects.
We detach the layer when the animation starts so that we can hide the window immediately (),

https://code.google.com/p/chromium/codesearch#chromium/src/ui/wm/core/window_animations.cc&l=86

but because the transition is kept in the cloned layer

https://code.google.com/p/chromium/codesearch#chromium/src/ui/compositor/layer_owner.cc&l=33

chrome ends up saving the wrong position.

b) is not absolutely necessary to fix this issue, but I think it makes sense to reset the transform (or may be to the original transform it had) upon detaching its layer because the transform was for effects, not to move the location. That's my thinking. WDYT?

 Issue 610144  has been merged into this issue.

 Issue 608218  has been merged into this issue.

oshima, it looks to me we all agree approach a) is a reasonable way to fix this. Could we send out CLs? yours is avoid saving when invisible for apps and mine is avoid saving when invisible for browser and settings windows.

 Issue 607558  has been merged into this issue.

Please take that CL, add tests and send to review. 

I don't know what you mean by CL?

https://codereview.chromium.org/1947993002/
https://codereview.chromium.org/1944333003

Is this code to test?  I tried to download from the first link and it just showed me the code.  I'm running Chrome OS and not on Windows.

It is the code change. When applied or updated on chrome, issue will be fixed. You don't need to do anything special.

OK, I'll wait for the updated code then as I have no idea how to apply it to my machine.

Just find an edge case, Files app, which is not effectively covered in changes in comment 22, re-investigating on this. 

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

commit ac95109db90a079c1d56bc61b5a2b27b2b2fae63
Author: warx <warx@chromium.org>
Date: Wed May 18 06:10:16 2016

Hide animation may have changed the transform, when recreating layer if we don't reset the transform, the installed transform from last hiding animation (such as a closing window operation) will cause window bounds changed.

BUG= 602661 
TEST=add unittest in visibility_controller

Review-Url: https://codereview.chromium.org/1986963003
Cr-Commit-Position: refs/heads/master@{#394340}

[modify] https://crrev.com/ac95109db90a079c1d56bc61b5a2b27b2b2fae63/ui/wm/core/visibility_controller_unittest.cc
[modify] https://crrev.com/ac95109db90a079c1d56bc61b5a2b27b2b2fae63/ui/wm/core/window_animations.cc

Will this be cherry picked back to stable or only put in trunk?

[Automated comment] Less than 2 weeks to go before stable on M51, manual review required.

I hope this is reflective of my issue report -and- that it gets fixed soon.  This is absolutely annoying as hell.

https://bugs.chromium.org/p/chromium/issues/detail?id=611005#c3

This is showing fixed, it is not.

Version 51.0.2704.55 beta (64-bit)
Platform 8172.39.0 (Official Build) beta-channel lulu

return to Started, waiting on M51 merge review

Thank you for the update.

+sshruthi since this is a change on the Chrome side.

This seems a bit late for R51 at this point, I am leaning toward not merging for R51, since this was already not considered a stable blocker.

Not to be an annoying user, but this is very very very very very annoying.  I find myself frustrated by each app reopen requires me to adjust everything again and again.  

This makes a stable ChromeOS device act like a developer preview.  

It looks like this fix has baked on canary for more than a week. How do things look here? Has this been verified on canary and no other issues/regressions?

Merge approved for M51 (branch 2704). It looks like a straight-forward fix and warx@ confirmed everything looks good on canary.

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

commit 0d99651952addde3c579d2ffeb3f02fc1912c48f
Author: xdai <xdai@chromium.org>
Date: Fri May 27 22:58:27 2016

[Merge to M51] Fix the issue that Apps window keeps moving down to the right when reopened.

It's merged into M51 on behalf of warx@. Hide animation may have changed the transform, when recreating layer if we don't reset the transform, the installed transform from last hiding animation (such as a closing window operation) will cause window bounds changed.

BUG= 602661 
TBR=sky@chromium.org
TEST=add unittest in visibility_controller

Review-Url: https://codereview.chromium.org/1986963003
Cr-Commit-Position: refs/heads/master@{#394340}
(cherry picked from commit ac95109db90a079c1d56bc61b5a2b27b2b2fae63)

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

Cr-Commit-Position: refs/branch-heads/2704@{#674}
Cr-Branched-From: 6e53600def8f60d8c632fadc70d7c1939ccea347-refs/heads/master@{#386251}

[modify] https://crrev.com/0d99651952addde3c579d2ffeb3f02fc1912c48f/ui/wm/core/visibility_controller_unittest.cc
[modify] https://crrev.com/0d99651952addde3c579d2ffeb3f02fc1912c48f/ui/wm/core/window_animations.cc

Not fixed on 51.0.2704.55. 

1.Files app still moves to the right but only slightly down
2.Status area > settings > window moves a lot down and right
3.Keep app same as #2 settings window.
4.Open Drive as window > same as #2

Mozilla/5.0 (X11; CrOS armv7l 8172.39.0) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.55 Safari/537.36

#CBC-RS/TC-watchlist

From my point of view: https://chromium.googlesource.com/chromium/src.git/+log/0d99651952addde3c579d2ffeb3f02fc1912c48f/
should be effective since 51.0.2704.71

Sorry for jumping the gun. I should have checked your point of view.

Thanks for fixing this annoying issue.

Thanks, looking forward to a 51 push to my Toshiba Chromebook.  Ready to see this go awa.

 Issue 611005  has been merged into this issue.

I can confirm this is fixed for me on 51.0.2704.79 (Guado) but another serious audio issue needs to be triaged ASAP >> https://bugs.chromium.org/p/chromium/issues/detail?id=617273

There was an update today on the beta channel and at least without being connected to an external display it looks like this might be resolved but I'll know for sure Monday when I'm connected to my external monitor.

Version 51.0.2704.79 beta (64-bit)
Platform 8172.47.0 (Official Build) beta-channel lulu

I can confirm this is resolved now.

Version 51.0.2704.79 beta (64-bit)
Platform 8172.47.0 (Official Build) beta-channel lulu