Move sqlite_channel_id_store from chrome/browser/net to net/extras.

net/sqlite/sqlite_server_bound_cert_store.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chrome/browser/net/sqlite_server_bound_cert_store.h"
+#include "net/sqlite/sqlite_server_bound_cert_store.h"
 
 #include <list>
 #include <set>
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/file_util.h"
 #include "base/files/file_path.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/metrics/histogram.h"
 #include "base/strings/string_util.h"
 #include "base/threading/thread.h"
 #include "base/threading/thread_restrictions.h"
 #include "net/cert/x509_certificate.h"
 #include "net/cookies/cookie_util.h"
+#include "net/sqlite/special_storage_policy_delegate.h"
 #include "net/ssl/ssl_client_cert_type.h"
 #include "sql/error_delegate_util.h"
 #include "sql/meta_table.h"
 #include "sql/statement.h"
 #include "sql/transaction.h"
-#include "third_party/sqlite/sqlite3.h"
 #include "url/gurl.h"
-#include "webkit/browser/quota/special_storage_policy.h"
+
+namespace net {
 
 // This class is designed to be shared between any calling threads and the
 // background task runner. It batches operations and commits them on a timer.
 class SQLiteServerBoundCertStore::Backend
     : public base::RefCountedThreadSafe<SQLiteServerBoundCertStore::Backend> {
  public:
   Backend(
       const base::FilePath& path,
       const scoped_refptr<base::SequencedTaskRunner>& background_task_runner,
-      quota::SpecialStoragePolicy* special_storage_policy)
+      scoped_ptr<SpecialStoragePolicyDelegate> special_storage_policy)
       : path_(path),
         num_pending_(0),
         force_keep_session_state_(false),
         background_task_runner_(background_task_runner),
-        special_storage_policy_(special_storage_policy),
+        special_storage_policy_(special_storage_policy.Pass()),
         corruption_detected_(false) {}
 
   // Creates or loads the SQLite database.
   void Load(const LoadedCallback& loaded_callback);
 
   // Batch a server bound cert addition.
   void AddServerBoundCert(
       const net::DefaultServerBoundCertStore::ServerBoundCert& cert);
 
   // Batch a server bound cert deletion.
@@ skipping 68 matching lines @@
   // True if the persistent store should skip clear on exit rules.
   bool force_keep_session_state_;
   // Guard |pending_|, |num_pending_| and |force_keep_session_state_|.
   base::Lock lock_;
 
   // Cache of origins we have certificates stored for.
   std::set<std::string> cert_origins_;
 
   scoped_refptr<base::SequencedTaskRunner> background_task_runner_;
 
-  scoped_refptr<quota::SpecialStoragePolicy> special_storage_policy_;
+  scoped_ptr<SpecialStoragePolicyDelegate> special_storage_policy_;
 
   // Indicates if the kill-database callback has been scheduled.
   bool corruption_detected_;
 
   DISALLOW_COPY_AND_ASSIGN(Backend);
 };
 
 // Version number of the database.
 static const int kCurrentVersionNumber = 4;
 static const int kCompatibleVersionNumber = 1;
