Replace usage of GURL(origin.Serialize()) with origin.GetURL()

chrome/browser/ui/login/login_handler.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 "chrome/browser/ui/login/login_handler.h"
 
 #include <string>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 23 matching lines @@
 #include "content/public/common/origin_util.h"
 #include "net/base/auth.h"
 #include "net/base/host_port_pair.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_auth_scheme.h"
 #include "net/http/http_transaction_factory.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "ui/gfx/text_elider.h"
+#include "url/origin.h"
 
 #if defined(ENABLE_EXTENSIONS)
 #include "components/guest_view/browser/guest_view_base.h"
 #include "extensions/browser/view_type_utils.h"
 #endif
 
 using autofill::PasswordForm;
 using content::BrowserThread;
 using content::NavigationController;
 using content::RenderViewHost;
@@ skipping 388 matching lines @@
 
 // static
 std::string LoginHandler::GetSignonRealm(
     const GURL& url,
     const net::AuthChallengeInfo& auth_info) {
   std::string signon_realm;
   if (auth_info.is_proxy) {
     // Historically we've been storing the signon realm for proxies using
     // net::HostPortPair::ToString().
     net::HostPortPair host_port_pair =
-        net::HostPortPair::FromURL(GURL(auth_info.challenger.Serialize()));
+        net::HostPortPair::FromURL(auth_info.challenger.GetURL());
     signon_realm = host_port_pair.ToString();
     signon_realm.append("/");
   } else {
     // Take scheme, host, and port from the url.
     signon_realm = url.GetOrigin().spec();
     // This ends with a "/".
   }
   signon_realm.append(auth_info.realm);
   return signon_realm;
 }
 
 // static
 PasswordForm LoginHandler::MakeInputForPasswordManager(
     const GURL& request_url,
     const net::AuthChallengeInfo& auth_info) {
   PasswordForm dialog_form;
   if (base::LowerCaseEqualsASCII(auth_info.scheme, net::kBasicAuthScheme)) {
     dialog_form.scheme = PasswordForm::SCHEME_BASIC;
   } else if (base::LowerCaseEqualsASCII(auth_info.scheme,
                                         net::kDigestAuthScheme)) {
     dialog_form.scheme = PasswordForm::SCHEME_DIGEST;
   } else {
     dialog_form.scheme = PasswordForm::SCHEME_OTHER;
   }
   if (auth_info.is_proxy) {
-    dialog_form.origin = GURL(auth_info.challenger.Serialize());
+    dialog_form.origin = auth_info.challenger.GetURL();
   } else if (!auth_info.challenger.IsSameOriginWith(url::Origin(request_url))) {
     dialog_form.origin = GURL();
     NOTREACHED();  // crbug.com/32718
   } else {
-    dialog_form.origin = GURL(auth_info.challenger.Serialize());
+    dialog_form.origin = auth_info.challenger.GetURL();
   }

[Peter Kasting, 2016/10/05 00:32:08]

I don't want to force you to clean this up in this patch, but this whole
conditional looks strange to me.

It seems like it should equivalently* be:

  dialog_form.origin = auth_info.challenger.GetURL();
  DCHECK(auth_info.is_proxy ||
         auth_info.challenger.IsSameOriginWith(url::Origin(request_url)));

*Except that we now don't handle DCHECK failure, since the style guide bans
that.  If there is a security issue here, then probably the DCHECK should be a
CHECK instead?  The referenced Chromium bug is long fixed, so I'm not really
sure what the reference is supposed to mean (maybe "this bugfix should have
resulted in this being unreachable").

If you don't want to make this change in this CL, would you be willing to post
it as a separate CL and find a reviewer familiar enough with this specific code
to look?

[Charlie Harrison, 2016/10/05 12:32:03]

Yikes you're right, this should get cleaned up. I threw up a dependent patch
here:
https://codereview.chromium.org/2392243002/

   dialog_form.signon_realm = GetSignonRealm(dialog_form.origin, auth_info);
   return dialog_form;
 }
 
 // static
 void LoginHandler::GetDialogStrings(const GURL& request_url,
                                     const net::AuthChallengeInfo& auth_info,
                                     base::string16* authority,
                                     base::string16* explanation) {
   GURL authority_url;
 
   if (auth_info.is_proxy) {
     *authority = l10n_util::GetStringFUTF16(
         IDS_LOGIN_DIALOG_PROXY_AUTHORITY,
         url_formatter::FormatOriginForSecurityDisplay(
             auth_info.challenger, url_formatter::SchemeDisplay::SHOW));
-    authority_url = GURL(auth_info.challenger.Serialize());
+    authority_url = auth_info.challenger.GetURL();
   } else {
     *authority = l10n_util::GetStringFUTF16(
         IDS_LOGIN_DIALOG_AUTHORITY,
         url_formatter::FormatUrlForSecurityDisplay(request_url));
     authority_url = request_url;
   }
 
   if (!content::IsOriginSecure(authority_url)) {
     // TODO(asanka): The string should be different for proxies and servers.
     // http://crbug.com/620756
@@ skipping 138 matching lines @@
   bool is_main_frame =
       (request->load_flags() & net::LOAD_MAIN_FRAME_DEPRECATED) != 0;
   LoginHandler* handler = LoginHandler::Create(auth_info, request);
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&LoginHandler::LoginDialogCallback, request->url(),
                  base::RetainedRef(auth_info), base::RetainedRef(handler),
                  is_main_frame));
   return handler;
 }