Kill bad apps.

chrome/browser/apps/guest_view/app_view_browsertest.cc

Index: chrome/browser/apps/guest_view/app_view_browsertest.cc
diff --git a/chrome/browser/apps/guest_view/app_view_browsertest.cc b/chrome/browser/apps/guest_view/app_view_browsertest.cc
index 0278507d3b520be64442f238bf2ee3cfe3dab465..9ef493b36800ce7af1aa93b4752b87fc6138c0b4 100644
--- a/chrome/browser/apps/guest_view/app_view_browsertest.cc
+++ b/chrome/browser/apps/guest_view/app_view_browsertest.cc
@@ -1,16 +1,21 @@
 // Copyright 2014 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.
-

[Devlin, 2015/06/30 19:54:10]

we want this newline. :)

[EhsanK, 2015/07/02 19:15:13]

Done. :)

 #include "base/strings/stringprintf.h"
 #include "chrome/browser/apps/app_browsertest_util.h"
 #include "components/guest_view/browser/guest_view_manager.h"
 #include "components/guest_view/browser/guest_view_manager_factory.h"
 #include "components/guest_view/browser/test_guest_view_manager.h"
+#include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_process_host.h"
+#include "content/public/browser/render_process_host_observer.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_utils.h"
+#include "extensions/browser/app_window/app_window_registry.h"
+#include "extensions/browser/guest_view/app_view/app_view_guest.h"
+#include "extensions/browser/guest_view/extensions_guest_view_manager_delegate.h"
+#include "extensions/browser/process_manager.h"
 #include "extensions/common/switches.h"
 #include "extensions/test/extension_test_message_listener.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
@@ -20,6 +25,52 @@
 using guest_view::GuestViewManager;
 using guest_view::TestGuestViewManagerFactory;
 
