Skip to content

Commit

Permalink
Revert of Don't save the touch view mode window state in the session …
Browse files Browse the repository at this point in the history
…restore set (https://codereview.chromium.org/252383007/)

Reason for revert:
Looks like this broke the ChromeOS bots:

http://build.chromium.org/p/chromium.chromiumos/buildstatus?builder=Linux%20ChromiumOS%20Tests%20%282%29&number=23629

Original issue's description:
> Don't save the touch view mode window state in the session restore set
> 
> BUG=354637
> TEST=unit test & visual test
> 
> Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=268353

TBR=sky@chromium.org,oshima@chromium.org,skuhne@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=354637

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@268375 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
dalecurtis@chromium.org committed May 6, 2014
1 parent a09aa4a commit 548c72f
Show file tree
Hide file tree
Showing 14 changed files with 29 additions and 241 deletions.
79 changes: 0 additions & 79 deletions ash/wm/maximize_mode/maximize_mode_window_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,8 @@
#include "ash/test/shell_test_api.h"
#include "ash/wm/mru_window_tracker.h"
#include "ash/wm/overview/window_selector_controller.h"
#include "ash/wm/window_properties.h"
#include "ash/wm/window_state.h"
#include "ash/wm/wm_event.h"
#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h"
#include "base/values.h"
#include "ui/aura/client/aura_constants.h"
Expand Down Expand Up @@ -60,21 +58,6 @@ class MaximizeModeWindowManagerTest : public test::AshTestBase {
type, bounds, gfx::Size(), true, true);
}

// Creates a window which also has a widget.
aura::Window* CreateWindowWithWidget(const gfx::Rect& bounds) {
views::Widget* widget = new views::Widget();
views::Widget::InitParams params;
params.context = CurrentContext();
// Note: The widget will get deleted with the window.
widget->Init(params);
widget->Show();
aura::Window* window = widget->GetNativeWindow();
window->SetBounds(bounds);
window->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_NORMAL);

return window;
}

