Fix going back to a script-injected about:blank frame.

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 <utility>
 
 #include "base/bind.h"
@@ skipping 3187 matching lines @@
   EXPECT_EQ(1, controller.GetLastCommittedEntryIndex());
 
   // 3. Go back, recreating the iframe.
   {
     TestNavigationObserver back_load_observer(shell()->web_contents());
     controller.GoBack();
     back_load_observer.Wait();
   }
   ASSERT_EQ(1U, root->child_count());
   EXPECT_EQ(main_url, root->current_url());
-
-  // TODO(creis): The child's current_url should be about:blank, but we're not
-  // currently getting a commit in this case.  For now, we'll lack a commit for
-  // this frame, similar to the slow URL case.  See https://crbug.com/626416.
-  if (SiteIsolationPolicy::UseSubframeNavigationEntries())
-    EXPECT_TRUE(root->child_at(0)->current_url().is_empty());
-  else
-    EXPECT_EQ(blank_url, root->child_at(0)->current_url());
+  EXPECT_EQ(blank_url, root->child_at(0)->current_url());
 
   // Verify that the parent was able to script the iframe.
   {
     std::string value;
     EXPECT_TRUE(ExecuteScriptAndExtractString(
         root->child_at(0),
         "domAutomationController.send(document.body.innerHTML)", &value));
     EXPECT_EQ(expected_text, value);
   }
 
   EXPECT_EQ(2, controller.GetEntryCount());
   EXPECT_EQ(0, controller.GetLastCommittedEntryIndex());
   EXPECT_EQ(entry, controller.GetLastCommittedEntry());
 
   // The entry should have a FrameNavigationEntry for the blank subframe.
   if (SiteIsolationPolicy::UseSubframeNavigationEntries()) {
     ASSERT_EQ(1U, entry->root_node()->children.size());
     EXPECT_EQ(blank_url, entry->root_node()->children[0]->frame_entry->url());
   }
 }
 
