SessionRestore: Store session cookies and restore them if chrome crashes or auto-restarts.

net/base/cookie_monster.cc

Index: net/base/cookie_monster.cc
diff --git a/net/base/cookie_monster.cc b/net/base/cookie_monster.cc
index 03061c6851041bf6b9e2a6ab707a52b57f076fde..cd9ba40d45ffd52f39412cbbcab42384399c9403 100644
--- a/net/base/cookie_monster.cc
+++ b/net/base/cookie_monster.cc
@@ -320,6 +320,8 @@ ChangeCausePair ChangeCauseMapping[] = {
   { CookieMonster::Delegate::CHANGE_COOKIE_EVICTED, true },
   // DELETE_COOKIE_EXPIRED_OVERWRITE
   { CookieMonster::Delegate::CHANGE_COOKIE_EXPIRED_OVERWRITE, true },
+  // DELETE_COOKIE_OLD_SESSION_COOKIE
+  { CookieMonster::Delegate::CHANGE_COOKIE_EXPLICIT, false },
   // DELETE_COOKIE_LAST_ENTRY
   { CookieMonster::Delegate::CHANGE_COOKIE_EXPLICIT, false }
 };
@@ -370,7 +372,9 @@ CookieMonster::CookieMonster(PersistentCookieStore* store, Delegate* delegate)
           TimeDelta::FromSeconds(kDefaultAccessUpdateThresholdSeconds)),
       delegate_(delegate),
       last_statistic_record_time_(Time::Now()),
-      keep_expired_cookies_(false) {
+      keep_expired_cookies_(false),
+      persist_session_cookies_(false),
+      old_session_cookie_behavior_(SESSION_COOKIES_UNDECIDED) {
   InitializeHistograms();
   SetDefaultCookieableSchemes();
 }
@@ -386,7 +390,9 @@ CookieMonster::CookieMonster(PersistentCookieStore* store,
           last_access_threshold_milliseconds)),
       delegate_(delegate),
       last_statistic_record_time_(base::Time::Now()),
