Sort the origins in the cookies_tree_model by effective TLD, as opposed

to a lexicographical sort on the entire origin as a string.

BUG=28192
TEST=cookies_tree_model_unittest.cc
Review URL: http://codereview.chromium.org/404039

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@32522 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 87 insertions, 8 deletions

diff --git a/chrome/browser/cookies_tree_model.cc b/chrome/browser/cookies_tree_model.cc
index 4819c98..736160f 100644
--- a/chrome/browser/cookies_tree_model.cc
+++ b/chrome/browser/cookies_tree_model.cc
@@ -20,6 +20,7 @@
 #include "grit/generated_resources.h"
 #include "grit/theme_resources.h"
 #include "net/base/cookie_monster.h"
+#include "net/base/registry_controlled_domain.h"
 #include "net/url_request/url_request_context.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 
@@ -59,7 +60,54 @@ class OriginNodeComparator {
  public:
   bool operator() (const CookieTreeNode* lhs,
                    const CookieTreeNode* rhs) {
-    return (lhs->GetTitle() < rhs->GetTitle());
+    // We want to order by registry controlled domain, so we would get
+    // google.com, ad.google.com, www.google.com,
+    // microsoft.com, ad.microsoft.com. CanonicalizeHost transforms the origins
+    // into a form like google.com.www so that string comparisons work.
+    return (CanonicalizeHost(lhs->GetTitle()) <
+            CanonicalizeHost(rhs->GetTitle()));
+  }
+
+ private:
+  std::string CanonicalizeHost(const std::wstring& host_w) {
+    // The canonicalized representation makes the registry controlled domain
+    // come first, and then adds subdomains in reverse order, e.g.
+    // 1.mail.google.com would become google.com.mail.1, and then a standard
+    // string comparison works to order hosts by registry controlled domain
+    // first. Leading dots are ignored, ".google.com" is the same as
+    // "google.com".
+
+    std::string host = WideToUTF8(host_w);
+    std::string retval = net::RegistryControlledDomainService::
+        GetDomainAndRegistry(host);
+    if (!retval.length())  // Is an IP address or other special origin.
+      return host;
+
+    std::string::size_type position = host.rfind(retval);
+
+    // The host may be the registry controlled domain, in which case fail fast.
+    if (position == 0 || position == std::string::npos)
+      return host;
+
+    // If host is www.google.com, retval will contain google.com at this point.
+    // Start operating to the left of the registry controlled domain, e.g. in
+    // the www.google.com example, start at index 3.
+    --position;
+
+    // If position == 0, that means it's a dot; this will be ignored to treat
+    // ".google.com" the same as "google.com".
+    while (position > 0) {
+      retval += std::string(".");
+      // Copy up to the next dot. host[position] is a dot so start after it.
+      std::string::size_type next_dot = host.rfind(".", position - 1);
+      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;
   }
 };
 
diff --git a/chrome/browser/cookies_tree_model_unittest.cc b/chrome/browser/cookies_tree_model_unittest.cc
index 199dcaf..317c003 100644
--- a/chrome/browser/cookies_tree_model_unittest.cc
+++ b/chrome/browser/cookies_tree_model_unittest.cc
@@ -154,7 +154,7 @@ TEST_F(CookiesTreeModelTest, Remove) {
     // foo2 -> cookies -> b, foo3 -> cookies -> c
     EXPECT_EQ(10, cookies_model.GetRoot()->GetTotalNodeCount());
   }
-    DeleteCookie(cookies_model.GetRoot()->GetChild(0));
+  DeleteCookie(cookies_model.GetRoot()->GetChild(0));
   {
     SCOPED_TRACE("First origin removed");
     EXPECT_STREQ("B,C", GetMonsterCookies(monster).c_str());
@@ -176,7 +176,7 @@ TEST_F(CookiesTreeModelTest, RemoveCookiesNode) {
     // foo2 -> cookies -> b, foo3 -> cookies -> c
     EXPECT_EQ(10, cookies_model.GetRoot()->GetTotalNodeCount());
   }
-    DeleteCookie(cookies_model.GetRoot()->GetChild(0)->GetChild(0));
+  DeleteCookie(cookies_model.GetRoot()->GetChild(0)->GetChild(0));
   {
     SCOPED_TRACE("First origin removed");
     EXPECT_STREQ("B,C", GetMonsterCookies(monster).c_str());
@@ -201,7 +201,7 @@ TEST_F(CookiesTreeModelTest, RemoveCookieNode) {
     // foo2 -> cookies -> b, foo3 -> cookies -> c
     EXPECT_EQ(10, cookies_model.GetRoot()->GetTotalNodeCount());
   }
-    DeleteCookie(cookies_model.GetRoot()->GetChild(1)->GetChild(0));
+  DeleteCookie(cookies_model.GetRoot()->GetChild(1)->GetChild(0));
   {
     SCOPED_TRACE("Second origin COOKIES node removed");
     EXPECT_STREQ("A,C", GetMonsterCookies(monster).c_str());
@@ -229,7 +229,7 @@ TEST_F(CookiesTreeModelTest, RemoveSingleCookieNode) {
     EXPECT_STREQ("A,B,C,D", GetMonsterCookies(monster).c_str());
     EXPECT_STREQ("A,B,C,D", GetDisplayedCookies(&cookies_model).c_str());
   }
-    DeleteCookie(cookies_model.GetRoot()->GetChild(2));
+  DeleteCookie(cookies_model.GetRoot()->GetChild(2));
   {
     SCOPED_TRACE("Third origin removed");
     EXPECT_STREQ("A,B", GetMonsterCookies(monster).c_str());
@@ -255,8 +255,8 @@ TEST_F(CookiesTreeModelTest, RemoveSingleCookieNodeOf3) {
     EXPECT_STREQ("A,B,C,D,E", GetMonsterCookies(monster).c_str());
     EXPECT_STREQ("A,B,C,D,E", GetDisplayedCookies(&cookies_model).c_str());
   }
-    DeleteCookie(cookies_model.GetRoot()->GetChild(2)->GetChild(0)->
-        GetChild(1));
+  DeleteCookie(cookies_model.GetRoot()->GetChild(2)->GetChild(0)->
+      GetChild(1));
   {
     SCOPED_TRACE("Middle cookie in third origin removed");
     EXPECT_STREQ("A,B,C,E", GetMonsterCookies(monster).c_str());
@@ -282,7 +282,7 @@ TEST_F(CookiesTreeModelTest, RemoveSecondOrigin) {
     EXPECT_STREQ("A,B,C,D,E", GetMonsterCookies(monster).c_str());
     EXPECT_STREQ("A,B,C,D,E", GetDisplayedCookies(&cookies_model).c_str());
   }
-    DeleteCookie(cookies_model.GetRoot()->GetChild(1));
+  DeleteCookie(cookies_model.GetRoot()->GetChild(1));
   {
     SCOPED_TRACE("Second origin removed");
     EXPECT_STREQ("A,C,D,E", GetMonsterCookies(monster).c_str());
@@ -292,4 +292,35 @@ TEST_F(CookiesTreeModelTest, RemoveSecondOrigin) {
   }
 }
 
+TEST_F(CookiesTreeModelTest, OriginOrdering) {
+  net::CookieMonster* monster = profile_->GetCookieMonster();
+  monster->SetCookie(GURL("http://a.foo2.com"), "A=1");
+  monster->SetCookie(GURL("http://foo2.com"), "B=1");
+  monster->SetCookie(GURL("http://b.foo1.com"), "C=1");
+  monster->SetCookie(GURL("http://foo4.com"), "D=1; domain=.foo4.com;"
+      " path=/;");  // Leading dot on the foo4
+  monster->SetCookie(GURL("http://a.foo1.com"), "E=1");
+  monster->SetCookie(GURL("http://foo1.com"), "F=1");
+  monster->SetCookie(GURL("http://foo3.com"), "G=1");
+  monster->SetCookie(GURL("http://foo4.com"), "H=1");
+
+  CookiesTreeModel cookies_model(profile_.get());
+
+  {
+    SCOPED_TRACE("Initial State 8 cookies");
+    // D starts with a ., CookieMonster orders that lexicographically first
+    EXPECT_STREQ("D,E,A,C,F,B,G,H", GetMonsterCookies(monster).c_str());
+    EXPECT_STREQ("F,E,C,B,A,G,D,H",
+        GetDisplayedCookies(&cookies_model).c_str());
+  }
+  DeleteCookie(cookies_model.GetRoot()->GetChild(1));  // Delete "E"
+  {
+    SCOPED_TRACE("Second origin removed");
+    EXPECT_STREQ("D,A,C,F,B,G,H", GetMonsterCookies(monster).c_str());
+    EXPECT_STREQ("F,C,B,A,G,D,H", GetDisplayedCookies(&cookies_model).c_str());
+  }
+}
+
+
+
 }  // namespace