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

chrome/browser/ui/login/login_prompt.cc

Index: chrome/browser/ui/login/login_prompt.cc
diff --git a/chrome/browser/ui/login/login_prompt.cc b/chrome/browser/ui/login/login_prompt.cc
index 53f68a7b4a8f518502bdf7c005e1aaa274b15769..ca60fdaa7d663c5b2f344a3d1358e573a42a169c 100644
--- a/chrome/browser/ui/login/login_prompt.cc
+++ b/chrome/browser/ui/login/login_prompt.cc
@@ -510,7 +510,10 @@ void LoginDialogCallback(const GURL& request_url,
   }
 
   if (is_main_frame &&
-      parent_contents->GetVisibleURL().GetOrigin() != request_url.GetOrigin()) {
+      (parent_contents->GetVisibleURL().GetOrigin() !=
+          request_url.GetOrigin() ||
+       parent_contents->GetLastCommittedURL().GetOrigin() !=
+          request_url.GetOrigin())) {

[Peter Kasting, 2014/08/05 20:18:10]

It seems a bit weird that we check both of these.

If the two are the same, we should only need to check one.  If the two aren't
the same, then it seems like one must be the one we want to check, and the other
not.  In either case, I'd think we'd only ever want to check one.

Clearly always checking just the visible URL isn't correct or you wouldn't be
making a change.  So does always checking the last committed URL work?  If not,
why not, and how can we write the code so that we know when we should check one
and when the other?

[meacer, 2014/08/05 22:29:19]

The two different cases are:

1- The user navigates directly by entering a url in the omnibox. In this case:

resource_url!=GetLastCommittedURL and resource_url==GetVisibleURL

because the navigation hasn't committed yet (which is the oddity of requests
with HTTP Auth headers).

2- The page redirects to a cross origin resource. In this case:

resource_url!=GetLastCommitedURL and resource_url!=GetVisibleURL

So you are right, it looks like we don't need to check for GetVisibleURL. I'm
trying to think of a situation where resource_url isn't equal to
GetLastCommittedURL but is is equal to GetVisibleURL, but I can't find any.

[Peter Kasting, 2014/08/05 23:15:34]

I would probably change to just checking against GetLastCommittedURL() then. 
You might want to take the comment that's inside the conditional here and hoist
it above, and expand it to note how it catches both scenarios you describe.

[meacer, 2014/08/05 23:32:23]

Done.

     // Show a blank interstitial for main-frame, cross origin requests
     // so that the correct URL is shown in the omnibox.
     base::Closure callback = base::Bind(&ShowLoginPrompt,