+// Verify that we correctly load nested iframes injected into a page if we go
+// back and recreate them.  Also confirm that form values are not restored for
+// forms injected into about:blank pages.  See https://crbug.com/657896.
+IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
+                       FrameNavigationEntry_RecreatedInjectedBlankSubframe) {
+  // 1. Start on a page that injects a nested iframe into an injected
+  // about:blank iframe.
+  GURL main_url(embedded_test_server()->GetURL(
+      "/navigation_controller/inject_subframe_into_blank_iframe.html"));
+  GURL blank_url(url::kAboutBlankURL);
+  GURL inner_url(
+      embedded_test_server()->GetURL("/navigation_controller/form.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), main_url));
+  NavigationControllerImpl& controller = static_cast<NavigationControllerImpl&>(
+      shell()->web_contents()->GetController());
+  FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
+                            ->GetFrameTree()
+                            ->root();
+
+  // Verify that the inner iframe was able to load.
+  ASSERT_EQ(1U, root->child_count());
+  ASSERT_EQ(1U, root->child_at(0)->child_count());
+  ASSERT_EQ(0U, root->child_at(0)->child_at(0)->child_count());
+  EXPECT_EQ(main_url, root->current_url());
+  EXPECT_EQ(blank_url, root->child_at(0)->current_url());
+  EXPECT_EQ(inner_url, root->child_at(0)->child_at(0)->current_url());
+
+  EXPECT_EQ(1, controller.GetEntryCount());
+  EXPECT_EQ(0, controller.GetLastCommittedEntryIndex());
+  NavigationEntryImpl* entry = controller.GetLastCommittedEntry();
+
+  // The entry should have FrameNavigationEntries for the subframes.
+  if (SiteIsolationPolicy::UseSubframeNavigationEntries()) {
+    ASSERT_EQ(1U, entry->root_node()->children.size());
+    EXPECT_EQ(blank_url, entry->root_node()->children[0]->frame_entry->url());
+    EXPECT_EQ(inner_url,
+              entry->root_node()->children[0]->children[0]->frame_entry->url());
+  }
+
+  // Set a value in the form which will be stored in the PageState.
+  EXPECT_TRUE(
+      ExecuteScript(root->child_at(0)->child_at(0),
+                    "document.getElementById('itext').value = 'modified';"));
+
+  // 2. Navigate the main frame same-site, destroying the subframes.
+  GURL main_url_2(embedded_test_server()->GetURL(
+      "/navigation_controller/simple_page_1.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), main_url_2));
+  ASSERT_EQ(0U, root->child_count());
+  EXPECT_EQ(main_url_2, root->current_url());
+
+  EXPECT_EQ(2, controller.GetEntryCount());
+  EXPECT_EQ(1, controller.GetLastCommittedEntryIndex());
+
+  // 3. Go back, recreating the subframes.
+  {
+    TestNavigationObserver back_load_observer(shell()->web_contents());
+    controller.GoBack();
+    back_load_observer.Wait();
+  }
+  ASSERT_EQ(1U, root->child_count());
+  EXPECT_EQ(main_url, root->current_url());
+  EXPECT_EQ(blank_url, root->child_at(0)->current_url());
+
+  // Verify that the inner iframe went to the correct URL.
+  EXPECT_EQ(inner_url, root->child_at(0)->child_at(0)->current_url());
+
+  EXPECT_EQ(2, controller.GetEntryCount());
+  EXPECT_EQ(0, controller.GetLastCommittedEntryIndex());
+  EXPECT_EQ(entry, controller.GetLastCommittedEntry());
+
+  // The entry should have FrameNavigationEntries for the subframes.
+  if (SiteIsolationPolicy::UseSubframeNavigationEntries()) {
+    ASSERT_EQ(1U, entry->root_node()->children.size());
+    EXPECT_EQ(blank_url, entry->root_node()->children[0]->frame_entry->url());
+    EXPECT_EQ(inner_url,
+              entry->root_node()->children[0]->children[0]->frame_entry->url());
+  }
+
+  // With injected about:blank iframes, we never restore form values from
+  // PageState.
+  std::string form_value;

[alexmos, 2016/10/21 06:34:16]

nit: maybe initialize to something other than ""?  Though I guess the
EXPECT_TRUE would catch any ExecuteScript... failures.

[Charlie Reis, 2016/10/21 07:34:30]

Done.

+  EXPECT_TRUE(
+      ExecuteScriptAndExtractString(root->child_at(0)->child_at(0),
+                                    "window.domAutomationController.send("
+                                    "document.getElementById('itext').value);",
+                                    &form_value));
+  EXPECT_EQ("", form_value);
+}
+
+// Verify that we correctly load a nested iframe created by an injected iframe
+// srcdoc if we go back and recreate the frames.  Also verify that form values
+// are correctly restored for forms within srcdoc frames, unlike forms injected
+// into about:blank pages (as tested in
+// FrameNavigationEntry_RecreatedInjectedBlankSubframe).
+//
+// This test worked before and after the fix for https://crbug.com/657896, but
+// it failed with a preliminary version of the fix.
+IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
+                       FrameNavigationEntry_RecreatedInjectedSrcdocSubframe) {
+  // 1. Start on a page that injects a nested iframe srcdoc which contains a
+  // nested iframe.
+  GURL main_url(embedded_test_server()->GetURL(
+      "/navigation_controller/inject_iframe_srcdoc_with_nested_frame.html"));
+  GURL blank_url(url::kAboutBlankURL);
+  GURL inner_url(
+      embedded_test_server()->GetURL("/navigation_controller/form.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), main_url));
+  NavigationControllerImpl& controller = static_cast<NavigationControllerImpl&>(
+      shell()->web_contents()->GetController());
+  FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
+                            ->GetFrameTree()
+                            ->root();
+
+  // Verify that the inner iframe was able to load.
+  ASSERT_EQ(1U, root->child_count());
+  ASSERT_EQ(1U, root->child_at(0)->child_count());
+  ASSERT_EQ(0U, root->child_at(0)->child_at(0)->child_count());
+  EXPECT_EQ(main_url, root->current_url());
+  EXPECT_EQ(blank_url, root->child_at(0)->current_url());
+  EXPECT_EQ(inner_url, root->child_at(0)->child_at(0)->current_url());
+
+  EXPECT_EQ(1, controller.GetEntryCount());
+  EXPECT_EQ(0, controller.GetLastCommittedEntryIndex());
+  NavigationEntryImpl* entry = controller.GetLastCommittedEntry();
+
+  // The entry should have FrameNavigationEntries for the subframes.
+  if (SiteIsolationPolicy::UseSubframeNavigationEntries()) {
+    ASSERT_EQ(1U, entry->root_node()->children.size());
+    EXPECT_EQ(blank_url, entry->root_node()->children[0]->frame_entry->url());
+    EXPECT_EQ(inner_url,
+              entry->root_node()->children[0]->children[0]->frame_entry->url());
+  }
+
+  // Set a value in the form which will be stored in the PageState.
+  EXPECT_TRUE(
+      ExecuteScript(root->child_at(0)->child_at(0),
+                    "document.getElementById('itext').value = 'modified';"));
+
+  // 2. Navigate the main frame same-site, destroying the subframes.
+  GURL main_url_2(embedded_test_server()->GetURL(
+      "/navigation_controller/simple_page_1.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), main_url_2));
+  ASSERT_EQ(0U, root->child_count());
+  EXPECT_EQ(main_url_2, root->current_url());
+
+  EXPECT_EQ(2, controller.GetEntryCount());
+  EXPECT_EQ(1, controller.GetLastCommittedEntryIndex());
+
+  // 3. Go back, recreating the subframes.
+  {
+    TestNavigationObserver back_load_observer(shell()->web_contents());
+    controller.GoBack();
+    back_load_observer.Wait();
+  }
+  ASSERT_EQ(1U, root->child_count());
+  // TODO(creis): This line is unexpectedly failing in PlzNavigate, so the test
+  // is disabled there for now.
+  ASSERT_EQ(1U, root->child_at(0)->child_count());

[Charlie Reis, 2016/10/21 04:39:25]

Not sure why this isn't working in PlzNavigate (even before my changes).  I've
had to disable the test in that mode for now.

[alexmos, 2016/10/21 06:34:16]

Acknowledged.  I tried it out locally, and it seems it alternates between
loading and not loading the iframes (i.e., going forward and back again would
work the second time).  Anyway, something to investigate separately.

+  ASSERT_EQ(0U, root->child_at(0)->child_at(0)->child_count());
+  EXPECT_EQ(main_url, root->current_url());
+  EXPECT_EQ(blank_url, root->child_at(0)->current_url());
+
+  // Verify that the inner iframe went to the correct URL.
+  EXPECT_EQ(inner_url, root->child_at(0)->child_at(0)->current_url());
+
+  EXPECT_EQ(2, controller.GetEntryCount());
+  EXPECT_EQ(0, controller.GetLastCommittedEntryIndex());
+  EXPECT_EQ(entry, controller.GetLastCommittedEntry());
+
+  // The entry should have FrameNavigationEntries for the subframes.
+  if (SiteIsolationPolicy::UseSubframeNavigationEntries()) {
+    ASSERT_EQ(1U, entry->root_node()->children.size());
+    EXPECT_EQ(blank_url, entry->root_node()->children[0]->frame_entry->url());
+    EXPECT_EQ(inner_url,
+              entry->root_node()->children[0]->children[0]->frame_entry->url());
+  }
+
+  // With injected iframe srcdoc pages, we do restore form values from
+  // PageState.
+  std::string form_value;
+  EXPECT_TRUE(
+      ExecuteScriptAndExtractString(root->child_at(0)->child_at(0),
+                                    "window.domAutomationController.send("
+                                    "document.getElementById('itext').value);",
+                                    &form_value));
+  EXPECT_EQ("modified", form_value);
+}
+
 // Ensure we don't crash if an onload handler removes an about:blank frame after
 // recreating it on a back/forward.  See https://crbug.com/638166.
 IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
                        FrameNavigationEntry_RemoveRecreatedBlankSubframe) {
   // 1. Start on a page that removes its about:blank iframe during onload.
   GURL main_url(embedded_test_server()->GetURL(
       "/navigation_controller/remove_blank_iframe_on_load.html"));
   GURL blank_url(url::kAboutBlankURL);
   EXPECT_TRUE(NavigateToURL(shell(), main_url));
   NavigationControllerImpl& controller = static_cast<NavigationControllerImpl&>(
@@ skipping 2253 matching lines @@
   subframe_delayer.WaitForNavigationFinished();
   EXPECT_EQ(1, controller.GetLastCommittedEntryIndex());
   EXPECT_EQ(url_a, root->current_url());
   EXPECT_EQ(frame_url_a2, root->child_at(0)->current_url());
 
   // Let the main frame commit.
   mainframe_delayer.WaitForNavigationFinished();
   EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
   EXPECT_EQ(2, controller.GetLastCommittedEntryIndex());
   EXPECT_EQ(url_b, root->current_url());
-
-  // TODO(creis): The child's current_url should be about:blank, but we're not
-  // currently getting a commit in this case.  For now, we'll lack a commit for
-  // this frame, similar to the slow URL case.  See https://crbug.com/626416.
-  if (SiteIsolationPolicy::UseSubframeNavigationEntries())
-    EXPECT_TRUE(root->child_at(0)->current_url().is_empty());
-  else
-    EXPECT_EQ(frame_url_b1, root->child_at(0)->current_url());
+  EXPECT_EQ(frame_url_b1, root->child_at(0)->current_url());
 
   // Check the PageState of the previous entry to ensure it isn't corrupted.
   NavigationEntry* entry = controller.GetEntryAtIndex(1);
   EXPECT_EQ(url_a, entry->GetURL());
   ExplodedPageState exploded_state;
   EXPECT_TRUE(
       DecodePageState(entry->GetPageState().ToEncodedData(), &exploded_state));
   EXPECT_EQ(url_a, GURL(exploded_state.top.url_string.string()));
   EXPECT_EQ(frame_url_a2,
             GURL(exploded_state.top.children.at(0).url_string.string()));
@@ skipping 1022 matching lines @@
   histogram.ExpectTotalCount(kReloadToReloadMetricName, 4);
   histogram.ExpectTotalCount(kReloadMainResourceToReloadMetricName, 3);
 
   controller.ReloadToRefreshContent(false);
   EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
   histogram.ExpectTotalCount(kReloadToReloadMetricName, 4);
   histogram.ExpectTotalCount(kReloadMainResourceToReloadMetricName, 3);
 }
 
 }  // namespace content