-      keep_expired_cookies_(false) {
+      keep_expired_cookies_(false),
+      persist_session_cookies_(false),
+      old_session_cookie_behavior_(SESSION_COOKIES_UNDECIDED) {
   InitializeHistograms();
   SetDefaultCookieableSchemes();
 }
@@ -1399,6 +1405,44 @@ CookieMonster* CookieMonster::GetCookieMonster() {
   return this;
 }
 
+void CookieMonster::SetPersistSessionCookies(bool persist_session_cookies) {
+  persist_session_cookies_ = persist_session_cookies;
+}
+
+void CookieMonster::SaveSessionCookies() {
+  if (store_) {
+    store_->SetClearLocalStateOnExit(false);

[erikwright (departed), 2011/11/28 16:18:16]

...

[erikwright (departed), 2011/11/28 16:31:44]

Never mind, I've answered my own question.

+  }
+}
+
+void CookieMonster::RestoreOldSessionCookies() {
+  old_session_cookie_behavior_ = SESSION_COOKIES_MERGE;
+  for (CookieMap::iterator it = cookies_.begin(); it != cookies_.end(); ++it) {
+    it->second->SetIsOldSessionCookie(false);
+    if (delegate_) {
+      delegate_->OnCookieChanged(
+          *it->second, false, CookieMonster::Delegate::CHANGE_COOKIE_EXPLICIT);

[erikwright (departed), 2011/11/28 16:18:16]

...

[erikwright (departed), 2011/11/28 16:31:44]

I don't know what I was planning to comment on here.

+    }
+  }
+}
+
+void CookieMonster::DiscardOldSessionCookies() {
+  old_session_cookie_behavior_ = SESSION_COOKIES_DELETE;
+  // It is ok to call DeleteSessionCookies even if |store_| hasn't initialized
+  // yet.
+  if (store_)
+    store_->DeleteSessionCookies();

[erikwright (departed), 2011/11/28 16:18:16]

Won't this also delete any new session cookies set for domains visited while the
restore prompt was up?

[erikwright (departed), 2011/11/28 16:31:44]

I suppose one solution is to pass a timestamp (i.e., the time that loading
started) and only delete the ones not created/modified since then.

Another solution is to rely on deletion during InternalDeleteCookie (i.e., pass
true for sync_to_store).

[marja, 2011/11/29 12:56:01]

Yep, it was buggy. The change of approach (always restore after a crash, don't
wait for the infobar) removes this problem.

+  for (CookieMap::iterator it = cookies_.begin(); it != cookies_.end();) {
+    CookieMap::iterator curit = it;
+    ++it;
+    if (curit->second->IsOldSessionCookie()) {
+      // Don't sync this to store. Session cookies will be deleted from the
+      // store separately.
+      InternalDeleteCookie(curit, false, DELETE_COOKIE_OLD_SESSION_COOKIE);
+    }
+  }
+}
+
 CookieMonster::~CookieMonster() {
   DeleteAll(false);
 }
@@ -1468,10 +1512,26 @@ void CookieMonster::StoreLoadedCookies(
 
   for (std::vector<CanonicalCookie*>::const_iterator it = cookies.begin();
        it != cookies.end(); ++it) {
+    bool notify = true;
+    if (!(*it)->IsPersistent()) {
+      if (old_session_cookie_behavior_ == SESSION_COOKIES_DELETE) {
+        // Ignore this cookie; old session cookies are getting deleted from the
+        // database.
+        continue;
+      } else if (old_session_cookie_behavior_ == SESSION_COOKIES_UNDECIDED) {
+        // Mark restored session cookies as old session cookies, so they can be
+        // discarded if needed.
+        (*it)->SetIsOldSessionCookie(true);
+        notify = false;
+      }
+      // else, |old_session_cookie_behavior_| is |SESSION_COOKIES_MERGE|, and we
+      // treat the old session cookie as a normal cookie.
+    }
+
     int64 cookie_creation_time = (*it)->CreationDate().ToInternalValue();
 
     if (creation_times_.insert(cookie_creation_time).second) {
-      InternalInsertCookie(GetKey((*it)->Domain()), *it, false);
+      InternalInsertCookie(GetKey((*it)->Domain()), *it, false, notify);
       const Time cookie_access_time((*it)->LastAccessDate());
       if (earliest_access_time_.is_null() ||
           cookie_access_time < earliest_access_time_)
@@ -1713,6 +1773,14 @@ void CookieMonster::FindCookiesForKey(
       continue;
     }
 
+    if (cc->IsOldSessionCookie()) {
+      // The cookie is an old session cookie, and we're supposed to ignore
+      // it. If |old_session_cookie_behavior_| is something else, |cookies_|
+      // contains no cookies marked as session cookies.

[erikwright (departed), 2011/11/28 16:18:16]

'as _old_ session cookies'

[marja, 2011/11/29 12:56:01]

(Obsolete)

+      DCHECK(old_session_cookie_behavior_ == SESSION_COOKIES_UNDECIDED);
+      continue;

[erikwright (departed), 2011/11/28 16:18:16]

I think we should also be deleting this cookie (from the Cookie Monster and the
SQLitePersistentCookieStore) here.

[marja, 2011/11/29 12:56:01]

(Obsolete)

+    }
+
     // Filter out HttpOnly cookies, per options.
     if (options.exclude_httponly() && cc->IsHttpOnly())
       continue;
@@ -1771,13 +1839,15 @@ bool CookieMonster::DeleteAnyEquivalentCookie(const std::string& key,
 
 void CookieMonster::InternalInsertCookie(const std::string& key,
                                          CanonicalCookie* cc,
-                                         bool sync_to_store) {
+                                         bool sync_to_store,
+                                         bool notify) {
   lock_.AssertAcquired();
 
-  if (cc->IsPersistent() && store_ && sync_to_store)
+  if ((cc->IsPersistent() || persist_session_cookies_) &&
+      store_ && sync_to_store)
     store_->AddCookie(*cc);
   cookies_.insert(CookieMap::value_type(key, cc));
-  if (delegate_.get()) {
+  if (notify && delegate_.get()) {
     delegate_->OnCookieChanged(
         *cc, false, CookieMonster::Delegate::CHANGE_COOKIE_EXPLICIT);
   }
@@ -1853,6 +1923,23 @@ bool CookieMonster::SetCanonicalCookie(scoped_ptr<CanonicalCookie>* cc,
   VLOG(kVlogSetCookies) << "SetCookie() key: " << key << " cc: "
                         << (*cc)->DebugString();
 
+  if (persist_session_cookies_ &&
+      old_session_cookie_behavior_ == SESSION_COOKIES_UNDECIDED) {
+    // We're setting a cookie, and there are session cookies for which we

[jochen (gone - plz use gerrit), 2011/11/28 15:47:19]

I think we should also delete all session cookies for a key when we read cookies
for that key, wdyt?

[erikwright (departed), 2011/11/28 16:18:16]

Also, this should occur before the DeleteAnyEquivalentCookie statement. A
pre-existing HTTPONLY session cookie would alter the behaviour there.

[marja, 2011/11/29 12:56:01]

(Obsolete.)

+    // haven't yet decided whether to merge them or not. Delete all old session
+    // cookies for the same key. If |old_session_cookie_behavior_| is something
+    // else than |SESSION_COOKIES_UNDECIDED|, |cookies_| contains no cookies
+    // marked as session cookies.

[erikwright (departed), 2011/11/28 16:18:16]

'as _old_ session cookies'

[marja, 2011/11/29 12:56:01]

(Obsolete.)

+    CookieMapItPair its = cookies_.equal_range(key);
+    while (its.first != its.second) {
+      CookieMap::iterator curit = its.first;
+      ++its.first;
+      if (curit->second->IsOldSessionCookie()) {
+        InternalDeleteCookie(curit, true, DELETE_COOKIE_EXPLICIT);
+      }
+    }
+  }
+
   // Realize that we might be setting an expired cookie, and the only point
   // was to delete the cookie which we've already done.
   if (!already_expired || keep_expired_cookies_) {
@@ -1862,7 +1949,7 @@ bool CookieMonster::SetCanonicalCookie(scoped_ptr<CanonicalCookie>* cc,
           ((*cc)->ExpiryDate() - creation_time).InMinutes());
     }
 
-    InternalInsertCookie(key, cc->release(), true);
+    InternalInsertCookie(key, cc->release(), true, true);
   }
 
   // We assume that hopefully setting a cookie will be less common than
@@ -1891,7 +1978,7 @@ void CookieMonster::InternalUpdateCookieAccessTime(CanonicalCookie* cc,
       (current - cc->LastAccessDate()).InMinutes());
 
   cc->SetLastAccessDate(current);
-  if (cc->IsPersistent() && store_)
+  if ((cc->IsPersistent() || persist_session_cookies_) && store_)
     store_->UpdateCookieAccessTime(*cc);
 }
 
@@ -1913,7 +2000,8 @@ void CookieMonster::InternalDeleteCookie(CookieMap::iterator it,
   CanonicalCookie* cc = it->second;
   VLOG(kVlogSetCookies) << "InternalDeleteCookie() cc: " << cc->DebugString();
 
-  if (cc->IsPersistent() && store_ && sync_to_store)
+  if ((cc->IsPersistent() || persist_session_cookies_)
+      && store_ && sync_to_store)
     store_->DeleteCookie(*cc);
   if (delegate_.get()) {
     ChangeCausePair mapping = ChangeCauseMapping[deletion_cause];
@@ -2519,7 +2607,8 @@ CookieMonster::CanonicalCookie::CanonicalCookie()
     : secure_(false),
       httponly_(false),
       has_expires_(false),
-      is_persistent_(false) {
+      is_persistent_(false),
+      is_old_session_cookie_(false) {
   SetSessionCookieExpiryTime();
 }
 
@@ -2543,7 +2632,8 @@ CookieMonster::CanonicalCookie::CanonicalCookie(
       secure_(secure),
       httponly_(httponly),
       has_expires_(has_expires),
-      is_persistent_(is_persistent) {
+      is_persistent_(is_persistent),
+      is_old_session_cookie_(false) {
   if (!has_expires_) {
     DCHECK(!is_persistent_);
     SetSessionCookieExpiryTime();
@@ -2563,7 +2653,8 @@ CookieMonster::CanonicalCookie::CanonicalCookie(const GURL& url,
       secure_(pc.IsSecure()),
       httponly_(pc.IsHttpOnly()),
       has_expires_(pc.HasExpires()),
-      is_persistent_(pc.HasExpires()) {
+      is_persistent_(pc.HasExpires()),
+      is_old_session_cookie_(false) {
   if (has_expires_)
     expiry_date_ = CanonExpiration(pc, creation_date_);
   else