Disallow top-level navigation in gaia iframe

chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc

Index: chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc
diff --git a/chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc b/chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc
index 35469d9ad32cbf9777c14e4624349fde28691e29..6ad0efca56fcac9bd64f87db1ac20f91c3fecede 100644
--- a/chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc
+++ b/chrome/browser/ui/webui/signin/inline_login_ui_browsertest.cc
@@ -18,6 +18,8 @@
 #include "content/public/browser/web_ui_controller.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/test/browser_test_utils.h"
+#include "net/base/url_util.h"
+#include "net/test/embedded_test_server/embedded_test_server.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -161,3 +163,23 @@ IN_PROC_BROWSER_TEST_F(InlineLoginUISafeIframeBrowserTest, NoWebUIInIframe) {
   EXPECT_CALL(foo_provider(), NewWebUI(_, _)).Times(0);
   ui_test_utils::NavigateToURL(browser(), url);
 }
+
+// Make sure that the gaia iframe cannot trigger top-frame navigation.
+IN_PROC_BROWSER_TEST_F(InlineLoginUISafeIframeBrowserTest,
+    TopFrameNavigationDisallowed) {
+  ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
+
+  // Loads into gaia iframe a web page that attempts to deframe on load.
+  GURL deframe_url(embedded_test_server()->GetURL("/login/deframe.html"));
+  GURL url(net::AppendOrReplaceQueryParameter(
+      signin::GetPromoURL(signin::SOURCE_START_PAGE, false),
+      "frameUrl", deframe_url.spec()));
+
+  ui_test_utils::NavigateToURL(browser(), url);

[xiyuan, 2014/04/17 01:15:29]

NavigateToURL waits for one LOAD_STOP notification. Would it reflect the loading
state in iframe?  If yes, would it be good enough for the deframe script to run
and finish?

[guohui, 2014/04/17 12:43:26]

it does, but the iframe is initially set to blank page, thus the test passes
without the fix in this CL...

Corrected in a new patch.

+  content::WebContents* contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  EXPECT_EQ(url, contents->GetURL());

[Charlie Reis, 2014/04/17 01:20:20]

GetLastCommittedURL()

[guohui, 2014/04/17 12:43:26]

GetLastCommittedURL returns the committed url, but in this case, even if top
frame navigation is allowed, the navigation would be pending, thus
GetLastCommittedURL would always return the old URL. Verified without the fix in
a local build.

Changed to GetVisibleURL.

+
+  content::NavigationController& controller = contents->GetController();
+  EXPECT_TRUE(controller.GetPendingEntry() == NULL);

[Charlie Reis, 2014/04/17 01:20:20]

This doesn't seem like a sufficient check to me.  We could have committed the
deframe navigation and this would pass.  Can you also check that
GetLastCommittedURL() is what we expect?

[guohui, 2014/04/17 12:43:26]

After i added the WaitForUIReady call above, both the GetVisibleURL check and
GetPendingEntry check fail without my fix.

+}