DO NOT LAND

net/ftp/ftp_auth_cache.h

Index: net/ftp/ftp_auth_cache.h
diff --git a/net/ftp/ftp_auth_cache.h b/net/ftp/ftp_auth_cache.h
index 526b3588782d062d47712c96848ec1a231f7c6f5..52172326f43836ff920d278c871ca8303a8cfb7b 100644
--- a/net/ftp/ftp_auth_cache.h
+++ b/net/ftp/ftp_auth_cache.h
@@ -9,7 +9,7 @@
 
 #include "net/base/auth.h"
 #include "net/base/net_export.h"
-#include "url/gurl.h"
+#include "url/origin.h"
 
 namespace net {
 
@@ -27,10 +27,10 @@ class NET_EXPORT_PRIVATE FtpAuthCache {
   static const size_t kMaxEntries;
 
   struct Entry {
-    Entry(const GURL& origin, const AuthCredentials& credentials);
+    Entry(const url::Origin& origin, const AuthCredentials& credentials);
     ~Entry();
 
-    GURL origin;
+    url::Origin origin;
     AuthCredentials credentials;
   };
 
@@ -38,15 +38,15 @@ class NET_EXPORT_PRIVATE FtpAuthCache {
   ~FtpAuthCache();
 
   // Return Entry corresponding to given |origin| or NULL if not found.
-  Entry* Lookup(const GURL& origin);
+  Entry* Lookup(const url::Origin& origin);
 
   // Add an entry for |origin| to the cache using |credentials|. If there is
   // already an entry for |origin|, it will be overwritten.
-  void Add(const GURL& origin, const AuthCredentials& credentials);
+  void Add(const url::Origin& origin, const AuthCredentials& credentials);
 
   // Remove the entry for |origin| from the cache, if one exists and matches
   // |credentials|.
-  void Remove(const GURL& origin, const AuthCredentials& credentials);
+  void Remove(const url::Origin& origin, const AuthCredentials& credentials);

[Ryan Sleevi, 2015/05/22 02:50:03]

So, overall, changes like this make me... Generally nervous. And I guess this
probably highlights some of my unease re: url::Origin

Yes, if you get rid of GURL::GetOrigin(), then this is necessary. But this comes
with it a non-trivial, subtle behaviour change that has to be evaluated on a
case by case basis. That is, we have to make sure that you can't pass a 'null'
origin here, or someone could flood the cache. In the existing behaviour, all
such invalid origins would collapse into a single origin.

You can argue it's good ("but they COULD BE different") or bad (if anyone can
get junk in, they can do a DoS)

That's why I'm nervous about the overall approach. I absolutely don't think a
bombing run like this can be rubberstamped - every one of these calls has to be
checked for callers to see if they're handling user input, if bad origins could
be provided, what the expectations are, etc.

 
  private:
   typedef std::list<Entry> EntryList;