Fix View Frame Source for iframe of a file:// URL shows main frame's filename

chrome/browser/chrome_navigation_browsertest.cc

Index: chrome/browser/chrome_navigation_browsertest.cc
diff --git a/chrome/browser/chrome_navigation_browsertest.cc b/chrome/browser/chrome_navigation_browsertest.cc
index dddf739fab6b76efa8bbaa6d0de9be16a1134443..47af0002795310e19662ceba34638ccf2a784703 100644
--- a/chrome/browser/chrome_navigation_browsertest.cc
+++ b/chrome/browser/chrome_navigation_browsertest.cc
@@ -7,13 +7,18 @@
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/renderer_context_menu/render_view_context_menu_test_util.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
+#include "components/url_formatter/url_formatter.h"
+#include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/notification_service.h"
+#include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/context_menu_params.h"
+#include "content/public/common/url_constants.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "net/dns/mock_host_resolver.h"
@@ -132,6 +137,56 @@ IN_PROC_BROWSER_TEST_F(
   }
 }
 
+// Tests that viewing frame source on a local file:// page with an iframe
+// with a remote URL shows the correct tab title.
+IN_PROC_BROWSER_TEST_F(ChromeNavigationBrowserTest, TestViewFrameSource) {
+  StartServerWithExpiredCert();

[Charlie Reis, 2016/11/23 23:52:20]

Why do you need an expired cert?  The test above only needed it for the
interstitial.

[afakhry, 2016/11/24 01:21:08]

Sorry, that was a copy-paste line.

+
+  // The local page file:// URL.
+  GURL local_page_with_iframe_url = ui_test_utils::GetTestUrl(
+      base::FilePath(base::FilePath::kCurrentDirectory),
+      base::FilePath(FILE_PATH_LITERAL("iframe.html")));
+
+  // The non-file:// URL of the page to load in the iframe.
+  GURL iframe_target_url = embedded_test_server()->GetURL("/title1.html");
+  ui_test_utils::NavigateToURL(browser(), local_page_with_iframe_url);
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+
+  content::TestNavigationManager nav(web_contents, iframe_target_url);

[Charlie Reis, 2016/11/23 23:52:20]

It looks like a TestNavigationObserver might be a better fit here, since we
don't appear to be pausing or resuming the navigation.

nit: Use a more complete name, like observer or manager.

[afakhry, 2016/11/24 01:21:09]

Done.

+  ASSERT_TRUE(content::ExecuteScript(
+      web_contents->GetMainFrame(),
+      base::StringPrintf("var iframe = document.getElementById('test');\n"
+                         "iframe.setAttribute('src', '%s');\n",
+                         iframe_target_url.spec().c_str())));
+  nav.WaitForNavigationFinished();
+  WaitForLoadStop(web_contents);

[Charlie Reis, 2016/11/23 23:52:20]

I don't think we need this line (especially if we use TestNavigationObserver).

[afakhry, 2016/11/24 01:21:09]

Done.

+
+  content::RenderFrameHost* frame =
+      content::ChildFrameAt(web_contents->GetMainFrame(), 0);
+  ASSERT_TRUE(frame);
+  ASSERT_NE(frame, web_contents->GetMainFrame());
+
+  content::ContextMenuParams params;
+  params.page_url = local_page_with_iframe_url;
+  params.frame_url = frame->GetLastCommittedURL();
+  params.frame_page_state = content::PageState::CreateFromURL(params.frame_url);
+  TestRenderViewContextMenu menu(frame, params);
+  menu.Init();
+  menu.ExecuteCommand(IDC_CONTENT_CONTEXT_VIEWFRAMESOURCE, 0);
+  ASSERT_EQ(browser()->tab_strip_model()->count(), 2);
+  content::WebContents* new_web_contents =
+      browser()->tab_strip_model()->GetWebContentsAt(1);
+  ASSERT_NE(new_web_contents, web_contents);
+  WaitForLoadStop(new_web_contents);
+
+  GURL view_frame_source_url(
+      content::kViewSourceScheme + std::string(":") +
+      embedded_test_server()->GetURL("/title1.html").spec());

[Charlie Reis, 2016/11/23 23:52:20]

Use iframe_target_url here?

[afakhry, 2016/11/24 01:21:08]

Done.

+  EXPECT_EQ(url_formatter::FormatUrl(view_frame_source_url),
+            new_web_contents->GetTitle());
+}
+
 class ChromeNavigationPortMappedBrowserTest : public InProcessBrowserTest {
  public:
   ChromeNavigationPortMappedBrowserTest() {}