Rework the ApplicationManager interface apptest to better test ApplicationManagerListener

R=sky@chromium.org
BUG=none

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

Cr-Commit-Position: refs/heads/master@{#368112}

2 files changed, 79 insertions, 56 deletions

diff --git a/mojo/shell/application_manager_apptest.cc b/mojo/shell/application_manager_apptest.cc
index 363f34b..ca9c9e4 100644
--- a/mojo/shell/application_manager_apptest.cc
+++ b/mojo/shell/application_manager_apptest.cc
@@ -10,6 +10,7 @@
 #include "base/bind.h"
 #include "base/macros.h"
 #include "base/message_loop/message_loop.h"
+#include "base/process/process_handle.h"
 #include "mojo/application/public/cpp/application_impl.h"
 #include "mojo/application/public/cpp/application_test_base.h"
 #include "mojo/application/public/cpp/interface_factory.h"
@@ -63,9 +64,10 @@ class ApplicationManagerAppTestDelegate
 
 }  // namespace
 
-class ApplicationManagerAppTest : public mojo::test::ApplicationTestBase {
+class ApplicationManagerAppTest : public mojo::test::ApplicationTestBase,
+                                  public mojom::ApplicationManagerListener {
  public:
-  ApplicationManagerAppTest() : delegate_(nullptr) {}
+  ApplicationManagerAppTest() : delegate_(nullptr), binding_(this) {}
   ~ApplicationManagerAppTest() override {}
 
   void OnDriverQuit() {
@@ -73,11 +75,36 @@ class ApplicationManagerAppTest : public mojo::test::ApplicationTestBase {
   }
 
  protected:
+  struct ApplicationInfo {
+    ApplicationInfo(int id, const std::string& url)
+        : id(id), url(url), pid(base::kNullProcessId) {}
+
+    int id;
+    std::string url;
+    base::ProcessId pid;
+  };
+
+  void AddListenerAndWaitForApplications() {
+    mojom::ApplicationManagerPtr application_manager;
+    application_impl()->ConnectToService("mojo:shell", &application_manager);
+
+    mojom::ApplicationManagerListenerPtr listener;
+    InterfaceRequest<mojom::ApplicationManagerListener> request =
+        GetProxy(&listener);
+    application_manager->AddListener(std::move(listener));
+    binding_.Bind(std::move(request));
+    binding_.WaitForIncomingMethodCall();
+  }
+
   const std::string& data() const {
     DCHECK(delegate_);
     return delegate_->data();
   }
 
+  const std::vector<ApplicationInfo>& applications() const {
+    return applications_;
+  }
+
   ApplicationManagerAppTestDelegate* delegate() { return delegate_; }
 
  private:
@@ -87,23 +114,68 @@ class ApplicationManagerAppTest : public mojo::test::ApplicationTestBase {
     return delegate_;
   }
 
+  // mojom::ApplicationManagerListener:
+  void SetRunningApplications(
+      Array<mojom::ApplicationInfoPtr> applications) override {}
+  void ApplicationInstanceCreated(
+      mojom::ApplicationInfoPtr application) override {
+    applications_.push_back(ApplicationInfo(application->id, application->url));
+  }
+  void ApplicationInstanceDestroyed(int id) override {
+    for (auto it = applications_.begin(); it != applications_.end(); ++it) {
+      auto& application = *it;
+      if (application.id == id) {
+        applications_.erase(it);
+        break;
+      }
+    }
+  }
+  void ApplicationPIDAvailable(int id, uint32_t pid) override {
+    for (auto& application : applications_) {
+      if (application.id == id) {
+        application.pid = pid;
+        break;
+      }
+    }
+  }
+
   ApplicationManagerAppTestDelegate* delegate_;
+  Binding<mojom::ApplicationManagerListener> binding_;
+  std::vector<ApplicationInfo> applications_;
 
   DISALLOW_COPY_AND_ASSIGN(ApplicationManagerAppTest);
 };
 
 TEST_F(ApplicationManagerAppTest, CreateInstanceForHandle) {
+  AddListenerAndWaitForApplications();
+
   // 1. Launch a process. (Actually, have the runner launch a process that
   //    launches a process. #becauselinkerrors).
   mojo::shell::test::mojom::DriverPtr driver;
   application_impl()->ConnectToService("exe:application_manager_apptest_driver",
                                        &driver);
 
-  // 2. Wait for it to connect to us. (via mojo:application_manager_apptests)
+  // 2. Wait for the target to connect to us. (via
+  //    mojo:application_manager_apptests)
   base::MessageLoop::current()->Run();
 
-  // 3. Profit!
-  EXPECT_EQ(data(), "From Target");
+  // 3.1. Validate that we got the ping from the target process...
+  EXPECT_EQ("From Target", data());
+
+  // 3.2. ... and that the right applications/processes were created.
+  //      Note that the target process will be created even if the tests are
+  //      run with --single-process.
+  EXPECT_EQ(2u, applications().size());
+  {
+    auto& application = applications().front();
+    EXPECT_EQ("exe://application_manager_apptest_driver/", application.url);
+    EXPECT_NE(base::kNullProcessId, application.pid);
+  }
+  {
+    auto& application = applications().back();
+    EXPECT_EQ("exe://application_manager_apptest_target/", application.url);
+    EXPECT_NE(base::kNullProcessId, application.pid);
+  }
 
   driver.set_connection_error_handler(
       base::Bind(&ApplicationManagerAppTest::OnDriverQuit,
@@ -112,56 +184,5 @@ TEST_F(ApplicationManagerAppTest, CreateInstanceForHandle) {
   base::MessageLoop::current()->Run();
 }
 
-class GetRunningApplicationInfoAppTest
-    : public ApplicationManagerAppTest,
-      public mojom::ApplicationManagerListener {
- public:
-  GetRunningApplicationInfoAppTest() : binding_(this) {}
-  ~GetRunningApplicationInfoAppTest() override {}
-
- protected:
-  void QueryApplications() {
-    mojom::ApplicationManagerPtr application_manager;
-    application_impl()->ConnectToService("mojo:shell", &application_manager);
-
-    mojom::ApplicationManagerListenerPtr listener;
-    InterfaceRequest<mojom::ApplicationManagerListener> request =
-        GetProxy(&listener);
-    application_manager->AddListener(std::move(listener));
-    binding_.Bind(std::move(request));
-    binding_.WaitForIncomingMethodCall();
-  }
-
-  bool Contains(const std::string& name) const {
-    return names_.find(name) != names_.end();
-  }
-
- private:
-  // Overridden from mojom::ApplicationManagerListener:
-  void SetRunningApplications(
-      Array<mojom::ApplicationInfoPtr> applications) override {
-    for (size_t i = 0; i < applications.size(); ++i)
-      names_.insert(applications[i]->url);
-  }
-  void ApplicationInstanceCreated(
-      mojom::ApplicationInfoPtr application) override {
-    names_.insert(application->url);
-  }
-  void ApplicationInstanceDestroyed(int id) override {}
-  void ApplicationPIDAvailable(int id, uint32_t pid) override {}
-
-  std::set<std::string> names_;
-  Binding<mojom::ApplicationManagerListener> binding_;
-
-  DISALLOW_COPY_AND_ASSIGN(GetRunningApplicationInfoAppTest);
-};
-
-TEST_F(GetRunningApplicationInfoAppTest, GetRunningApplicationInfo) {
-  QueryApplications();
-  EXPECT_TRUE(Contains("mojo://mojo_shell_apptests/"));
-  EXPECT_TRUE(Contains("mojo://shell/"));
-  EXPECT_TRUE(Contains("mojo://tracing/"));
-}
-
 }  // namespace shell
 }  // namespace mojo
diff --git a/mojo/shell/application_manager_apptest_driver.cc b/mojo/shell/application_manager_apptest_driver.cc
index 8eb919b..e92aa7d 100644
--- a/mojo/shell/application_manager_apptest_driver.cc
+++ b/mojo/shell/application_manager_apptest_driver.cc
@@ -123,6 +123,8 @@ class TargetApplicationDelegate : public mojo::ApplicationDelegate,
     options.fds_to_remap = &handle_passing_info;
   #endif
     target_ = base::LaunchProcess(child_command_line, options);
+    DCHECK(target_.IsValid());
+    receiver->SetPID(target_.Pid());
 
     if (base::CommandLine::ForCurrentProcess()->HasSwitch("use-new-edk")) {
       MojoHandle platform_handle_wrapper;