@@ skipping 437 matching lines @@
 
   sql::Transaction transaction(db_.get());
   if (!transaction.Begin()) {
     LOG(WARNING) << "Unable to delete certificates on shutdown.";
     return;
   }
 
   for (std::set<std::string>::iterator it = cert_origins_.begin();
        it != cert_origins_.end(); ++it) {
     const GURL url(net::cookie_util::CookieOriginToURL(*it, true));
     if (!url.is_valid() || !special_storage_policy_->IsStorageSessionOnly(url))

[Ryan Sleevi, 2014/07/14 19:18:04]

So, to start the discussion:

If we look at this, the goal of this special_storage_policy_ is to clean up all
OBCs that are marked as temporary (vis-a-vis session-only storage).

Question 1: Does this IsStorageSessionOnly change dynamically (from creation)?
My understanding was/is "No", but it might be worth checking.

If no:
- Can we plumb whether or not a given origin is/should be temporary at creation,
rather than at shutdown?

Question 2: Does it make more sense to have the "purge temporaries" be an
exposed bit of functionality, and let the higher layer (//content? //chrome?
Does this affect iOS/CroNet embedders?) purge the temporaries.


My gut feeling is that this distinction - between persistence and impermanence -
may not be right for this class.

For example, this only considers impermanence at shutdown. If we think
IsStorageSessionOnly() may change for a URL, then it's possible we create an OBC
when permanence is denied, but when it's allowed, and we shutdown, we still
allow the OBC to be saved to disk. Note that in both cases, we *always* write it
to the SQLite store, which equally seems... Odd.

Just thinking out loud here.

[mef, 2014/07/14 20:18:02]

I see your concern, although I kind of agree with bludell's suggestion to
separate c14n/refactoring from functional changes. I also assume that it
currently functions as intended. :-)

[mef, 2014/07/15 18:53:03]

I think the answer is 'Maybe'. The quota::SpecialStoragePolicy is an interface,
which doesn't mention static vs. dynamic requirement. It appears to have 4
implementations, only one of which is neither mock, nor trivial:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ext...
in that it forwards the call to cookie_settings_->IsCookieSessionOnly(origin); 
Please correct me if I'm wrong, but I suppose that cookie settings could change
dynamically during session, and therefore IsStorageSessionOnly cannot be
guaranteed to be the same.
I'd say not always.
Um, but how would that higher level do that? Do we need to add "purge
temporaries" interface to lower level?
 
Umm, so what's wrong in this scenario? Both cookie and its cert will or will not
be saved according to current permanence setting. I admit that if permanence is
denied and browser is killed before/without cleanup, then some data may remain
in the database (depending on its flush points).
We could add another in-memory-only layer for non-permanent storage, but maybe
this is a job for higher layer.
I appreciate that and would like to hear more opinions on this topic.

       continue;
     del_smt.Reset(true);
     del_smt.BindString(0, *it);
     if (!del_smt.Run())
       NOTREACHED() << "Could not delete a certificate from the DB.";
   }
 
   if (!transaction.Commit())
     LOG(WARNING) << "Unable to delete certificates on shutdown.";
 }
 
 void SQLiteServerBoundCertStore::Backend::SetForceKeepSessionState() {
   base::AutoLock locked(lock_);
   force_keep_session_state_ = true;
 }
 
 SQLiteServerBoundCertStore::SQLiteServerBoundCertStore(
     const base::FilePath& path,
     const scoped_refptr<base::SequencedTaskRunner>& background_task_runner,
-    quota::SpecialStoragePolicy* special_storage_policy)
+    scoped_ptr<SpecialStoragePolicyDelegate> special_storage_policy)
     : backend_(new Backend(path,
                            background_task_runner,
-                           special_storage_policy)) {}
+                           special_storage_policy.Pass())) {}
 
 void SQLiteServerBoundCertStore::Load(
     const LoadedCallback& loaded_callback) {
   backend_->Load(loaded_callback);
 }
 
 void SQLiteServerBoundCertStore::AddServerBoundCert(
     const net::DefaultServerBoundCertStore::ServerBoundCert& cert) {
   backend_->AddServerBoundCert(cert);
 }
 
 void SQLiteServerBoundCertStore::DeleteServerBoundCert(
     const net::DefaultServerBoundCertStore::ServerBoundCert& cert) {
   backend_->DeleteServerBoundCert(cert);
 }
 
 void SQLiteServerBoundCertStore::SetForceKeepSessionState() {
   backend_->SetForceKeepSessionState();
 }
 
 SQLiteServerBoundCertStore::~SQLiteServerBoundCertStore() {
   backend_->Close();
   // We release our reference to the Backend, though it will probably still have
   // a reference if the background thread has not run Close() yet.
 }
+
+}  // namespace net