Change, unify, and specify ordering of cookies from CookieMonster.

Affected interfaces:
	* GetCookiesWithOptions (used by HTTP requests)
	* GetAllCookiesForURL (used by extensions and Cookies UI)
	* GetAllCookies (used by Cookies UI).

BUG=8850 (indirectly)
TEST=Standard cookie monster tests, unit_tests::CookiesTreeModelTest.OriginOrdering, browser_tests::ExtensionApiTest.Cookies

Review URL: http://codereview.chromium.org/3170034

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

3 files changed, 64 insertions, 2 deletions

diff --git a/net/base/cookie_monster.cc b/net/base/cookie_monster.cc
index 029d841..262da6a 100644
--- a/net/base/cookie_monster.cc
+++ b/net/base/cookie_monster.cc
@@ -1163,9 +1163,19 @@ CookieMonster::CookieList CookieMonster::GetAllCookies() {
                         CookieMapItPair(cookies_.begin(), cookies_.end()),
                         NULL);
 
-  CookieList cookie_list;
+  // Copy the CanonicalCookie pointers from the map so that we can use the same
+  // sorter as elsewhere, then copy the result out.
+  std::vector<CanonicalCookie*> cookie_ptrs;
+  cookie_ptrs.reserve(cookies_.size());
   for (CookieMap::iterator it = cookies_.begin(); it != cookies_.end(); ++it)
-    cookie_list.push_back(*it->second);
+    cookie_ptrs.push_back(it->second);
+  std::sort(cookie_ptrs.begin(), cookie_ptrs.end(), CookieSorter);
+
+  CookieList cookie_list;
+  cookie_list.reserve(cookie_ptrs.size());
+  for (std::vector<CanonicalCookie*>::const_iterator it = cookie_ptrs.begin();
+       it != cookie_ptrs.end(); ++it)
+    cookie_list.push_back(**it);
 
   return cookie_list;
 }
@@ -1179,6 +1189,7 @@ CookieMonster::CookieList CookieMonster::GetAllCookiesForURL(const GURL& url) {
 
   std::vector<CanonicalCookie*> cookie_ptrs;
   FindCookiesForHostAndDomain(url, options, false, &cookie_ptrs);
+  std::sort(cookie_ptrs.begin(), cookie_ptrs.end(), CookieSorter);
 
   CookieList cookies;
   for (std::vector<CanonicalCookie*>::const_iterator it = cookie_ptrs.begin();
diff --git a/net/base/cookie_monster.h b/net/base/cookie_monster.h
index e6aa48dc..9ec6433 100644
--- a/net/base/cookie_monster.h
+++ b/net/base/cookie_monster.h
@@ -88,6 +88,8 @@ class CookieMonster : public CookieStore {
   virtual bool SetCookieWithOptions(const GURL& url,
                                     const std::string& cookie_line,
                                     const CookieOptions& options);
+  // The returned cookies are ordered by longest path, then earliest
+  // creation date.
   virtual std::string GetCookiesWithOptions(const GURL& url,
                                             const CookieOptions& options);
   virtual void DeleteCookie(const GURL& url, const std::string& cookie_name);
@@ -109,11 +111,15 @@ class CookieMonster : public CookieStore {
 
   // Returns all the cookies, for use in management UI, etc. This does not mark
   // the cookies as having been accessed.
+  // The returned cookies are ordered by longest path, then earliest
+  // creation date.
   CookieList GetAllCookies();
 
   // Returns all the cookies, for use in management UI, etc. Filters results
   // using given url scheme, host / domain and path. This does not mark the
   // cookies as having been accessed.
+  // The returned cookies are ordered by longest path, then earliest
+  // creation date.
   CookieList GetAllCookiesForURL(const GURL& url);
 
   // Delete all of the cookies.
diff --git a/net/base/cookie_monster_unittest.cc b/net/base/cookie_monster_unittest.cc
index 3ab9953..cf9bb0a 100644
--- a/net/base/cookie_monster_unittest.cc
+++ b/net/base/cookie_monster_unittest.cc
@@ -1761,4 +1761,49 @@ TEST(CookieMonsterTest, UniqueCreationTime) {
   }
 }
 
+TEST(CookieMonsterTest, CookieOrdering) {
+  // Put a random set of cookies into a monster and make sure
+  // they're returned in the right order.
+  scoped_refptr<net::CookieMonster> cm = new CookieMonster(NULL, NULL);
+  EXPECT_TRUE(cm->SetCookie(GURL("http://d.c.b.a.google.com/aa/x.html"),
+                            "c=1"));
+  EXPECT_TRUE(cm->SetCookie(GURL("http://b.a.google.com/aa/bb/cc/x.html"),
+                            "d=1; domain=b.a.google.com"));
+  EXPECT_TRUE(cm->SetCookie(GURL("http://b.a.google.com/aa/bb/cc/x.html"),
+                            "a=4; domain=b.a.google.com"));
+  EXPECT_TRUE(cm->SetCookie(GURL("http://c.b.a.google.com/aa/bb/cc/x.html"),
+                            "e=1; domain=c.b.a.google.com"));
+  EXPECT_TRUE(cm->SetCookie(GURL("http://d.c.b.a.google.com/aa/bb/x.html"),
+                            "b=1"));
+  EXPECT_TRUE(cm->SetCookie(GURL("http://news.bbc.co.uk/midpath/x.html"),
+                            "g=10"));
+  EXPECT_EQ("d=1; a=4; e=1; b=1; c=1",
+            cm->GetCookiesWithOptions(
+                GURL("http://d.c.b.a.google.com/aa/bb/cc/dd"),
+                CookieOptions()));
+  {
+    unsigned int i = 0;
+    CookieMonster::CookieList cookies(cm->GetAllCookiesForURL(
+        GURL("http://d.c.b.a.google.com/aa/bb/cc/dd")));
+    ASSERT_EQ(5u, cookies.size());
+    EXPECT_EQ("d", cookies[i++].Name());
+    EXPECT_EQ("a", cookies[i++].Name());
+    EXPECT_EQ("e", cookies[i++].Name());
+    EXPECT_EQ("b", cookies[i++].Name());
+    EXPECT_EQ("c", cookies[i++].Name());
+  }
+
+  {
+    unsigned int i = 0;
+    CookieMonster::CookieList cookies(cm->GetAllCookies());
+    ASSERT_EQ(6u, cookies.size());
+    EXPECT_EQ("d", cookies[i++].Name());
+    EXPECT_EQ("a", cookies[i++].Name());
+    EXPECT_EQ("e", cookies[i++].Name());
+    EXPECT_EQ("g", cookies[i++].Name());
+    EXPECT_EQ("b", cookies[i++].Name());
+    EXPECT_EQ("c", cookies[i++].Name());
+  }
+}
+
 } // namespace