Fix cross origin check when deciding to show the HTTP auth interstitial.

chrome/browser/ui/login/login_prompt_browsertest.cc

 // Copyright (c) 2012 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 <algorithm>
 #include <list>
 #include <map>
 
 #include "base/metrics/field_trial.h"
 #include "base/strings/utf_string_conversions.h"
@@ skipping 43 matching lines @@
 
     AuthInfo(const std::string& username,
              const std::string& password)
         : username_(username), password_(password) {}
   };
 
   typedef std::map<std::string, AuthInfo> AuthMap;
 
   void SetAuthFor(LoginHandler* handler);
 
+  void TestCrossOriginPrompt(const GURL& visit_url,
+                             std::string landing_host) const;

[Peter Kasting, 2014/08/06 01:07:54]

Nit: Pass by const ref

[meacer, 2014/08/06 17:36:49]

Done.

+
   AuthMap auth_map_;
   std::string bad_password_;
   std::string bad_username_;
   std::string password_;
   std::string username_basic_;
   std::string username_digest_;
 };
 
 void LoginPromptBrowserTest::SetAuthFor(LoginHandler* handler) {
   const net::AuthChallengeInfo* challenge = handler->auth_info();
@@ skipping 702 matching lines @@
     ASSERT_EQ(1u, observer.handlers().size());
 
     while (!observer.handlers().empty()) {
       WindowedAuthCancelledObserver auth_cancelled_waiter(controller);
       LoginHandler* handler = *observer.handlers().begin();
 
       ASSERT_TRUE(handler);
       // When a cross origin iframe displays a login prompt, the blank
       // interstitial shouldn't be displayed and the omnibox should show the
       // main frame's url, not the iframe's.
-      EXPECT_EQ(new_host, contents->GetURL().host());
+      EXPECT_EQ(new_host, contents->GetVisibleURL().host());

[nasko, 2014/08/06 10:17:37]

Awesome!!!

 
       handler->CancelAuth();
       auth_cancelled_waiter.Wait();
     }
   }
 
   // Should stay on the main frame's url once the prompt the iframe is closed.
-  EXPECT_EQ("www.a.com", contents->GetURL().host());
+  EXPECT_EQ("www.a.com", contents->GetVisibleURL().host());
 
   EXPECT_EQ(1, observer.auth_needed_count());
   EXPECT_TRUE(test_server()->Stop());
 }
 
 IN_PROC_BROWSER_TEST_F(LoginPromptBrowserTest, SupplyRedundantAuths) {
   ASSERT_TRUE(test_server()->Start());
 
   // Get NavigationController for tab 1.
   content::WebContents* contents_1 =
@@ skipping 367 matching lines @@
 
   EXPECT_EQ(expected_title, contents->GetTitle());
   EXPECT_EQ(0, observer.auth_supplied_count());
   EXPECT_EQ(1, observer.auth_needed_count());
   EXPECT_EQ(1, observer.auth_cancelled_count());
   EXPECT_TRUE(test_server()->Stop());
 }
 
 // If a cross origin navigation triggers a login prompt, the destination URL
 // should be shown in the omnibox.
-IN_PROC_BROWSER_TEST_F(LoginPromptBrowserTest,
-                       ShowCorrectUrlForCrossOriginMainFrameRequests) {
-  const char* kTestPage = "files/login/cross_origin.html";
-  host_resolver()->AddRule("www.a.com", "127.0.0.1");
-  ASSERT_TRUE(test_server()->Start());
-
+void LoginPromptBrowserTest::TestCrossOriginPrompt(
+    const GURL& visit_url,
+    std::string auth_host) const {

[nasko, 2014/08/06 10:17:37]

nit: const std::string&

[meacer, 2014/08/06 17:36:49]

Done.

   content::WebContents* contents =
       browser()->tab_strip_model()->GetActiveWebContents();
   NavigationController* controller = &contents->GetController();
   LoginPromptBrowserTestObserver observer;
 
   observer.Register(content::Source<NavigationController>(controller));
 
-  // Load a page which navigates to a cross origin page with a login prompt.
+  // Load a page which will trigger a login prompt.
   {
-    GURL test_page = test_server()->GetURL(kTestPage);
-    ASSERT_EQ("127.0.0.1", test_page.host());
-
     WindowedAuthNeededObserver auth_needed_waiter(controller);
     browser()->OpenURL(OpenURLParams(
-        test_page, Referrer(), CURRENT_TAB, content::PAGE_TRANSITION_TYPED,
+        visit_url, Referrer(), CURRENT_TAB, content::PAGE_TRANSITION_TYPED,
         false));
-    ASSERT_EQ("127.0.0.1", contents->GetURL().host());
+    ASSERT_EQ(visit_url.host(), contents->GetVisibleURL().host());
     auth_needed_waiter.Wait();
     ASSERT_EQ(1u, observer.handlers().size());
     WaitForInterstitialAttach(contents);
 
     // The omnibox should show the correct origin for the new page when the
     // login prompt is shown.
-    EXPECT_EQ("www.a.com", contents->GetURL().host());
+    EXPECT_EQ(auth_host, contents->GetVisibleURL().host());
     EXPECT_TRUE(contents->ShowingInterstitialPage());
 
     // Cancel and wait for the interstitial to detach.
     LoginHandler* handler = *observer.handlers().begin();
     scoped_refptr<content::MessageLoopRunner> loop_runner(
         new content::MessageLoopRunner);
     InterstitialObserver interstitial_observer(contents,
                                                base::Closure(),
                                                loop_runner->QuitClosure());
     handler->CancelAuth();
     if (content::InterstitialPage::GetInterstitialPage(contents))
       loop_runner->Run();
-    EXPECT_EQ("www.a.com", contents->GetURL().host());
+    EXPECT_EQ(auth_host, contents->GetVisibleURL().host());
     EXPECT_FALSE(contents->ShowingInterstitialPage());
   }
 }
 
+// If a cross origin direct navigation triggers a login prompt, the login
+// interstitial should be shown.
+IN_PROC_BROWSER_TEST_F(LoginPromptBrowserTest,
+                       ShowCorrectUrlForCrossOriginMainFrameRequests) {
+  ASSERT_TRUE(test_server()->Start());
+
+  GURL test_page = test_server()->GetURL(kAuthBasicPage);
+  ASSERT_EQ("127.0.0.1", test_page.host());
+  std::string auth_host = "127.0.0.1";

[nasko, 2014/08/06 10:17:37]

nit: std::string auth_host("127.0.0.1");

[meacer, 2014/08/06 17:36:49]

Done.

+  TestCrossOriginPrompt(test_page, auth_host);
+}
+
+// If a cross origin redirect triggers a login prompt, the destination URL
+// should be shown in the omnibox when the auth dialog is displayed.
+IN_PROC_BROWSER_TEST_F(LoginPromptBrowserTest,
+                       ShowCorrectUrlForCrossOriginMainFrameRedirects) {
+  host_resolver()->AddRule("www.a.com", "127.0.0.1");
+  ASSERT_TRUE(test_server()->Start());
+
+  const char* kTestPage = "files/login/cross_origin.html";

[nasko, 2014/08/06 10:17:37]

Why not use std::string?

[meacer, 2014/08/06 17:36:49]

Mainly for consistency with other test cases. Happy to change though.

+  GURL test_page = test_server()->GetURL(kTestPage);
+  ASSERT_EQ("127.0.0.1", test_page.host());
+  std::string auth_host = "www.a.com";
+  TestCrossOriginPrompt(test_page, auth_host);
+}
+
 }  // namespace