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

chrome/browser/browsing_data/cookies_tree_model.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/browsing_data/cookies_tree_model.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <functional>
@@ skipping 99 matching lines @@
     if (next_dot == std::string::npos) {
       retval += host.substr(0, position);
       break;
     }
     retval += host.substr(next_dot + 1, position - (next_dot + 1));
     position = next_dot;
   }
   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) {
     // Fall through each below cases to return true.
     case CookieTreeNode::DetailedInfo::TYPE_DATABASE:
     case CookieTreeNode::DetailedInfo::TYPE_LOCAL_STORAGE:
     case CookieTreeNode::DetailedInfo::TYPE_SESSION_STORAGE:
     case CookieTreeNode::DetailedInfo::TYPE_APPCACHE:
     case CookieTreeNode::DetailedInfo::TYPE_INDEXED_DB:
     case CookieTreeNode::DetailedInfo::TYPE_FILE_SYSTEM:
@@ skipping 1067 matching lines @@
 
 void CookiesTreeModel::PopulateCookieInfoWithFilter(
     LocalDataContainer* container,
     ScopedBatchUpdateNotifier* notifier,
     const base::string16& filter) {
   CookieTreeRootNode* root = static_cast<CookieTreeRootNode*>(GetRoot());
 
   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);
+    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;
+    // 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)) {
       CookieTreeHostNode* host_node = root->GetOrCreateHostNode(source);
       CookieTreeCookiesNode* cookies_node =
           host_node->GetOrCreateCookiesNode();
       CookieTreeCookieNode* new_cookie = new CookieTreeCookieNode(it);
       cookies_node->AddCookieNode(new_cookie);
     }
   }
@@ skipping 298 matching lines @@
 void CookiesTreeModel::MaybeNotifyBatchesEnded() {
   // Only notify the observers if this is the outermost call to EndBatch() if
   // called in a nested manner.
   if (batches_ended_ == batches_started_ &&
       batches_seen_ == batches_expected_) {
     FOR_EACH_OBSERVER(Observer,
                       cookies_observer_list_,
                       TreeModelEndBatch(this));
   }
 }