Implement <appview> based on out-of-process iframes GuestView.

chrome/browser/apps/guest_view/app_view_browsertest.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.

[lazyboy, 2016/04/12 18:09:34]

What about app_view_apitest.cc, have you tried running them with OOPIF?

[lfg, 2016/04/12 20:53:38]

They all pass, but since they run on the unsupported app_shell test framework,
do you think it's worth maintaining it?

[lfg, 2016/04/12 21:45:43]

I've enabled the tests.

 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <utility>
 
 #include "base/macros.h"
 #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_service.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_process_host_observer.h"
+#include "content/public/common/content_switches.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_utils.h"
 #include "extensions/browser/api/app_runtime/app_runtime_api.h"
 #include "extensions/browser/api/extensions_api_client.h"
 #include "extensions/browser/app_window/app_window_registry.h"
 #include "extensions/browser/guest_view/app_view/app_view_constants.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"
 #include "net/test/embedded_test_server/http_request.h"
 #include "net/test/embedded_test_server/http_response.h"
+#include "testing/gtest/include/gtest/gtest.h"
 
 using extensions::ExtensionsAPIClient;
 using guest_view::GuestViewManager;
 using guest_view::TestGuestViewManagerFactory;
 
 namespace {
 
 class RenderProcessHostObserverForExit
     : public content::RenderProcessHostObserver {
  public:
@@ skipping 28 matching lines @@
   bool render_process_host_exited_;
   content::RenderProcessHost* observed_host_;
   scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
   base::TerminationStatus status_;
 
   DISALLOW_COPY_AND_ASSIGN(RenderProcessHostObserverForExit);
 };
 
 }  // namespace
 
-class AppViewTest : public extensions::PlatformAppBrowserTest {
+class AppViewTest : public extensions::PlatformAppBrowserTest,
+                    public testing::WithParamInterface<bool> {
  public:
   AppViewTest() {
     GuestViewManager::set_factory_for_testing(&factory_);
   }
 
   enum TestServer {
     NEEDS_TEST_SERVER,
     NO_TEST_SERVER
   };
 
@@ skipping 33 matching lines @@
     ASSERT_TRUE(done_listener.WaitUntilSatisfied());
   }
 
   guest_view::TestGuestViewManager* test_guest_view_manager() const {
     return test_guest_view_manager_;
   }
 
  private:
   void SetUpCommandLine(base::CommandLine* command_line) override {
     extensions::PlatformAppBrowserTest::SetUpCommandLine(command_line);
+
+    bool use_cross_process_frames_for_guests = GetParam();
+    if (use_cross_process_frames_for_guests)
+      command_line->AppendSwitch(switches::kUseCrossProcessFramesForGuests);
   }
 
   void SetUpOnMainThread() override {
     extensions::PlatformAppBrowserTest::SetUpOnMainThread();
     test_guest_view_manager_ = static_cast<guest_view::TestGuestViewManager*>(
         guest_view::GuestViewManager::CreateWithDelegate(
             browser()->profile(),
             scoped_ptr<guest_view::GuestViewManagerDelegate>(
                 ExtensionsAPIClient::Get()->CreateGuestViewManagerDelegate(
                     browser()->profile()))));
   }
 
   TestGuestViewManagerFactory factory_;
   guest_view::TestGuestViewManager* test_guest_view_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(AppViewTest);
 };
 
+INSTANTIATE_TEST_CASE_P(AppViewTests, AppViewTest, testing::Bool());
+
 // Tests that <appview> is able to navigate to another installed app.
