Fixes bug where FocusController would stack a layer directly above a

ash/wm/workspace/workspace_manager_unittest.cc

 // Copyright (c) 2012 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 "ash/wm/workspace/workspace_manager.h"
 
+#include <map>
+
 #include "ash/ash_switches.h"
 #include "ash/root_window_controller.h"
 #include "ash/screen_ash.h"
 #include "ash/shelf/shelf_layout_manager.h"
 #include "ash/shelf/shelf_widget.h"
 #include "ash/shell.h"
 #include "ash/shell_window_ids.h"
 #include "ash/system/status_area_widget.h"
 #include "ash/test/ash_test_base.h"
 #include "ash/test/shell_test_api.h"
@@ skipping 15 matching lines @@
 #include "ui/compositor/layer.h"
 #include "ui/compositor/scoped_animation_duration_scale_mode.h"
 #include "ui/gfx/screen.h"
 #include "ui/views/widget/widget.h"
 
 using aura::Window;
 
 namespace ash {
 namespace internal {
 
+// Returns a string containing the names of all the children of |window| (in
+// order). Each entry is separated by a space.
+std::string GetWindowNames(const aura::Window* window) {
+  std::string result;
+  for (size_t i = 0; i < window->children().size(); ++i) {
+    if (i != 0)
+      result += " ";
+    result += window->children()[i]->name();
+  }
+  return result;
+}
+
+// Returns a string containing the names of windows corresponding to each of the
+// child layers of |window|'s layer. Any layers that don't correspond to a child
+// Window of |window| are ignored. The result is ordered based on the layer
+// ordering.
+std::string GetLayerNames(const aura::Window* window) {
+  typedef std::map<const ui::Layer*, std::string> LayerToWindowNameMap;
+  LayerToWindowNameMap window_names;
+  for (size_t i = 0; i < window->children().size(); ++i) {
+    window_names[window->children()[i]->layer()] =
+        window->children()[i]->name();
+  }
+
+  std::string result;
+  const std::vector<ui::Layer*>& layers(window->layer()->children());
+  for (size_t i = 0; i < layers.size(); ++i) {
+    LayerToWindowNameMap::iterator layer_i =
+        window_names.find(layers[i]);
+    if (layer_i != window_names.end()) {
+      if (!result.empty())
+        result += " ";
+      result += layer_i->second;
+    }
+  }
+  return result;
+}
+
 class WorkspaceManagerTest : public test::AshTestBase {
  public:
   WorkspaceManagerTest() : manager_(NULL) {}
   virtual ~WorkspaceManagerTest() {}
 
   aura::Window* CreateTestWindowUnparented() {
     aura::Window* window = new aura::Window(NULL);
     window->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_NORMAL);
     window->SetType(aura::client::WINDOW_TYPE_NORMAL);
     window->Init(ui::LAYER_TEXTURED);
@@ skipping 1388 matching lines @@
 
   window1->layer()->GetAnimator()->StopAnimating();
   window2->layer()->GetAnimator()->StopAnimating();
   // |window1| should be flush right and |window2| flush left.
   EXPECT_EQ(base::IntToString(
                 desktop_area.width() - window1->bounds().width()) +
             ",32 640x320", window1->bounds().ToString());
   EXPECT_EQ("0,48 256x512", window2->bounds().ToString());
 }
 
+// This tests simulates a browser and an app and verifies the ordering of the
+// windows and layers doesn't get out of sync as various operations occur. Its
+// really testing code in FocusController, but easier to simulate here. Just as
+// with a real browser the browser here has a transient child window
+// (corresponds to the status bubble).
+TEST_F(WorkspaceManagerTest, VerifyLayerOrdering) {
+  aura::test::EventGenerator generator(
+      Shell::GetPrimaryRootWindow(), gfx::Point());
+  generator.MoveMouseTo(5, 5);

[stevenjb, 2013/03/22 18:28:15]

Could you document why the initial MoveMouseTo?

[sky, 2013/03/22 19:30:15]

That's not necessary and left over from another test. I'll nuke it.

+
+  scoped_ptr<Window> browser(
+      aura::test::CreateTestWindowWithDelegate(
+          NULL,
+          aura::client::WINDOW_TYPE_NORMAL,
+          gfx::Rect(5, 6, 7, 8),
+          NULL));
+  browser->SetName("browser");
+  SetDefaultParentByPrimaryRootWindow(browser.get());
+  browser->Show();
+  wm::ActivateWindow(browser.get());
+
+  // |status_bubble| is made a transient child of |browser| and as a result
+  // owned by |browser|.
+  aura::test::TestWindowDelegate* status_bubble_delegate =
+      aura::test::TestWindowDelegate::CreateSelfDestroyingDelegate();
+  status_bubble_delegate->set_can_focus(false);
+  Window* status_bubble =
+      aura::test::CreateTestWindowWithDelegate(
+          status_bubble_delegate,
+          aura::client::WINDOW_TYPE_POPUP,
+          gfx::Rect(5, 6, 7, 8),
+          NULL);
+  browser->AddTransientChild(status_bubble);
+  SetDefaultParentByPrimaryRootWindow(status_bubble);
+  status_bubble->SetName("status_bubble");
+
+  scoped_ptr<Window> app(
+      aura::test::CreateTestWindowWithDelegate(
+          NULL,
+          aura::client::WINDOW_TYPE_NORMAL,
+          gfx::Rect(5, 6, 7, 8),
+          NULL));
+  app->SetName("app");
+  SetDefaultParentByPrimaryRootWindow(app.get());
+
+  aura::Window* parent = browser->parent();
+
+  app->Show();
+  wm::ActivateWindow(app.get());
+  EXPECT_EQ(GetWindowNames(parent), GetLayerNames(parent));
+
+  // Minimize the app, focus should go the browser.
+  app->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_MINIMIZED);

[stevenjb, 2013/03/22 18:28:15]

Test that focus does go to the browser here? I know that's not what we are
testing, but it seems like it would help document the conditions for the test
setup better than just the comment.

[sky, 2013/03/22 19:30:15]

Sure.

+  EXPECT_EQ(GetWindowNames(parent), GetLayerNames(parent));
+
+  // Minimize the browser.
+  browser->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_MINIMIZED);

[stevenjb, 2013/03/22 18:28:15]

Mostly out of curiosity, where does focus go here? I assume neither window
should be focused now?

[sky, 2013/03/22 19:30:15]

I believe it goes no where, but I think ideally we want the launcher to get
focus in this case.

+  EXPECT_EQ(GetWindowNames(parent), GetLayerNames(parent));
+
+  // Show the browser (which should restore it).

[stevenjb, 2013/03/22 18:28:15]

But not focus it, correct?

[sky, 2013/03/22 19:30:15]

Not sure there. The focus rules may implicitly focus it. I'm not going to
assert, but will below.

+  browser->Show();
+  EXPECT_EQ(GetWindowNames(parent), GetLayerNames(parent));
+
+  // Activate the browser.
+  ash::wm::ActivateWindow(browser.get());

[stevenjb, 2013/03/22 18:28:15]

Until here, right?

+  EXPECT_EQ(GetWindowNames(parent), GetLayerNames(parent));
+
+  // Restore the app. This differs from above code for |browser| as internally
+  // the app code does this.
+  app->SetProperty(aura::client::kShowStateKey, ui::SHOW_STATE_NORMAL);

[stevenjb, 2013/03/22 18:28:15]

Should we change the App code to use Show() instead of Restore()? It seems like
they should be consistent. Making that change doesn't seem to affect the bug, so
it seems like it should be consistent in the test (or tested separately)?

[sky, 2013/03/22 19:30:15]

Yes, but it won't make a difference for this particular test. That said,
changing the app code to use show should fix 179387. I'll add a comment to that
bug.

+  EXPECT_EQ(GetWindowNames(parent), GetLayerNames(parent));
+
+  // Activate the app.
+  ash::wm::ActivateWindow(app.get());
+  EXPECT_EQ(GetWindowNames(parent), GetLayerNames(parent));
+}
+
 }  // namespace internal
 }  // namespace ash