Extending "ChromeSitePerProcessPDFTest.EmbeddedPDFInsideCrossOriginFrame" to verify guest destructi…

chrome/browser/chrome_site_per_process_browsertest.cc

 // 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.
 
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/path_service.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/chrome_notification_types.h"
@@ skipping 302 matching lines @@
     return test_guest_view_manager_;
   }
 
  private:
   guest_view::TestGuestViewManagerFactory factory_;
   guest_view::TestGuestViewManager* test_guest_view_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeSitePerProcessPDFTest);
 };
 
+// Helper class to wait until a given WebContents is destroyed.
+class WebContentsDestructionObserver : content::WebContentsObserver {

[alexmos, 2016/11/23 00:23:51]

There's a WebContentsDestroyedWatcher in content/public/test/test_utils.h.  Is
it possible to use that one instead of a new class?

[EhsanK, 2016/11/23 00:51:39]

Yes indeed. Thanks for the suggestion.

+ public:
+  explicit WebContentsDestructionObserver(content::WebContents* web_contents)
+      : content::WebContentsObserver(web_contents),
+        web_contents_destroyed_(false) {}
+  ~WebContentsDestructionObserver() override {}
+
+  void Wait() {
+    if (web_contents_destroyed_)
+      return;
+    message_loop_runner_ = new content::MessageLoopRunner();
+    message_loop_runner_->Run();
+  }
+
+ private:
+  void WebContentsDestroyed() override {
+    web_contents_destroyed_ = true;
+    if (message_loop_runner_)
+      message_loop_runner_->Quit();
+  }
+
+  bool web_contents_destroyed_;
+  scoped_refptr<content::MessageLoopRunner> message_loop_runner_;
+
+  DISALLOW_COPY_AND_ASSIGN(WebContentsDestructionObserver);
+};
+
 // TODO(ekaramad): This test is flaky on Windows 7. Enable it when the issue is
 // fixed ((https://crbug.com/666379).
 #if defined(OS_WIN)
 #define MAYBE_EmbeddedPDFInsideCrossOriginFrame \
   DISABLED_EmbeddedPDFInsideCrossOriginFrame
 #else
 #define MAYBE_EmbeddedPDFInsideCrossOriginFrame \
   EmbeddedPDFInsideCrossOriginFrame
 #endif
 // This test verifies that when navigating an OOPIF to a page with <embed>-ed
-// PDF, the guest is properly created (https://crbug.com/649856).
+// PDF, the guest is properly created, and by removing the embedder frame, the
+// guest is properly destroyed (https://crbug.com/649856).
 IN_PROC_BROWSER_TEST_F(ChromeSitePerProcessPDFTest,
                        MAYBE_EmbeddedPDFInsideCrossOriginFrame) {
   // Navigate to a page with an <iframe>.
   GURL main_url(embedded_test_server()->GetURL("a.com", "/iframe.html"));
   ui_test_utils::NavigateToURL(browser(), main_url);
 
   // Initially, no guests are created.
   EXPECT_EQ(0U, test_guest_view_manager()->num_guests_created());
 
   // Navigate subframe to a cross-site page with an embedded PDF.
   content::WebContents* active_web_contents =
       browser()->tab_strip_model()->GetActiveWebContents();
   GURL frame_url =
       embedded_test_server()->GetURL("b.com", "/page_with_embedded_pdf.html");
 
   // Ensure the page finishes loading without crashing.
   EXPECT_TRUE(NavigateIframeToURL(active_web_contents, "test", frame_url));
 
   // Wait until the guest for PDF is created.
   test_guest_view_manager()->WaitForSingleGuestCreated();

[alexmos, 2016/11/23 00:23:51]

Can you save the result of this and use it below, instead of calling
GetLastGuestCreated again?

[EhsanK, 2016/11/23 00:51:39]

Acknowledged.

+
+  // Now detach the frame and observe that the guest is destroyed.
+  WebContentsDestructionObserver observer(
+      test_guest_view_manager()->GetLastGuestCreated());
+  EXPECT_TRUE(ExecuteScript(
+      active_web_contents,
+      "document.body.removeChild(document.querySelector('iframe'));"));
+  observer.Wait();

[alexmos, 2016/11/23 00:23:51]

Would it be useful to also check that GetNumGuestsActive() is 0 after this
point?

[EhsanK, 2016/11/23 00:51:39]

Acknowledged.

 }