ash: Restore previous show state after exiting fullscreen.

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

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/ui/views/apps/chrome_native_app_window_views_aura.h"
 
 #include <utility>
 
 #include "apps/ui/views/app_window_frame_view.h"
 #include "base/macros.h"
@@ skipping 75 matching lines @@
   // window. The root window does not have a delegate, which is needed to
   // handle the event in Linux.
   window->SetEventTargeter(
       std::unique_ptr<ui::EventTargeter>(new AppWindowEasyResizeWindowTargeter(
           window, gfx::Insets(frame->resize_inside_bounds_size()), this)));
 
   return frame;
 }
 
 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())
     return ui::SHOW_STATE_MAXIMIZED;
   if (IsFullscreen()) {
     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;
   }
 
   return GetRestorableState(restore_state);
 }
 
 bool ChromeNativeAppWindowViewsAura::IsAlwaysOnTop() const {
   return widget()->IsAlwaysOnTop();
 }
 
 void ChromeNativeAppWindowViewsAura::UpdateShape(
     std::unique_ptr<SkRegion> region) {
   bool had_shape = !!shape();
 
   ChromeNativeAppWindowViews::UpdateShape(std::move(region));
 
   aura::Window* native_window = widget()->GetNativeWindow();
   if (shape() && !had_shape) {
     native_window->SetEventTargeter(std::unique_ptr<ui::EventTargeter>(
         new ShapedAppWindowTargeter(native_window, this)));
   } else if (!shape() && had_shape) {
     native_window->SetEventTargeter(std::unique_ptr<ui::EventTargeter>());
   }
 }