Encrypt all stored cookies on selected operating systems.

content/browser/net/sqlite_persistent_cookie_store.h

Index: content/browser/net/sqlite_persistent_cookie_store.h
diff --git a/content/browser/net/sqlite_persistent_cookie_store.h b/content/browser/net/sqlite_persistent_cookie_store.h
index 65e39824eac39e1b4b041e98f1de66b10b7d1bda..42cb934be9e44942b6c97633f7ed34f804f64193 100644
--- a/content/browser/net/sqlite_persistent_cookie_store.h
+++ b/content/browser/net/sqlite_persistent_cookie_store.h
@@ -32,6 +32,7 @@ class SpecialStoragePolicy;
 }
 
 namespace content {
+class CookieCryptoDelegate;
 
 // Implements the PersistentCookieStore interface in terms of a SQLite database.
 // For documentation about the actual member functions consult the documentation
@@ -43,13 +44,14 @@ class CONTENT_EXPORT SQLitePersistentCookieStore
  public:
   // All blocking database accesses will be performed on
   // |background_task_runner|, while |client_task_runner| is used to invoke
-  // callbacks.
+  // callbacks.  This object takes ownership of the |crypto_delegate|.
   SQLitePersistentCookieStore(
       const base::FilePath& path,
       const scoped_refptr<base::SequencedTaskRunner>& client_task_runner,
       const scoped_refptr<base::SequencedTaskRunner>& background_task_runner,
       bool restore_old_session_cookies,
-      quota::SpecialStoragePolicy* special_storage_policy);
+      quota::SpecialStoragePolicy* special_storage_policy,
+      CookieCryptoDelegate* crypto_delegate);

[erikwright (departed), 2013/10/07 20:39:25]

make this a scoped_ptr. That is the modern way to document transfer of
ownership.

[bcwhite, 2013/10/08 16:10:25]

Could you provide an example?  A search for "takes\ ownership" shows everything
accepting a raw pointer.

Thanks.

[erikwright (departed), 2013/10/08 16:50:56]

Precisely. With the new model, comments are no longer required. scoped_ptr<X> is
an explicit indication of ownership transfer.

https://code.google.com/p/chromium/codesearch#search/&q=scoped_ptr%3C%5Ba-z0-...

[bcwhite, 2013/10/08 18:23:36]

I've tried this and it doesn't work well.  You can't pass a pointer to a derived
class or NULL, at least one of which is necessary.

[erikwright (departed), 2013/10/08 18:41:52]

Yes you can, and we do in much code:

scoped_ptr<X>() for a NULL pointer.

class Y : public X {};
void foobar(scoped_ptr<X> x);

void client_code() {
  scoped_ptr<Y> y(new Y)
  foobar(y.PassAs<X>());
}

[bcwhite, 2013/10/08 20:18:09]

That means creating a scoped_ptr<Y> in the caller just to cast/pass it as a
scoped_ptr<X> which in turn passes it again (to a backend) as another
scoped_ptr<X> before finally being passed to the member variable.  As it is, it
just passes "new Crypto" as the parameter.

This seems like a horrible (and inefficient) hack, using explicit casts
defeating C++'s built-in polymorphism, for something that is effectively a
code-comment.  You're sure about this change?

[erikwright (departed), 2013/10/08 20:24:35]

No. It will look like this:

new SQLitePersistentCookieStore(
  ...,
#ifdef ...
  scoped_ptr<CookieCryptoDelegate>(new CookieCryptoDelegateImpl));
#else
  scoped_ptr<CookieCryptoDelegate>());
#endif

or, if you take the MaybeCreate route:

new SQLitePersistentCookieStore(
  ...,
  CookieCryptoDelegateImpl::MaybeCreate().PassAs<CookieCryptoDelegate>());

MaybeCreate() would look like:

scoped_ptr<CookieCryptoDelegateImpl> CookieCryptoDelegateImpl::MaybeCreate() {
#ifdef ...
  return scoped_ptr<CookieCryptoDelegateImpl>(new CookieCryptoDelegateImpl);
#else
  return scoped_ptr<CookieCryptoDelegateImpl>();
#endif
}

It's not effectively a code comment. It forces call-sites to explicitly give up
ownership, reduces the likelihood of use-after-free, memory leak, and other
types of errors, and improves readability by making ownership transfers
immediately apparent.

[bcwhite, 2013/10/08 21:08:39]

Done.

 
   // net::CookieMonster::PersistentCookieStore:
   virtual void Load(const LoadedCallback& loaded_callback) OVERRIDE;