Fix more flaky OOPIF tests by waiting for the old RenderFrame to be deleted.

content/browser/frame_host/navigation_controller_impl_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 "content/browser/frame_host/navigation_controller_impl.h"
 
 #include <stdint.h>
 #include <algorithm>
 #include <utility>
 #include <vector>
@@ skipping 2221 matching lines @@
         "        +--Site A -- proxies for B\n"
         "Where A = http://127.0.0.1/\n"
         "      B = http://foo.com/",
         visualizer.DepictFrameTree(root));
   }
 }
 
 // Verify the tree of FrameNavigationEntries after NAVIGATION_TYPE_NEW_SUBFRAME
 // commits.
 // Disabled due to flakes; see https://crbug.com/646836.
-#if defined(OS_ANDROID) || defined(OS_LINUX)
-#define MAYBE_FrameNavigationEntry_NewSubframe \
-    DISABLED_FrameNavigationEntry_NewSubframe
-#else
-#define MAYBE_FrameNavigationEntry_NewSubframe FrameNavigationEntry_NewSubframe
-#endif
 IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
-                       MAYBE_FrameNavigationEntry_NewSubframe) {
+                       FrameNavigationEntry_NewSubframe) {
   GURL main_url(embedded_test_server()->GetURL(
       "/navigation_controller/simple_page_1.html"));
   EXPECT_TRUE(NavigateToURL(shell(), main_url));
   const NavigationControllerImpl& controller =
       static_cast<const NavigationControllerImpl&>(
           shell()->web_contents()->GetController());
   FrameTreeNode* root =
       static_cast<WebContentsImpl*>(shell()->web_contents())->
           GetFrameTree()->root();
 
   // 1. Create a same-site iframe.
   GURL frame_url(embedded_test_server()->GetURL(
       "/navigation_controller/simple_page_2.html"));
   {
     LoadCommittedCapturer capturer(shell()->web_contents());
     std::string script = "var iframe = document.createElement('iframe');"
                          "iframe.src = '" + frame_url.spec() + "';"
                          "document.body.appendChild(iframe);";
     EXPECT_TRUE(ExecuteScript(root, script));
     capturer.Wait();
   }
   NavigationEntryImpl* entry = controller.GetLastCommittedEntry();
 
   // 2. Navigate in the subframe same-site.
   GURL frame_url2(embedded_test_server()->GetURL(
       "/navigation_controller/page_with_links.html"));
   {
     FrameNavigateParamsCapturer capturer(root->child_at(0));
     NavigateFrameToURL(root->child_at(0), frame_url2);

[Łukasz Anforowicz, 2016/11/07 22:13:25]

No need for RenderFrameDeletedObserver here?  Is this because this is a
same-site navigation?

[Charlie Reis, 2016/11/07 22:54:51]

Correct.  The RenderFrame won't be deleted on same-site navigations, so it would
just wait forever and time out.

     capturer.Wait();
     EXPECT_TRUE(ui::PageTransitionTypeIncludingQualifiersIs(
         capturer.params().transition, ui::PAGE_TRANSITION_MANUAL_SUBFRAME));
     EXPECT_EQ(NAVIGATION_TYPE_NEW_SUBFRAME, capturer.details().type);
   }
 
   // We should have created a new NavigationEntry with the same main frame URL.
   EXPECT_EQ(2, controller.GetEntryCount());
   EXPECT_EQ(1, controller.GetLastCommittedEntryIndex());
   NavigationEntryImpl* entry2 = controller.GetLastCommittedEntry();
@@ skipping 18 matching lines @@
   {
     LoadCommittedCapturer capturer(shell()->web_contents());
     std::string script = "var iframe = document.createElement('iframe');"
                          "iframe.src = '" + foo_url.spec() + "';"
                          "document.body.appendChild(iframe);";
     EXPECT_TRUE(ExecuteScript(root, script));
     capturer.Wait();
   }
 
   // 4. Create a nested same-site iframe in the second subframe, wait for it to
-  // commit, then navigate it again.
+  // commit, then navigate it again cross-site.
   {
     LoadCommittedCapturer capturer(shell()->web_contents());
     std::string script = "var iframe = document.createElement('iframe');"
                          "iframe.src = '" + foo_url.spec() + "';"
                          "document.body.appendChild(iframe);";
     EXPECT_TRUE(ExecuteScript(root->child_at(1), script));
     capturer.Wait();
   }
   GURL bar_url(embedded_test_server()->GetURL(
       "bar.com", "/navigation_controller/simple_page_1.html"));
   {
     FrameNavigateParamsCapturer capturer(root->child_at(1)->child_at(0));
+    RenderFrameDeletedObserver deleted_observer(
+        root->child_at(1)->child_at(0)->current_frame_host());
     NavigateFrameToURL(root->child_at(1)->child_at(0), bar_url);
+    // Wait for the RenderFrame to go away, if this will be cross-process.
+    if (AreAllSitesIsolatedForTesting())
+      deleted_observer.WaitUntilDeleted();
     capturer.Wait();
     EXPECT_TRUE(ui::PageTransitionTypeIncludingQualifiersIs(
         capturer.params().transition, ui::PAGE_TRANSITION_MANUAL_SUBFRAME));
     EXPECT_EQ(NAVIGATION_TYPE_NEW_SUBFRAME, capturer.details().type);
   }
 
   // We should have created a new NavigationEntry with the same main frame URL.
   EXPECT_EQ(3, controller.GetEntryCount());
   EXPECT_EQ(2, controller.GetLastCommittedEntryIndex());
   NavigationEntryImpl* entry3 = controller.GetLastCommittedEntry();
@@ skipping 15 matching lines @@
   } else {
     // There are no subframe FrameNavigationEntries by default.
     EXPECT_EQ(0U, entry3->root_node()->children.size());
   }
 
   // 6. Navigate the second subframe cross-site, clearing its existing subtree.
   GURL baz_url(embedded_test_server()->GetURL(
       "baz.com", "/navigation_controller/simple_page_1.html"));
   {
     FrameNavigateParamsCapturer capturer(root->child_at(1));
+    RenderFrameDeletedObserver deleted_observer(
+        root->child_at(1)->current_frame_host());
     std::string script = "var frames = document.getElementsByTagName('iframe');"
                          "frames[1].src = '" + baz_url.spec() + "';";
     EXPECT_TRUE(ExecuteScript(root, script));
+    // Wait for the RenderFrame to go away, if this will be cross-process.
+    if (AreAllSitesIsolatedForTesting())
+      deleted_observer.WaitUntilDeleted();
     capturer.Wait();
     EXPECT_TRUE(ui::PageTransitionTypeIncludingQualifiersIs(
         capturer.params().transition, ui::PAGE_TRANSITION_MANUAL_SUBFRAME));
     EXPECT_EQ(NAVIGATION_TYPE_NEW_SUBFRAME, capturer.details().type);
   }
 
   // We should have created a new NavigationEntry with the same main frame URL.
   EXPECT_EQ(4, controller.GetEntryCount());
   EXPECT_EQ(3, controller.GetLastCommittedEntryIndex());
   NavigationEntryImpl* entry4 = controller.GetLastCommittedEntry();
@@ skipping 4524 matching lines @@
   // Verify that the extra header was NOT present for the subresource.
   const net::test_server::HttpRequest* image_request =
       FindAccumulatedRequest(image_url);
   ASSERT_TRUE(image_request);
   EXPECT_THAT(image_request->headers,
               testing::Not(testing::Contains(
                   testing::Key("X-ExtraHeadersVsSubresources"))));
 }
 
 }  // namespace content