ash: Restore previous show state after exiting fullscreen.

chrome/browser/ui/views/apps/chrome_native_app_window_views_aura.cc

Index: chrome/browser/ui/views/apps/chrome_native_app_window_views_aura.cc
diff --git a/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura.cc b/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura.cc
index 6b672506f49d2456b447507a35e9def2c62b049e..e995fa8c145dfead2e16d86e51982c691a0f43c6 100644
--- a/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura.cc
+++ b/chrome/browser/ui/views/apps/chrome_native_app_window_views_aura.cc
@@ -93,9 +93,9 @@ ChromeNativeAppWindowViewsAura::CreateNonStandardAppFrame() {
 }
 
 ui::WindowShowState ChromeNativeAppWindowViewsAura::GetRestoredState() const {
-  // Use kRestoreShowStateKey in case a window is minimized/hidden.
+  // Use kPreMinimizedShowStateKey in case a window is minimized/hidden.

[sky, 2017/01/18 22:59:49]

Shouldn't this check the current state and then use the appropriate
pre-mimized/fullscreen?

[Peng, 2017/01/19 14:12:38]

Looks like this function is used for saving the window's position and state.
Only the window is minimized or hidden, we need return the
state after Unmiminized() is called. If the PreMinimizedShowState is
fullscreen, we should just return the fullscreen show state I think.

[1]
https://cs.chromium.org/chromium/src/extensions/browser/app_window/app_window...

[sky, 2017/01/19 16:58:57]

This should return the current state. Why does it need special case minimized?

[Peng, 2017/01/19 20:20:03]

I think this function is used for getting and saving the window's state,
position and size. And use those information for re-create the window. For
example restarting the chrome browser. So when we restart the chrome process,
probably the users do not expect some windows are minimized (hidden). And user
may want all windows are in visible states (normal, fullscreen, docked or
maximized) I think.

[sky, 2017/01/19 21:55:38]

I believe this function is the same as
NativeWidgetDelegate::GetWindowPlacement(). I say this as
BrowserView::GetRestoredBounds(), which is overriding the same function, calls
NativeWidgetDelegate::GetWindowPlacement(). Looking at the implementation of
NativeWidgetAura::GetWindowPlacement() it seems to return the current show
state. Same thing goes for DesktopNativeWidgetAura, but that path involves a hop
to the WindowTreeHost.

[Peng, 2017/01/20 19:18:01]

I tried return current show state in this function. The app window will not
restart correctly. See [1]

[1] https://drive.google.com/file/d/0B3y_4ZHHx-3hQ0hsb1IyVWxtM1k/view

   ui::WindowShowState restore_state = widget()->GetNativeWindow()->GetProperty(

[sky, 2017/01/19 16:58:57]

Please move this to where it's needed.

[Peng, 2017/01/19 20:20:03]

Done.

-      aura::client::kRestoreShowStateKey);
+      aura::client::kPreMinimizedShowStateKey);
 
   // First normal states are checked.
   if (IsMaximized())
@@ -104,10 +104,10 @@ ui::WindowShowState ChromeNativeAppWindowViewsAura::GetRestoredState() const {
     return ui::SHOW_STATE_FULLSCREEN;
   }
 
-  if (widget()->GetNativeWindow()->GetProperty(
-          aura::client::kShowStateKey) == ui::SHOW_STATE_DOCKED ||
+  if (widget()->GetNativeWindow()->GetProperty(aura::client::kShowStateKey) ==
+          ui::SHOW_STATE_DOCKED ||

[sky, 2017/01/19 16:58:57]

Why can't this function return the showstate always? That's what
NativeWidgetAura does?

[Peng, 2017/01/19 20:20:03]

I cannot find GetRestoredState() in NativeWidgteAura.
But anyway, I am not familiar to this function, so I don't want to
change the behavior of this function. It could be risky. 

       widget()->GetNativeWindow()->GetProperty(
-          aura::client::kRestoreShowStateKey) == ui::SHOW_STATE_DOCKED) {
+          aura::client::kPreMinimizedShowStateKey) == ui::SHOW_STATE_DOCKED) {
     return ui::SHOW_STATE_DOCKED;
   }