Remove use of CanonicalCookie::Source() from browsing_data.

chrome/browser/browsing_data/cookies_tree_model.cc

Index: chrome/browser/browsing_data/cookies_tree_model.cc
diff --git a/chrome/browser/browsing_data/cookies_tree_model.cc b/chrome/browser/browsing_data/cookies_tree_model.cc
index 959624e12a0d987bad35f31d0e4275cfa89e3e80..755cfbdb9c53d352e0718bffc512e6ca86c141a3 100644
--- a/chrome/browser/browsing_data/cookies_tree_model.cc
+++ b/chrome/browser/browsing_data/cookies_tree_model.cc
@@ -117,21 +117,6 @@ std::string CanonicalizeHost(const GURL& url) {
   return retval;
 }
 
-// When creating a cookie tree node from a cookie, strip the port of the
-// cookie source and treat all non-file:// URLs as http://.
-GURL CanonicalizeCookieSource(const net::CanonicalCookie& cookie) {
-  GURL url = cookie.Source();
-  if (url.SchemeIsFile())
-    return url;
-
-  url::Replacements<char> replacements;
-  replacements.ClearPort();
-  if (url.SchemeIsCryptographic())
-    replacements.SetScheme("http", url::Component(0, 4));
-
-  return url.GetOrigin().ReplaceComponents(replacements);
-}
-
 #if defined(ENABLE_EXTENSIONS)
 bool TypeIsProtected(CookieTreeNode::DetailedInfo::NodeType type) {
   switch (type) {
@@ -1219,18 +1204,13 @@ void CookiesTreeModel::PopulateCookieInfoWithFilter(
   notifier->StartBatchUpdate();
   for (CookieList::iterator it = container->cookie_list_.begin();
        it != container->cookie_list_.end(); ++it) {
-    GURL source = CanonicalizeCookieSource(*it);
-    if (source.is_empty() || !group_by_cookie_source_) {

[Mike West, 2016/06/29 05:58:42]

I think this is the only spot where `group_by_cookie_source_` is used. If we
stop using it here, we can stop using it everywhere. The only place it seems to
be set to true is in the CookiesTreeModel created in
`LocalSharedObjectsContainer::CreateCookiesTreeModel`, but I don't think it has
any substantial effect on things (especially given the `source()` weirdness you
note.

Would you mind removing it in this patch? I hope it doesn't explode the size; it
looks pretty straightforward.

[mmenke, 2016/06/30 22:03:43]

Done.

-      std::string domain = it->Domain();
-      if (domain.length() > 1 && domain[0] == '.')
-        domain = domain.substr(1);
-
-      // We treat secure cookies just the same as normal ones.
-      source = GURL(std::string(url::kHttpScheme) +
-                    url::kStandardSchemeSeparator + domain + "/");
-    }
-    if (!source.SchemeIsHTTPOrHTTPS())

[Mike West, 2016/06/29 05:58:42]

1. Oh how I wish we didn't need to support WebView's crazy use of cookies on
`file:` URLs. :(

2. This patch is going to lump all `file:` cookies together rather than skipping
them. It would be lovely if we had any tests at all that verified that behavior.
But since we don't, it must not be important, right?

[mmenke, 2016/06/29 13:02:26]

We're *already* doing that, for cookies loaded from disk.  The persistent cookie
store doesn't save or load source GURLs.  So trying to improve file cookie
behavior at this layer seems a futile endeavor.  See
https://cs.chromium.org/chromium/src/net/extras/sqlite/sqlite_persistent_cook...

The cookie store doesn't seem to separate out file cookies in any way (Which is
why it has the scary warning out it).

Is this code even used by WebView?  I assumed since it was in Chrome, it was
not.

-      continue;
+    std::string domain = it->Domain();
+    if (domain.length() > 1 && domain[0] == '.')
+      domain = domain.substr(1);
+
+    // Secure cookies are treated just the same as normal ones.

[Mike West, 2016/06/29 05:58:42]

Since you're touching it anyway, how about "// Cookies ignore schemes, so group
all HTTPS and HTTP cookies together."?

[mmenke, 2016/06/30 22:03:42]

Done.

+    GURL source = GURL(std::string(url::kHttpScheme) +
+                       url::kStandardSchemeSeparator + domain + "/");
 
     if (filter.empty() || (CookieTreeHostNode::TitleForUrl(source)
                                .find(filter) != base::string16::npos)) {