// Create the Maximized mode window manager.
ash::MaximizeModeWindowManager* CreateMaximizeModeWindowManager() {
EXPECT_FALSE(maximize_mode_window_manager());
Expand Down Expand Up @@ -409,68 +392,6 @@ TEST_F(MaximizeModeWindowManagerTest,
EXPECT_EQ(rect.ToString(), fixed_window->bounds().ToString());
}

// Create a string which consists of the bounds and the state for comparison.
std::string GetPlacementString(const gfx::Rect& bounds,
ui::WindowShowState state) {
return bounds.ToString() + base::StringPrintf(" %d", state);
}

// Retrieves the window's restore state override - if any - and returns it as a
// string.
std::string GetPlacementOverride(aura::Window* window) {
gfx::Rect* bounds = window->GetProperty(ash::kRestoreBoundsOverrideKey);
if (bounds) {
gfx::Rect restore_bounds = *bounds;
ui::WindowShowState restore_state =
window->GetProperty(ash::kRestoreShowStateOverrideKey);
return GetPlacementString(restore_bounds, restore_state);
}
return std::string();
}

// Test that the restore state will be kept at its original value for
// session restauration purposes.
TEST_F(MaximizeModeWindowManagerTest, TestRestoreIntegrety) {
gfx::Rect bounds(10, 10, 200, 50);
gfx::Size empty_size;
gfx::Rect empty_bounds;
scoped_ptr<aura::Window> normal_window(
CreateWindowWithWidget(bounds));
scoped_ptr<aura::Window> maximized_window(
CreateWindowWithWidget(bounds));
wm::GetWindowState(maximized_window.get())->Maximize();

EXPECT_EQ(std::string(), GetPlacementOverride(normal_window.get()));
EXPECT_EQ(std::string(), GetPlacementOverride(maximized_window.get()));

ash::MaximizeModeWindowManager* manager = CreateMaximizeModeWindowManager();
ASSERT_TRUE(manager);

// With the maximization the override states should be returned in its
// pre-maximized state.
EXPECT_EQ(GetPlacementString(bounds, ui::SHOW_STATE_NORMAL),
GetPlacementOverride(normal_window.get()));
EXPECT_EQ(GetPlacementString(bounds, ui::SHOW_STATE_MAXIMIZED),
GetPlacementOverride(maximized_window.get()));

// Changing a window's state now does not change the returned result.
wm::GetWindowState(maximized_window.get())->Minimize();
EXPECT_EQ(GetPlacementString(bounds, ui::SHOW_STATE_MAXIMIZED),
GetPlacementOverride(maximized_window.get()));

// Destroy the manager again and check that the overrides get reset.
DestroyMaximizeModeWindowManager();
EXPECT_EQ(std::string(), GetPlacementOverride(normal_window.get()));
EXPECT_EQ(std::string(), GetPlacementOverride(maximized_window.get()));

// Changing a window's state now does not bring the overrides back.
wm::GetWindowState(maximized_window.get())->Restore();
gfx::Rect new_bounds(10, 10, 200, 50);
maximized_window->SetBounds(new_bounds);

EXPECT_EQ(std::string(), GetPlacementOverride(maximized_window.get()));
}

// Test that windows which got created before the maximizer was created can be
// destroyed while the maximizer is still running.
TEST_F(MaximizeModeWindowManagerTest, PreCreateWindowsDeleteWhileActive) {
Expand Down
20 changes: 1 addition & 19 deletions ash/wm/maximize_mode/maximize_mode_window_state.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#include "ash/wm/coordinate_conversion.h"
#include "ash/wm/maximize_mode/maximize_mode_window_manager.h"
#include "ash/wm/window_animations.h"
#include "ash/wm/window_properties.h"
#include "ash/wm/window_state.h"
#include "ash/wm/window_state_delegate.h"
#include "ash/wm/window_state_util.h"
#include "ash/wm/window_util.h"
Expand All @@ -22,8 +22,6 @@
#include "ui/aura/window_delegate.h"
#include "ui/gfx/display.h"
#include "ui/gfx/rect.h"
#include "ui/views/view_constants_aura.h"
#include "ui/views/widget/widget.h"

namespace ash {
namespace {
Expand Down Expand Up @@ -188,20 +186,6 @@ void MaximizeModeWindowState::AttachState(
wm::WindowState::State* previous_state) {
current_state_type_ = previous_state->GetType();

views::Widget* widget =
views::Widget::GetWidgetForNativeWindow(window_state->window());
if (widget) {
gfx::Rect bounds = widget->GetRestoredBounds();
if (!bounds.IsEmpty()) {
// We do not want to do a session restore to our window states. Therefore
// we tell the window to use the current default states instead.
window_state->window()->SetProperty(ash::kRestoreShowStateOverrideKey,
window_state->GetShowState());
window_state->window()->SetProperty(ash::kRestoreBoundsOverrideKey,
new gfx::Rect(widget->GetRestoredBounds()));
}
}

// Initialize the state to a good preset.
if (current_state_type_ != wm::WINDOW_STATE_TYPE_MAXIMIZED &&
current_state_type_ != wm::WINDOW_STATE_TYPE_MINIMIZED &&
Expand All @@ -215,8 +199,6 @@ void MaximizeModeWindowState::AttachState(
}

void MaximizeModeWindowState::DetachState(wm::WindowState* window_state) {
// From now on, we can use the default session restore mechanism again.
window_state->window()->ClearProperty(ash::kRestoreBoundsOverrideKey);
window_state->set_can_be_dragged(true);
}

Expand Down
10 changes: 0 additions & 10 deletions ash/wm/window_properties.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,19 +8,9 @@
#include "ui/aura/window_property.h"

DECLARE_WINDOW_PROPERTY_TYPE(ash::wm::WindowState*);
DECLARE_EXPORTED_WINDOW_PROPERTY_TYPE(ASH_EXPORT, gfx::Rect*)
DECLARE_EXPORTED_WINDOW_PROPERTY_TYPE(ASH_EXPORT, ui::WindowShowState)

namespace ash {

DEFINE_OWNED_WINDOW_PROPERTY_KEY(gfx::Rect,
kRestoreBoundsOverrideKey,
NULL);

DEFINE_WINDOW_PROPERTY_KEY(ui::WindowShowState,
kRestoreShowStateOverrideKey,
ui::SHOW_STATE_DEFAULT);

DEFINE_WINDOW_PROPERTY_KEY(bool, kStayInSameRootWindowKey, false);
DEFINE_WINDOW_PROPERTY_KEY(bool, kUsesScreenCoordinatesKey, false);
DEFINE_OWNED_WINDOW_PROPERTY_KEY(wm::WindowState,
Expand Down
13 changes: 0 additions & 13 deletions ash/wm/window_properties.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

#include "ash/ash_export.h"
#include "ui/base/ui_base_types.h"
#include "ui/gfx/rect.h"

namespace aura {
class Window;
Expand All @@ -25,18 +24,6 @@ class WindowState;

// Alphabetical sort.

// A property key which stores the bounds to restore a window to. These take
// preference over the current bounds/state. This is used by e.g. the always
// maximized mode window manager.
ASH_EXPORT extern const aura::WindowProperty<gfx::Rect*>* const
kRestoreBoundsOverrideKey;

// A property key which stores the bounds to restore a window to. These take
// preference over the current bounds/state if |kRestoreBoundsOverrideKey| is
// set. This is used by e.g. the always maximized mode window manager.
ASH_EXPORT extern const aura::WindowProperty<ui::WindowShowState>* const
kRestoreShowStateOverrideKey;

// If this is set to true, the window stays in the same root window
// even if the bounds outside of its root window is set.
// This is exported as it's used in the tests.
Expand Down
48 changes: 13 additions & 35 deletions chrome/browser/ui/views/apps/chrome_native_app_window_views.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
#include "ash/shell.h"
#include "ash/wm/immersive_fullscreen_controller.h"
#include "ash/wm/panels/panel_frame_view.h"
#include "ash/wm/window_properties.h"
#include "ash/wm/window_state.h"
#include "ash/wm/window_state_delegate.h"
#include "ash/wm/window_state_observer.h"
Expand Down Expand Up @@ -388,46 +387,25 @@ ChromeNativeAppWindowViews::CreateNonStandardAppFrame() {

// ui::BaseWindow implementation.

gfx::Rect ChromeNativeAppWindowViews::GetRestoredBounds() const {
#if defined(USE_ASH)
gfx::Rect* bounds = widget()->GetNativeWindow()->GetProperty(
ash::kRestoreBoundsOverrideKey);
if (bounds && !bounds->IsEmpty())
return *bounds;
#endif
return widget()->GetRestoredBounds();
}

ui::WindowShowState ChromeNativeAppWindowViews::GetRestoredState() const {
#if !defined(USE_ASH)
if (IsMaximized())
return ui::SHOW_STATE_MAXIMIZED;
if (IsFullscreen())
if (IsFullscreen()) {
#if defined(USE_ASH)
if (immersive_fullscreen_controller_.get() &&
immersive_fullscreen_controller_->IsEnabled()) {
// Restore windows which were previously in immersive fullscreen to
// maximized. Restoring the window to a different fullscreen type
// makes for a bad experience.
return ui::SHOW_STATE_MAXIMIZED;
}
#endif
return ui::SHOW_STATE_FULLSCREEN;
#else
}
#if defined(USE_ASH)
// Use kRestoreShowStateKey in case a window is minimized/hidden.
ui::WindowShowState restore_state = widget()->GetNativeWindow()->GetProperty(
aura::client::kRestoreShowStateKey);
if (widget()->GetNativeWindow()->GetProperty(
ash::kRestoreBoundsOverrideKey)) {
// If an override is given, we use that restore state (after filtering).
restore_state = widget()->GetNativeWindow()->GetProperty(
ash::kRestoreShowStateOverrideKey);
} else {
// Otherwise first normal states are checked.
if (IsMaximized())
return ui::SHOW_STATE_MAXIMIZED;
if (IsFullscreen()) {
if (immersive_fullscreen_controller_.get() &&
immersive_fullscreen_controller_->IsEnabled()) {
// Restore windows which were previously in immersive fullscreen to
// maximized. Restoring the window to a different fullscreen type
// makes for a bad experience.
return ui::SHOW_STATE_MAXIMIZED;
}
return ui::SHOW_STATE_FULLSCREEN;
}
}
// Whitelist states to return so that invalid and transient states
// are not saved and used to restore windows when they are recreated.
switch (restore_state) {
Expand All @@ -443,7 +421,7 @@ ui::WindowShowState ChromeNativeAppWindowViews::GetRestoredState() const {
case ui::SHOW_STATE_END:
return ui::SHOW_STATE_NORMAL;
}
#endif // !defined(USE_ASH)
#endif
return ui::SHOW_STATE_NORMAL;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ class ChromeNativeAppWindowViews : public apps::NativeAppWindowViews,
apps::AppWindowFrameView* CreateNonStandardAppFrame();

// ui::BaseWindow implementation.
virtual gfx::Rect GetRestoredBounds() const OVERRIDE;
virtual ui::WindowShowState GetRestoredState() const OVERRIDE;
virtual bool IsAlwaysOnTop() const OVERRIDE;

Expand Down
9 changes: 0 additions & 9 deletions chrome/browser/ui/views/frame/browser_frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -191,15 +191,6 @@ bool BrowserFrame::UseCustomFrame() const {
return use_custom_frame_pref_.GetValue();
}

bool BrowserFrame::ShouldSaveWindowPlacement() const {
return native_browser_frame_->ShouldSaveWindowPlacement();
}

void BrowserFrame::GetWindowPlacement(gfx::Rect* bounds,
ui::WindowShowState* show_state) const {
return native_browser_frame_->GetWindowPlacement(bounds, show_state);
}

///////////////////////////////////////////////////////////////////////////////
// BrowserFrame, views::Widget overrides:

Expand Down
7 changes: 0 additions & 7 deletions chrome/browser/ui/views/frame/browser_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,6 @@ class BrowserFrame
// Returns |true| if we should use the custom frame.
bool UseCustomFrame() const;

// Returns true when the window placement should be saved.
bool ShouldSaveWindowPlacement() const;

// Retrieves the window placement (show state and bounds) for restoring.
void GetWindowPlacement(gfx::Rect* bounds,
ui::WindowShowState* show_state) const;

// Overridden from views::Widget:
virtual views::internal::RootView* CreateRootView() OVERRIDE;
virtual views::NonClientFrameView* CreateNonClientFrameView() OVERRIDE;
Expand Down
22 changes: 0 additions & 22 deletions chrome/browser/ui/views/frame/browser_frame_ash.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

#include "chrome/browser/ui/views/frame/browser_frame_ash.h"

#include "ash/wm/window_properties.h"
#include "ash/wm/window_state.h"
#include "ash/wm/window_state_delegate.h"
#include "ash/wm/window_util.h"
Expand Down Expand Up @@ -98,27 +97,6 @@ void BrowserFrameAsh::OnWindowTargetVisibilityChanged(bool visible) {
views::NativeWidgetAura::OnWindowTargetVisibilityChanged(visible);
}

bool BrowserFrameAsh::ShouldSaveWindowPlacement() const {
return NULL != GetWidget()->GetNativeWindow()->GetProperty(
ash::kRestoreBoundsOverrideKey);
}

void BrowserFrameAsh::GetWindowPlacement(
gfx::Rect* bounds,
ui::WindowShowState* show_state) const {
gfx::Rect* override_bounds = GetWidget()->GetNativeWindow()->GetProperty(
ash::kRestoreBoundsOverrideKey);
if (override_bounds && !override_bounds->IsEmpty()) {
*bounds = *override_bounds;
*show_state = GetWidget()->GetNativeWindow()->GetProperty(
ash::kRestoreShowStateOverrideKey);
} else {
*bounds = GetWidget()->GetRestoredBounds();
*show_state = GetWidget()->GetNativeWindow()->GetProperty(
aura::client::kShowStateKey);
}
}

////////////////////////////////////////////////////////////////////////////////
// BrowserFrameAsh, NativeBrowserFrame implementation:

Expand Down
5 changes: 0 additions & 5 deletions chrome/browser/ui/views/frame/browser_frame_ash.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,6 @@ class BrowserFrameAsh : public views::NativeWidgetAura,
virtual const views::NativeWidget* AsNativeWidget() const OVERRIDE;
virtual bool UsesNativeSystemMenu() const OVERRIDE;
virtual int GetMinimizeButtonOffset() const OVERRIDE;
virtual bool ShouldSaveWindowPlacement() const OVERRIDE;
virtual void GetWindowPlacement(
gfx::Rect* bounds,
ui::WindowShowState* show_state) const OVERRIDE;


virtual ~BrowserFrameAsh();

Expand Down
Loading

0 comments on commit 548c72f

Please sign in to comment.