Make sure a cookie can be deleted, even when forced to session only.

BUG=63258
TEST=net_unittests --gtest_filter=CookieMonsterTest.ForceSessionOnly

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

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

3 files changed, 48 insertions, 7 deletions

diff --git a/net/base/cookie_monster.cc b/net/base/cookie_monster.cc
index 2fb8d0f..caf98a3 100644
--- a/net/base/cookie_monster.cc
+++ b/net/base/cookie_monster.cc
@@ -692,12 +692,8 @@ static std::string CanonPath(const GURL& url,
   return CanonPathWithString(url, path_string);
 }
 
-static Time CanonExpiration(const CookieMonster::ParsedCookie& pc,
-                            const Time& current,
-                            const CookieOptions& options) {
-  if (options.force_session())
-    return Time();
-
+static Time CanonExpirationInternal(const CookieMonster::ParsedCookie& pc,
+                                    const Time& current) {
   // First, try the Max-Age attribute.
   uint64 max_age = 0;
   if (pc.HasMaxAge() &&
@@ -718,6 +714,21 @@ static Time CanonExpiration(const CookieMonster::ParsedCookie& pc,
   return Time();
 }
 
+static Time CanonExpiration(const CookieMonster::ParsedCookie& pc,
+                            const Time& current,
+                            const CookieOptions& options) {
+  Time expiration_time = CanonExpirationInternal(pc, current);
+
+  if (options.force_session()) {
+    // Only override the expiry  adte if it's in the future. If the expiry date
+    // is before the creation date, the cookie is supposed to be deleted.
+    if (expiration_time.is_null() || expiration_time > current)
+      return Time();
+  }
+
+  return expiration_time;
+}
+
 bool CookieMonster::HasCookieableScheme(const GURL& url) {
   lock_.AssertAcquired();
 
diff --git a/net/base/cookie_monster_unittest.cc b/net/base/cookie_monster_unittest.cc
index 751b255..6fce2a0 100644
--- a/net/base/cookie_monster_unittest.cc
+++ b/net/base/cookie_monster_unittest.cc
@@ -2033,4 +2033,32 @@ TEST(CookieMonsterTest, GarbageCollectionTriggers) {
   }
 }
 
+// This test checks that setting a cookie forcing it to be a session only
+// cookie works as expected.
+TEST(CookieMonsterTest, ForceSessionOnly) {
+  GURL url_google(kUrlGoogle);
+  scoped_refptr<net::CookieMonster> cm(new net::CookieMonster(NULL, NULL));
+  net::CookieOptions options;
+
+  // Set a persistent cookie, but force it to be a session cookie.
+  options.set_force_session();
+  ASSERT_TRUE(cm->SetCookieWithOptions(url_google,
+      std::string(kValidCookieLine) + "; expires=Mon, 18-Apr-22 22:50:13 GMT",
+      options));
+
+  // Get the canonical cookie.
+  CookieMonster::CookieList cookie_list = cm->GetAllCookies();
+  ASSERT_EQ(1U, cookie_list.size());
+  ASSERT_FALSE(cookie_list[0].IsPersistent());
+
+  // Use a past expiry date to delete the cookie, but force it to session only.
+  ASSERT_TRUE(cm->SetCookieWithOptions(url_google,
+      std::string(kValidCookieLine) + "; expires=Mon, 18-Apr-1977 22:50:13 GMT",
+      options));
+
+  // Check that the cookie was deleted.
+  cookie_list = cm->GetAllCookies();
+  ASSERT_EQ(0U, cookie_list.size());
+}
+
 }  // namespace
diff --git a/net/base/cookie_options.h b/net/base/cookie_options.h
index 8ace523..203adaf 100644
--- a/net/base/cookie_options.h
+++ b/net/base/cookie_options.h
@@ -24,7 +24,9 @@ class CookieOptions {
   void set_include_httponly() { exclude_httponly_ = false; }
   bool exclude_httponly() const { return exclude_httponly_; }
 
-  // Forces a cookie to be saved as a session cookie.
+  // Forces a cookie to be saved as a session cookie. If the expiration time of
+  // the cookie is in the past, i.e. the cookie would end up being deleted, this
+  // option is ignored. See CookieMonsterTest.ForceSessionOnly.
   void set_force_session() { force_session_ = true; }
   bool force_session() const { return force_session_; }