Clear up session only storage on localstorage shutdown

content/browser/dom_storage/local_storage_context_mojo.h

Index: content/browser/dom_storage/local_storage_context_mojo.h
diff --git a/content/browser/dom_storage/local_storage_context_mojo.h b/content/browser/dom_storage/local_storage_context_mojo.h
index 9500eb199b8d6a2b86841424bfadc4e3a7f062de..7d372c1c25f25b904151fe1beaeec1369335c82f 100644
--- a/content/browser/dom_storage/local_storage_context_mojo.h
+++ b/content/browser/dom_storage/local_storage_context_mojo.h
@@ -10,6 +10,7 @@
 #include "base/files/file_path.h"
 #include "content/common/content_export.h"
 #include "content/common/leveldb_wrapper.mojom.h"
+#include "content/public/browser/browser_thread.h"
 #include "services/file/public/interfaces/file_system.mojom.h"
 #include "url/origin.h"
 
@@ -17,6 +18,10 @@ namespace service_manager {
 class Connector;
 }
 
+namespace storage {
+class SpecialStoragePolicy;
+}
+
 namespace content {
 
 class DOMStorageTaskRunner;
@@ -25,16 +30,18 @@ struct LocalStorageUsageInfo;
 
 // Used for mojo-based LocalStorage implementation (behind --mojo-local-storage
 // for now).

[michaeln, 2017/05/03 21:01:29]

Describe the thread access pattern in the class comment, created on main thread
but used exclusively on the IO thread.

[Marijn Kruisselbrink, 2017/05/04 23:03:46]

Done

-class CONTENT_EXPORT LocalStorageContextMojo {
+class CONTENT_EXPORT LocalStorageContextMojo
+    : public base::RefCountedThreadSafe<LocalStorageContextMojo> {

[michaeln, 2017/05/03 21:01:29]

I'd rather avoid refcounting if we can, but i see Shutdown() is async so it's
difficult to avoid w/o something odd like making Shutdown() delete itself upon
completion. That might not be awful because the only way to delete the object
would be to go thru Shutdown() which we don't want to skip, but it's a little
odd? If you were to go that route, call it something odd like
ShutdownAndDelete()

[Marijn Kruisselbrink, 2017/05/04 23:03:46]

Went that route. Seems reasonable, but somewhat unfortunate that now it's easier
to forget to actually shutdown and delete the class. Could be solved by wrapping
LocalStorageContextMojo instances in a unique_ptr with custom deleter or
something, but that also would be tricky because of the need for the deleter to
actually post a task to the correct task runner to delete the instance. Anyway
outside of test code DOMStorageContextWrapper is the only place that has an
instance of this class, and I did add a DCHECK there to make sure it always
shuts down this class before the wrapper is destructed.

  public:
   using GetStorageUsageCallback =
       base::OnceCallback<void(std::vector<LocalStorageUsageInfo>)>;
 
-  LocalStorageContextMojo(service_manager::Connector* connector,
-                          scoped_refptr<DOMStorageTaskRunner> task_runner,
-                          const base::FilePath& old_localstorage_path,
-                          const base::FilePath& subdirectory);
-  ~LocalStorageContextMojo();
+  LocalStorageContextMojo(
+      service_manager::Connector* connector,
+      scoped_refptr<DOMStorageTaskRunner> task_runner,
+      const base::FilePath& old_localstorage_path,
+      const base::FilePath& subdirectory,
+      storage::SpecialStoragePolicy* special_storage_policy);
 
   void OpenLocalStorage(const url::Origin& origin,
                         mojom::LevelDBWrapperRequest request);
@@ -44,6 +51,18 @@ class CONTENT_EXPORT LocalStorageContextMojo {
   void DeleteStorageForPhysicalOrigin(const url::Origin& origin);
   void Flush();
 
+  // Used by content settings to alter the behavior around
+  // what data to keep and what data to discard at shutdown.
+  // The policy is not so straight forward to describe, see
+  // the implementation for details.
+  void SetForceKeepSessionState() { force_keep_session_state_ = true; }
+
+  // Called when the owning BrowserContext is ending.
+  // Schedules the commit of any unsaved changes and will delete
+  // and keep data on disk per the content settings and special storage
+  // policies.
+  void Shutdown();
+
   // Clears any caches, to free up as much memory as possible. Next access to
   // storage for a particular origin will reload the data from the database.
   void PurgeMemory();
@@ -58,6 +77,9 @@ class CONTENT_EXPORT LocalStorageContextMojo {
 
   class LevelDBWrapperHolder;
 
+  friend class base::RefCountedThreadSafe<LocalStorageContextMojo>;
+  ~LocalStorageContextMojo();
+
   // Runs |callback| immediately if already connected to a database, otherwise
   // delays running |callback| untill after a connection has been established.
   // Initiates connecting to the database if no connection is in progres yet.
@@ -91,16 +113,24 @@ class CONTENT_EXPORT LocalStorageContextMojo {
       const url::Origin& origin,
       std::vector<LocalStorageUsageInfo> usage);
 
-  service_manager::Connector* const connector_;
+  void OnGotStorageUsageForShutdown(std::vector<LocalStorageUsageInfo> usage);
+  void OnShutdownCommitComplete(leveldb::mojom::DatabaseError error);
+
+  std::unique_ptr<service_manager::Connector> connector_;
   const base::FilePath subdirectory_;
 
   enum ConnectionState {
     NO_CONNECTION,
     CONNECTION_IN_PROGRESS,
-    CONNECTION_FINISHED
+    CONNECTION_FINISHED,
+    CONNECTION_SHUTDOWN
   } connection_state_ = NO_CONNECTION;
   bool database_initialized_ = false;
 
+  bool is_shutdown_ = false;
+  bool force_keep_session_state_ = false;
+  scoped_refptr<storage::SpecialStoragePolicy> special_storage_policy_;
+
   file::mojom::FileSystemPtr file_system_;
   filesystem::mojom::DirectoryPtr directory_;