Remove ArcSessionManager::Observer::OnArcBridgeShutdown.

chrome/browser/chromeos/arc/arc_session_manager_browsertest.cc

 // Copyright 2016 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 <memory>
 #include <string>
 
+#include "base/auto_reset.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/run_loop.h"
 #include "base/time/time.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chromeos/arc/arc_service_launcher.h"
 #include "chrome/browser/chromeos/arc/arc_session_manager.h"
@@ skipping 39 matching lines @@
 // Set unmanaged auth token for other Android unmanaged accounts.
 constexpr char kUnmanagedAuthToken[] = "unmanaged-auth-token";
 constexpr char kWellKnownConsumerName[] = "test@gmail.com";
 constexpr char kFakeUserName[] = "test@example.com";
 constexpr char kFakeGaiaId[] = "1234567890";
 
 }  // namespace
 
 namespace arc {
 
-// Observer of ARC bridge shutdown.
-class ArcSessionManagerShutdownObserver : public ArcSessionManager::Observer {
+// Waits for the "arc.enabled" preference change.
+class ArcPlayStoreEnabledChangedWaiter : public ArcSessionManager::Observer {
  public:
-  ArcSessionManagerShutdownObserver() {
+  ArcPlayStoreEnabledChangedWaiter() {
     ArcSessionManager::Get()->AddObserver(this);
   }
 
-  ~ArcSessionManagerShutdownObserver() override {
+  ~ArcPlayStoreEnabledChangedWaiter() override {
     ArcSessionManager::Get()->RemoveObserver(this);
   }
 
   void Wait() {
-    run_loop_.reset(new base::RunLoop);
-    run_loop_->Run();
-    run_loop_.reset();
-  }
-
-  // ArcSessionManager::Observer:
-  void OnArcBridgeShutdown() override {
-    if (!run_loop_)
-      return;
-    run_loop_->Quit();
+    base::RunLoop run_loop;
+    base::AutoReset<base::RunLoop*> reset(&run_loop_, &run_loop);

[Luis Héctor Chávez, 2017/02/15 18:44:50]

nice, this looks _much_ cleaner :)

[hidehiko, 2017/02/15 18:51:53]

Thanks :-).

+    run_loop.Run();
   }
 
  private:
-  std::unique_ptr<base::RunLoop> run_loop_;
+  // ArcSessionManager::Observer override:
+  void OnArcPlayStoreEnabledChanged(bool enabled) override {
+    if (run_loop_)

[Luis Héctor Chávez, 2017/02/15 18:44:50]

nit:

if (!enabled) {
  DCHECK(run_loop_);
  run_loop_->Quit();
}

[hidehiko, 2017/02/15 18:51:53]

It changes the semantics (from the brief document of this class), I think.
Do you mean we should create ArcPlayStoreDisabledWaiter?

[Luis Héctor Chávez, 2017/02/15 19:09:44]

So previously the semantics for this class was to wait until ARC session was
shut down. Now this listens for the play store state to change in any direction
(since you're not inspecting the value of |enabled|), which is not compatible
with what was there previously. At the very least this should limit to waiting
until the state goes from enabled to disabled. 

Naming the class ArcPlayStoreDisabledWaiter seems like a good direction, and
maybe also changing the class comment to "Waits for the "arc.enabled" preference
to change from enabled to disabled".

[hidehiko, 2017/02/15 19:52:43]

Ok, thank you for detailed explanation.
Done.

+      run_loop_->Quit();
+  }
 
-  DISALLOW_COPY_AND_ASSIGN(ArcSessionManagerShutdownObserver);
+  base::RunLoop* run_loop_ = nullptr;
+
+  DISALLOW_COPY_AND_ASSIGN(ArcPlayStoreEnabledChangedWaiter);
 };
 
 class ArcSessionManagerTest : public InProcessBrowserTest {
  protected:
   ArcSessionManagerTest() {}
 
   // InProcessBrowserTest:
   ~ArcSessionManagerTest() override {}
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
@@ skipping 115 matching lines @@
 
   EnableArc();
   ASSERT_EQ(ArcSessionManager::State::ACTIVE,
             ArcSessionManager::Get()->state());
 }
 
 IN_PROC_BROWSER_TEST_F(ArcSessionManagerTest, ManagedAndroidAccount) {
   EnableArc();
   token_service()->IssueTokenForAllPendingRequests(kManagedAuthToken,
                                                    base::Time::Max());
-  ArcSessionManagerShutdownObserver().Wait();
+  ArcPlayStoreEnabledChangedWaiter().Wait();
   ASSERT_EQ(ArcSessionManager::State::REMOVING_DATA_DIR,
             ArcSessionManager::Get()->state());
   ArcDataRemovedWaiter().Wait();
   ASSERT_EQ(ArcSessionManager::State::STOPPED,
             ArcSessionManager::Get()->state());
 }
 
 }  // namespace arc