Remove URL fragment from referrer HTTP header when opening link using "Open Link in New Tab" option.

chrome/browser/tab_contents/render_view_context_menu_browsertest.cc

Index: chrome/browser/tab_contents/render_view_context_menu_browsertest.cc
diff --git a/chrome/browser/tab_contents/render_view_context_menu_browsertest.cc b/chrome/browser/tab_contents/render_view_context_menu_browsertest.cc
index a3db8e8c27572fdc49f78e87fb71ebb5b75eb62e..666099c533da0a8b4ab3df3c392746e1452e6ce9 100644
--- a/chrome/browser/tab_contents/render_view_context_menu_browsertest.cc
+++ b/chrome/browser/tab_contents/render_view_context_menu_browsertest.cc
@@ -166,4 +166,50 @@ IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest, CopyLinkFromIncognito) {
       ui::Clipboard::GetPlainTextFormatType(), ui::Clipboard::BUFFER_STANDARD));
 }
 
+// Open link in new tab shouldn't send URL fragment as referrer

[pauljensen, 2013/03/14 18:53:44]

Our comments are almost always complete sentances.  Please change this to
"Verify ... referrer."

[mef, 2013/03/14 20:31:38]

Done.

+IN_PROC_BROWSER_TEST_F(ContextMenuBrowserTest, OpenInNewTabReferrer) {
+  EXPECT_FALSE(browser()->profile()->IsOffTheRecord());

[pauljensen, 2013/03/14 18:53:44]

Why do we need a non-incognito window?

[mef, 2013/03/14 20:31:38]

I guess we don't.

+  ui_test_utils::WindowedTabAddedNotificationObserver tab_observer(
+      content::NotificationService::AllSources());
+
+  ASSERT_TRUE(test_server()->Start());
+  GURL echoheader_url(test_server()->GetURL("echoheader?Referer"));
+
+  // Go to a page with a link to echoheader url

[pauljensen, 2013/03/14 18:53:44]

Capitalize URL and add period.

[mef, 2013/03/14 20:31:38]

Done.

+  ui_test_utils::NavigateToURL(
+      browser(), GURL("data:text/html,<a href='" + echoheader_url.spec()
+                      + "'>link</a>"));
+
+  // set up referrer url with fragment

[pauljensen, 2013/03/14 18:53:44]

Capitalize "set" and URL and add period.

[mef, 2013/03/14 20:31:38]

Done.

+  GURL referrer_with_fragment_url("http://foo.com/test#fragment");

[pauljensen, 2013/03/14 18:53:44]

Do we need the "_url" suffix?

[mef, 2013/03/14 20:31:38]

Done.

+  GURL referrer_url("http://foo.com/test");

[pauljensen, 2013/03/14 18:53:44]

Can we name this more descriptively, like "correct_referrer".

[mef, 2013/03/14 20:31:38]

Done.

+
+  // Copy link URL.
+  content::ContextMenuParams context_menu_params;
+  context_menu_params.page_url = GURL(referrer_with_fragment_url);
+  context_menu_params.link_url = GURL(echoheader_url);
+
+  // The menu_observer will select "Open in new tab", wait for the new tab to
+  // be added.
+  TestRenderViewContextMenu menu(
+      browser()->tab_strip_model()->GetActiveWebContents(),
+      context_menu_params);
+  menu.Init();
+  menu.ExecuteCommand(IDC_CONTENT_CONTEXT_OPENLINKNEWTAB, 0);
+
+  tab_observer.Wait();
+  content::WebContents* tab = tab_observer.GetTab();
+  content::WaitForLoadStop(tab);
+
+  // Verify that it's the correct tab.
+  EXPECT_EQ(echoheader_url, tab->GetURL());

[pauljensen, 2013/03/14 18:53:44]

Should this be ASSERT instead of EXPECT?  I imagine the rest of the test won't
work if this fails.

[mef, 2013/03/14 20:31:38]

Done.

+  // Verify that the data on the page matches referrer_url (without fragment)

[pauljensen, 2013/03/14 18:53:44]

Add period.  When mentioning variable names in comments we prepend and suffix
"|", so "|referrer_url|" should be how "referrer_url" should appear.

[mef, 2013/03/14 20:31:38]

Done.

+  std::string actual;
+  ASSERT_TRUE(content::ExecuteScriptAndExtractString(
+      tab,
+      "window.domAutomationController.send(window.document.referrer);",
+      &actual));
+  EXPECT_EQ(referrer_url.spec(), actual);
+}
+
 }  // namespace