Have TestFocusClient keep the Aura focus client updated.

This makes it harder to introduce bugs like "constructed the client
but forgot to set it on the window" or the more likely (and insidious)
"failed to null the client on shutdown, leaking state into another test
and triggering test flakiness".

Bug: none
Tbr: peter
Change-Id: Id9f77d46afe79030a0ffbf77f67e0716bd9e6e87
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2118764
Commit-Queue: Peter Kasting <pkasting@chromium.org>
Auto-Submit: Peter Kasting <pkasting@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753862}

content/shell/browser/shell_platform_data_aura.cc

@@ -108,8 +108,7 @@ ShellPlatformDataAura::ShellPlatformDataAura(const gfx::Size& initial_size) {
   host_->window()->Show();
   host_->window()->SetLayoutManager(new FillLayout(host_->window()));
 
-  focus_client_.reset(new aura::test::TestFocusClient());
-  aura::client::SetFocusClient(host_->window(), focus_client_.get());
+  focus_client_.reset(new aura::test::TestFocusClient(host_->window()));
 
   new wm::DefaultActivationClient(host_->window());
   capture_client_.reset(

ui/aura/demo/demo_main.cc

@@ -197,8 +197,7 @@ int DemoMain() {
       test_screen->CreateHostForPrimaryDisplay());
   std::unique_ptr<DemoWindowParentingClient> window_parenting_client(
       new DemoWindowParentingClient(host->window()));
-  aura::test::TestFocusClient focus_client;
-  aura::client::SetFocusClient(host->window(), &focus_client);
+  aura::test::TestFocusClient focus_client(host->window());
 
   // Create a hierarchy of test windows.
   gfx::Rect window1_bounds(100, 100, 400, 400);

ui/aura/test/aura_test_helper.cc

@@ -83,8 +83,6 @@ void AuraTestHelper::SetUp(ui::ContextFactory* context_factory) {
   setup_called_ = true;
 
   wm_state_ = std::make_unique<wm::WMState>();
-  // Needs to be before creating WindowTreeClient.
-  focus_client_ = std::make_unique<TestFocusClient>();
 
   if (!env) {
     // Some tests suites create Env globally rather than per test.
@@ -125,7 +123,7 @@ void AuraTestHelper::SetUp(ui::ContextFactory* context_factory) {
 
   Window* root_window = GetContext();
   new wm::DefaultActivationClient(root_window);  // Manages own lifetime.
-  client::SetFocusClient(root_window, focus_client_.get());
+  focus_client_ = std::make_unique<TestFocusClient>(root_window);
   capture_client_ = std::make_unique<client::DefaultCaptureClient>(root_window);
   parenting_client_ = std::make_unique<TestWindowParentingClient>(root_window);
 
@@ -140,16 +138,14 @@ void AuraTestHelper::TearDown() {
   g_instance = nullptr;
   teardown_called_ = true;
   parenting_client_.reset();
-  client::SetFocusClient(GetContext(), nullptr);
   capture_client_.reset();
+  focus_client_.reset();
   host_.reset();
 
   if (display::Screen::GetScreen() == test_screen_.get())
     display::Screen::SetScreenInstance(nullptr);
   test_screen_.reset();
 
-  focus_client_.reset();
-
   ui::ShutdownInputMethodForTesting();
 
   context_factories_.reset();

ui/aura/test/test_focus_client.cc

@@ -13,12 +13,16 @@ namespace test {
 ////////////////////////////////////////////////////////////////////////////////
 // TestFocusClient, public:
 
-TestFocusClient::TestFocusClient()
-    : focused_window_(NULL),
+TestFocusClient::TestFocusClient(Window* root_window)
+    : root_window_(root_window),
+      focused_window_(nullptr),
       observer_manager_(this) {
+  DCHECK(root_window_);
+  client::SetFocusClient(root_window_, this);
 }
 
 TestFocusClient::~TestFocusClient() {
+  client::SetFocusClient(root_window_, nullptr);
 }
 
 ////////////////////////////////////////////////////////////////////////////////

ui/aura/test/test_focus_client.h

@@ -18,7 +18,7 @@ namespace test {
 class TestFocusClient : public client::FocusClient,
                         public WindowObserver {
  public:
-  TestFocusClient();
+  explicit TestFocusClient(Window* root_window);
   ~TestFocusClient() override;
 
  private:
@@ -32,6 +32,7 @@ class TestFocusClient : public client::FocusClient,
   // Overridden from WindowObserver:
   void OnWindowDestroying(Window* window) override;
 
+  Window* root_window_;
   Window* focused_window_;
   ScopedObserver<Window, WindowObserver> observer_manager_;
   base::ObserverList<aura::client::FocusChangeObserver>::Unchecked

ui/wm/test/wm_test_helper.cc

@@ -32,8 +32,8 @@ WMTestHelper::WMTestHelper(const gfx::Size& default_window_size) {
 
   aura::client::SetWindowParentingClient(host_->window(), this);
 
-  focus_client_ = std::make_unique<aura::test::TestFocusClient>();
-  aura::client::SetFocusClient(host_->window(), focus_client_.get());
+  focus_client_ =
+      std::make_unique<aura::test::TestFocusClient>(host_->window());
 
   root_window_event_filter_ = std::make_unique<wm::CompoundEventFilter>();
   host_->window()->AddPreTargetHandler(root_window_event_filter_.get());