-IN_PROC_BROWSER_TEST_F(AppViewTest, TestAppViewWithUndefinedDataShouldSucceed) {
+IN_PROC_BROWSER_TEST_P(AppViewTest, TestAppViewWithUndefinedDataShouldSucceed) {
   const extensions::Extension* skeleton_app =
       InstallPlatformApp("app_view/shim/skeleton");
   TestHelper("testAppViewWithUndefinedDataShouldSucceed",
              "app_view/shim",
              skeleton_app->id(),
              NO_TEST_SERVER);
 }
 
 // Tests that <appview> correctly processes parameters passed on connect.
-IN_PROC_BROWSER_TEST_F(AppViewTest, TestAppViewRefusedDataShouldFail) {
+IN_PROC_BROWSER_TEST_P(AppViewTest, TestAppViewRefusedDataShouldFail) {

[Devlin, 2016/04/12 18:09:52]

Out of curiosity, is this significantly better than just running the tests on
the SiteIsolation trybot?  Is it worth making *all* the trybots run 7 extra
browser tests?

[lfg, 2016/04/12 20:53:38]

Yes, the SiteIsolation trybot tests with --site-per-process enabled, which is
different from --use-cross-process-frames-for-guests. This will test
--use-cross-process-frames-for-guests without --site-per-process, as well as
with --site-per-process (in the SiteIsolation trybots), so we get coverage for
all 4 possible modes.

(We did reach out to the infra team, and this was their recommendation over
adding another trybot).

   const extensions::Extension* skeleton_app =
       InstallPlatformApp("app_view/shim/skeleton");
   TestHelper("testAppViewRefusedDataShouldFail",
              "app_view/shim",
              skeleton_app->id(),
              NO_TEST_SERVER);
 }
 
 // Tests that <appview> correctly processes parameters passed on connect.
-IN_PROC_BROWSER_TEST_F(AppViewTest, TestAppViewGoodDataShouldSucceed) {
+IN_PROC_BROWSER_TEST_P(AppViewTest, TestAppViewGoodDataShouldSucceed) {
   const extensions::Extension* skeleton_app =
       InstallPlatformApp("app_view/shim/skeleton");
   TestHelper("testAppViewGoodDataShouldSucceed",
              "app_view/shim",
              skeleton_app->id(),
              NO_TEST_SERVER);
 }
 
 // Tests that <appview> correctly handles multiple successive connects.
-IN_PROC_BROWSER_TEST_F(AppViewTest, TestAppViewMultipleConnects) {
+IN_PROC_BROWSER_TEST_P(AppViewTest, TestAppViewMultipleConnects) {
   const extensions::Extension* skeleton_app =
       InstallPlatformApp("app_view/shim/skeleton");
   TestHelper("testAppViewMultipleConnects",
              "app_view/shim",
              skeleton_app->id(),
              NO_TEST_SERVER);
 }
 
 // Tests that <appview> does not embed self (the app which owns appview).
-IN_PROC_BROWSER_TEST_F(AppViewTest, TestAppViewEmbedSelfShouldFail) {
+IN_PROC_BROWSER_TEST_P(AppViewTest, TestAppViewEmbedSelfShouldFail) {
   const extensions::Extension* skeleton_app =
       InstallPlatformApp("app_view/shim/skeleton");
   TestHelper("testAppViewEmbedSelfShouldFail",
              "app_view/shim",
              skeleton_app->id(),
              NO_TEST_SERVER);
 }
 
-IN_PROC_BROWSER_TEST_F(AppViewTest, KillGuestWithInvalidInstanceID) {
+IN_PROC_BROWSER_TEST_P(AppViewTest, KillGuestWithInvalidInstanceID) {
   const extensions::Extension* bad_app =
       LoadAndLaunchPlatformApp("app_view/bad_app", "AppViewTest.LAUNCHED");
 
   content::RenderProcessHost* bad_app_render_process_host =
       extensions::AppWindowRegistry::Get(browser()->profile())
           ->GetCurrentAppWindowForApp(bad_app->id())
           ->web_contents()
           ->GetRenderProcessHost();
 
   // Monitor |bad_app|'s RenderProcessHost for its exiting.
   RenderProcessHostObserverForExit exit_observer(bad_app_render_process_host);
 
   // Choosing a |guest_instance_id| which does not exist.
   int invalid_guest_instance_id =
       test_guest_view_manager()->GetNextInstanceID();
   // Call the desired function to verify that the |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,
       bad_app->id(), bad_app_render_process_host);
   exit_observer.WaitUntilRenderProcessHostKilled();
 }
 
-IN_PROC_BROWSER_TEST_F(AppViewTest, KillGuestCommunicatingWithWrongAppView) {
+IN_PROC_BROWSER_TEST_P(AppViewTest, KillGuestCommunicatingWithWrongAppView) {
   const extensions::Extension* host_app =
       LoadAndLaunchPlatformApp("app_view/host_app", "AppViewTest.LAUNCHED");
   const extensions::Extension* guest_app =
       InstallPlatformApp("app_view/guest_app");
   const extensions::Extension* bad_app =
       LoadAndLaunchPlatformApp("app_view/bad_app", "AppViewTest.LAUNCHED");
   // The host app attemps to embed the guest
   EXPECT_TRUE(content::ExecuteScript(
       extensions::AppWindowRegistry::Get(browser()->profile())
           ->GetCurrentAppWindowForApp(host_app->id())
@@ skipping 19 matching lines @@
   extensions::AppRuntimeEventRouter::DispatchOnEmbedRequestedEvent(
       browser()->profile(), std::move(fake_embed_request_param), bad_app);
   bad_app_obs.WaitUntilRenderProcessHostKilled();
   // Now ask the guest to continue embedding.
   ASSERT_TRUE(
       ExecuteScript(extensions::ProcessManager::Get(browser()->profile())
                         ->GetBackgroundHostForExtension(guest_app->id())
                         ->web_contents(),
                     "continueEmbedding();"));
 }