+namespace {
+
+class RenderProcessHostObserverForExit
+    : public content::RenderProcessHostObserver {
+ public:
+  explicit RenderProcessHostObserverForExit(
+      content::RenderProcessHost* observed_host)
+      : render_process_host_exited_(false),
+        observed_host_(observed_host),
+        message_loop_runner_(nullptr) {
+    observed_host->AddObserver(this);
+    LOG(INFO) << "Listening for RPH Exit (ID = " << observed_host->GetID()

[Devlin, 2015/06/30 19:54:10]

These logs need to be cleaned up before committing.

[EhsanK, 2015/07/02 19:15:13]

Done.

+              << ")";
+  }
+
+  void WaitUntilRenderProcessHostKilled() {
+    if (render_process_host_exited_)
+      return;
+    message_loop_runner_ = new content::MessageLoopRunner;
+    message_loop_runner_->Run();
+  }
+
+  base::TerminationStatus termination_status() { return status_; }

[Devlin, 2015/06/30 19:54:10]

termination_status() const

[EhsanK, 2015/07/02 19:15:13]

Done.

+
+ private:
+  void RenderProcessExited(content::RenderProcessHost* host,
+                           base::TerminationStatus status,
+                           int exit_code) override {
+    LOG(INFO) << "RPH is exiting. (ID = " << host->GetID() << ").";
+    DCHECK(observed_host_ == host);
+    render_process_host_exited_ = true;
+    status_ = status;
+    observed_host_->RemoveObserver(this);
+    if (message_loop_runner_.get()) {
+      message_loop_runner_->Quit();
+    }
+  }
+
+  bool render_process_host_exited_;
+  content::RenderProcessHost* observed_host_;
+  scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
+  base::TerminationStatus status_;
+};

[Devlin, 2015/06/30 19:54:10]

DISALLOW_COPY_AND_ASSIGN

[EhsanK, 2015/07/02 19:15:13]

Done.

+
+}  // namespace
+
 class AppViewTest : public extensions::PlatformAppBrowserTest {
  public:
   AppViewTest() {
@@ -59,8 +110,7 @@ class AppViewTest : public extensions::PlatformAppBrowserTest {
     done_listener.set_failure_message("TEST_FAILED");
     if (!content::ExecuteScript(
             embedder_web_contents,
-            base::StringPrintf("runTest('%s', '%s')",
-                               test_name.c_str(),
+            base::StringPrintf("runTest('%s', '%s')", test_name.c_str(),
                                app_to_embed.c_str()))) {
       LOG(ERROR) << "UNABLE TO START TEST.";
       return;
@@ -68,6 +118,22 @@ class AppViewTest : public extensions::PlatformAppBrowserTest {
     ASSERT_TRUE(done_listener.WaitUntilSatisfied());
   }
 
+  guest_view::TestGuestViewManager* GetGuestViewManager() {
+    guest_view::TestGuestViewManager* manager =

[Devlin, 2015/06/30 19:54:10]

Instead of checking every time and then creating, could we just create during
initialization?

[EhsanK, 2015/07/02 19:15:13]

Done. Had to do it on the "SetUpOnMainThread" since before that browser() is
nullptr.

+        static_cast<guest_view::TestGuestViewManager*>(
+            guest_view::TestGuestViewManager::FromBrowserContext(
+                browser()->profile()));
+    if (!manager) {
+      manager = static_cast<guest_view::TestGuestViewManager*>(
+          GuestViewManager::CreateWithDelegate(
+              browser()->profile(),
+              scoped_ptr<guest_view::GuestViewManagerDelegate>(
+                  new extensions::ExtensionsGuestViewManagerDelegate(
+                      browser()->profile()))));
+    }
+    return manager;
+  }
+
  private:
   void SetUpCommandLine(base::CommandLine* command_line) override {
     extensions::PlatformAppBrowserTest::SetUpCommandLine(command_line);
@@ -125,3 +191,76 @@ IN_PROC_BROWSER_TEST_F(AppViewTest, TestAppViewEmbedSelfShouldFail) {
              skeleton_app->id(),
              NO_TEST_SERVER);
 }
+
+IN_PROC_BROWSER_TEST_F(AppViewTest, KillGuestWithInvalidInstanceID) {
+  const extensions::Extension* mock_bad_app =
+      LoadAndLaunchPlatformApp("app_view/bad_app", "AppViewTest.LAUNCHED");
+
+  content::RenderProcessHost* bad_app_render_process_host =
+      extensions::AppWindowRegistry::Get(browser()->profile())
+          ->GetCurrentAppWindowForApp(mock_bad_app->id())
+          ->web_contents()
+          ->GetRenderProcessHost();
+
+  // Monitor |mock_bad_app|'s "RenderProcessHost" for its exiting.

[Devlin, 2015/06/30 19:54:10]

nit: We don't really put class names in quotes.

[EhsanK, 2015/07/02 19:15:13]

Done.

+  RenderProcessHostObserverForExit exit_observer(bad_app_render_process_host);
+
+  // Get/Create the instance of "GuestViewManager".

[Devlin, 2015/06/30 19:54:10]

Some of these comments are a little unnecessary.  When possible, have the code
be the documentation.  Since we have a line that says "GetGuestViewManager()",
we don't really need the comment "Get the guest view manager" :)

[EhsanK, 2015/07/02 19:15:13]

Done.

+  guest_view::GuestViewManager* guest_view_manager = GetGuestViewManager();
+  EXPECT_TRUE(guest_view_manager != nullptr);
+
+  // Choosing a |guest_instance_id| which does not exist.
+  int invalid_guest_instance_id = guest_view_manager->GetNextInstanceID();
+  LOG(INFO) << "Guest instance ID: " << invalid_guest_instance_id;
+
+  // Call the desired function to verify that the |mock_bad_app| gets killed if
+  // the provided |guest_instance_id| is not mapped to any "GuestView"'s.
+  extensions::AppViewGuest::CompletePendingRequest(
+      browser()->profile(), GURL("about:blank"), invalid_guest_instance_id,
+      mock_bad_app->id(), bad_app_render_process_host->GetID());
+  exit_observer.WaitUntilRenderProcessHostKilled();
+  EXPECT_EQ(exit_observer.termination_status(),
+            base::TERMINATION_STATUS_PROCESS_WAS_KILLED);
+}
+
+IN_PROC_BROWSER_TEST_F(AppViewTest, KillGuestCommunicatingWithWrongAppView) {
+  const extensions::Extension* host_app =
+      LoadAndLaunchPlatformApp("app_view/host_app", "AppViewTest.LAUNCHED");
+  const extensions::Extension* mock_guest_extension =
+      InstallPlatformApp("app_view/guest_app");
+  const extensions::Extension* mock_bad_app =
+      LoadAndLaunchPlatformApp("app_view/bad_app", "AppViewTest.LAUNCHED");
+  // Ask the host to create and load an <appview>
+  EXPECT_TRUE(content::ExecuteScript(
+      extensions::AppWindowRegistry::Get(browser()->profile())
+          ->GetCurrentAppWindowForApp(host_app->id())
+          ->web_contents(),
+      base::StringPrintf("onAppCommand('%s', '%s');", "EMBED",
+                         mock_guest_extension->id().c_str())));
+  // Now listen for the guest content to announce that it is being requested to
+  // be embedded.
+  ExtensionTestMessageListener on_embed_requested_listener(
+      "AppViewTest.EmbedRequested", true);
+  EXPECT_TRUE(on_embed_requested_listener.WaitUntilSatisfied());
+  // Now assume the bad application is somehow sending a message to complete a
+  // pending request to attach to <appview>. It should be killed.
+  content::RenderProcessHost* bad_app_render_process_host =
+      extensions::ProcessManager::Get(browser()->profile())
+          ->GetBackgroundHostForExtension(mock_bad_app->id())
+          ->render_process_host();
+  RenderProcessHostObserverForExit bad_app_obs(bad_app_render_process_host);
+  // Make the false request.
+  int guest_instance_id =
+      extensions::AppViewGuest::GetAllRegisteredInstanceIdsForTesting()[0];
+  extensions::AppViewGuest::CompletePendingRequest(
+      browser()->profile(), GURL("about:blank"), guest_instance_id,
+      mock_bad_app->id(), bad_app_render_process_host->GetID());
+
+  // Proceed with the rest of embedding of the guest app.
+  on_embed_requested_listener.Reply("continue");
+  // Make sure the bad application has been terminated.
+  bad_app_obs.WaitUntilRenderProcessHostKilled();
+
+  EXPECT_EQ(bad_app_obs.termination_status(),
+            base::TERMINATION_STATUS_PROCESS_WAS_KILLED);
+}