Makes sure switching workspaces only animations windows that want to

Makes sure switching workspaces only animations windows that want to
be shown.

BUG=116337
TEST=covered by tests
R=ben@chromium.org


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=124360

[Ben Goodger (Google), 2012-03-01 00:53:11]

LGTM

On Wed, Feb 29, 2012 at 4:52 PM, <sky@chromium.org> wrote:

> Reviewers: Ben Goodger (Google),
>
> Description:
> Makes sure switching workspaces only animations windows that want to
> be shown.
>
> BUG=116337
> TEST=covered by tests
> R=ben@chromium.org
>
>
> Please review this at
http://codereview.chromium.**org/9557005/<http://codereview.chromium.org/9557...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
>
> Affected files:
>  M ash/wm/workspace/workspace_**manager.cc
>  M ash/wm/workspace/workspace_**manager_unittest.cc
>  M ui/aura/window.h
>
>
> Index: ash/wm/workspace/workspace_**manager.cc
> diff --git a/ash/wm/workspace/workspace_**manager.cc
> b/ash/wm/workspace/workspace_**manager.cc
> index 098c4e91a52de89c20abc818d9360a**171af33f14..**
> b16a35f556e4efa2102a9c743cc18c**43349ad90a 100644
> --- a/ash/wm/workspace/workspace_**manager.cc
> +++ b/ash/wm/workspace/workspace_**manager.cc
> @@ -280,7 +280,8 @@ void WorkspaceManager::**SetWindowLayerVisibility(
>     bool value) {
>   for (size_t i = 0; i < windows.size(); ++i) {
>     ui::Layer* layer = windows[i]->layer();
> -    if (layer) {
> +    // Only show the layer for windows that want to be visible.
> +    if (layer && (!value || windows[i]->TargetVisibility()**)) {
>       windows[i]->SetProperty(aura::**client::**kAnimationsDisabledKey,
>                               change_type == DONT_ANIMATE);
>       bool update_layer = true;
> Index: ash/wm/workspace/workspace_**manager_unittest.cc
> diff --git a/ash/wm/workspace/workspace_**manager_unittest.cc
> b/ash/wm/workspace/workspace_**manager_unittest.cc
> index e136100f41b965697404ded7b70778**1b0be35adc..**
> 7d9acd8d686c3fd469890c2628f733**5207a666b1 100644
> --- a/ash/wm/workspace/workspace_**manager_unittest.cc
> +++ b/ash/wm/workspace/workspace_**manager_unittest.cc
> @@ -396,5 +396,30 @@ TEST_F(WorkspaceManagerTest, SingleFullscreenWindow) {
>   EXPECT_EQ(gfx::Rect(0, 0, 250, 251), *GetRestoreBounds(w1.get()));
>  }
>
> +// Makes sure switching workspaces doesn't show transient windows.
> +TEST_F(WorkspaceManagerTest, DontShowTransientsOnSwitch) {
> +  scoped_ptr<Window> w1(CreateTestWindow());
> +  scoped_ptr<Window> w2(CreateTestWindow());
> +
> +  w1->SetBounds(gfx::Rect(0, 0, 250, 251));
> +  w2->SetBounds(gfx::Rect(0, 0, 250, 251));
> +  w1->AddTransientChild(w2.get()**);
> +
> +  w1->Show();
> +
> +  scoped_ptr<Window> w3(CreateTestWindow());
> +  w3->SetProperty(aura::client::**kShowStateKey,
> ui::SHOW_STATE_MAXIMIZED);
> +  w3->Show();
> +
> +  EXPECT_FALSE(w1->layer()->**IsDrawn());
> +  EXPECT_FALSE(w2->layer()->**IsDrawn());
> +  EXPECT_TRUE(w3->layer()->**IsDrawn());
> +
> +  w1->Show();
> +  EXPECT_TRUE(w1->layer()->**IsDrawn());
> +  EXPECT_FALSE(w2->layer()->**IsDrawn());
> +  EXPECT_FALSE(w3->layer()->**IsDrawn());
> +}
> +
>  }  // namespace internal
>  }  // namespace ash
> Index: ui/aura/window.h
> diff --git a/ui/aura/window.h b/ui/aura/window.h
> index 934bccb6bbf69405be1d5a94fab8bb**024d6c8f48..**
> 64114406f661d2b469c5fb460483c9**11e03b6073 100644
> --- a/ui/aura/window.h
> +++ b/ui/aura/window.h
> @@ -123,6 +123,10 @@ class AURA_EXPORT Window : public ui::LayerDelegate {
>   void Hide();
>   // Returns true if this window and all its ancestors are visible.
>   bool IsVisible() const;
> +  // Returns the visibility requested by this window. IsVisible() takes
> into
> +  // account the visibility of the layer and ancestors, where as this
> tracks
> +  // whether Show() without a Hide() has been invoked.
> +  bool TargetVisibility() const { return visible_; }
>
>   // Returns the window's bounds in screen coordinates.
>   gfx::Rect GetScreenBounds() const;
>
>
>

[commit-bot: I haz the power, 2012-03-01 01:04:13]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sky@chromium.org/9557005/1

[commit-bot: I haz the power, 2012-03-01 04:18:28]

Change committed as 124360