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

chrome/browser/ui/login/login_handler.cc

Index: chrome/browser/ui/login/login_handler.cc
diff --git a/chrome/browser/ui/login/login_handler.cc b/chrome/browser/ui/login/login_handler.cc
index 926540da7ae6421437d782642093bcd70b349711..2a3ecdf5b5b5c5d098d25dbaa168064f16b19e83 100644
--- a/chrome/browser/ui/login/login_handler.cc
+++ b/chrome/browser/ui/login/login_handler.cc
@@ -41,6 +41,7 @@
 #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"
@@ -449,7 +450,7 @@ std::string LoginHandler::GetSignonRealm(
     // 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 {
@@ -475,12 +476,12 @@ PasswordForm LoginHandler::MakeInputForPasswordManager(
     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;
@@ -498,7 +499,7 @@ void LoginHandler::GetDialogStrings(const GURL& request_url,
         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,