diff options
9 files changed, 195 insertions, 54 deletions
diff --git a/chrome/browser/in_process_webkit/dom_storage_context.cc b/chrome/browser/in_process_webkit/dom_storage_context.cc index 9f763e2..821f798 100644 --- a/chrome/browser/in_process_webkit/dom_storage_context.cc +++ b/chrome/browser/in_process_webkit/dom_storage_context.cc @@ -119,3 +119,14 @@ DOMStorageContext::GetDispatcherHostSet() const { DCHECK(ChromeThread::CurrentlyOn(ChromeThread::IO)); return &dispatcher_host_set_; } + +void DOMStorageContext::PurgeMemory() { + // It is only safe to purge the memory from the LocalStorage namespace, + // because it is backed by disk and can be reloaded later. If we purge a + // SessionStorage namespace, its data will be gone forever, because it isn't + // currently backed by disk. + StorageNamespace* local_storage = + GetStorageNamespace(kLocalStorageNamespaceId); + if (local_storage) + local_storage->PurgeMemory(); +} diff --git a/chrome/browser/in_process_webkit/dom_storage_context.h b/chrome/browser/in_process_webkit/dom_storage_context.h index 5e85876..e652aa6 100644 --- a/chrome/browser/in_process_webkit/dom_storage_context.h +++ b/chrome/browser/in_process_webkit/dom_storage_context.h @@ -5,10 +5,12 @@ #ifndef CHROME_BROWSER_IN_PROCESS_WEBKIT_DOM_STORAGE_CONTEXT_H_ #define CHROME_BROWSER_IN_PROCESS_WEBKIT_DOM_STORAGE_CONTEXT_H_ +#include <map> +#include <set> + #include "base/file_path.h" -#include "base/hash_tables.h" -#include "chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h" +class DOMStorageDispatcherHost; class StorageArea; class StorageNamespace; class WebKitContext; @@ -18,10 +20,12 @@ class WebKitContext; // the same profile. The specifics of responsibilities are fairly well // documented here and in StorageNamespace and StorageArea. Everything is only // to be accessed on the WebKit thread unless noted otherwise. +// +// NOTE: Virtual methods facilitate mocking functions for testing. class DOMStorageContext { public: explicit DOMStorageContext(WebKitContext* webkit_context); - ~DOMStorageContext(); + virtual ~DOMStorageContext(); // Get the local storage instance. The pointer is owned by this class. StorageNamespace* LocalStorage(); @@ -48,11 +52,14 @@ class DOMStorageContext { // Sometimes an event from one DOM storage dispatcher host requires // communication to all of them. - typedef base::hash_set<DOMStorageDispatcherHost*> DispatcherHostSet; + typedef std::set<DOMStorageDispatcherHost*> DispatcherHostSet; void RegisterDispatcherHost(DOMStorageDispatcherHost* dispatcher_host); void UnregisterDispatcherHost(DOMStorageDispatcherHost* dispatcher_host); const DispatcherHostSet* GetDispatcherHostSet() const; + // Tells storage namespaces to purge any memory they do not need. + virtual void PurgeMemory(); + // The special ID used for local storage. static const int64 kLocalStorageNamespaceId = 0; @@ -72,11 +79,11 @@ class DOMStorageContext { // Maps ids to StorageAreas. We do NOT own these objects. StorageNamespace // (which does own them) will notify us when we should remove the entries. - typedef base::hash_map<int64, StorageArea*> StorageAreaMap; + typedef std::map<int64, StorageArea*> StorageAreaMap; StorageAreaMap storage_area_map_; // Maps ids to StorageNamespaces. We own these objects. - typedef base::hash_map<int64, StorageNamespace*> StorageNamespaceMap; + typedef std::map<int64, StorageNamespace*> StorageNamespaceMap; StorageNamespaceMap storage_namespace_map_; DISALLOW_IMPLICIT_CONSTRUCTORS(DOMStorageContext); diff --git a/chrome/browser/in_process_webkit/storage_area.cc b/chrome/browser/in_process_webkit/storage_area.cc index 885b0b15..603ead3 100644 --- a/chrome/browser/in_process_webkit/storage_area.cc +++ b/chrome/browser/in_process_webkit/storage_area.cc @@ -5,43 +5,60 @@ #include "chrome/browser/in_process_webkit/storage_area.h" #include "chrome/browser/in_process_webkit/dom_storage_dispatcher_host.h" +#include "chrome/browser/in_process_webkit/storage_namespace.h" #include "webkit/api/public/WebStorageArea.h" #include "webkit/api/public/WebString.h" using WebKit::WebStorageArea; -StorageArea::StorageArea(const string16& origin, WebStorageArea* storage_area, - int64 id) +StorageArea::StorageArea(const string16& origin, + int64 id, + StorageNamespace* owner) : origin_(origin), - storage_area_(storage_area), - id_(id) { - DCHECK(storage_area_.get()); + id_(id), + owner_(owner) { + DCHECK(owner_); } StorageArea::~StorageArea() { } unsigned StorageArea::Length() { + CreateWebStorageAreaIfNecessary(); return storage_area_->length(); } NullableString16 StorageArea::Key(unsigned index) { + CreateWebStorageAreaIfNecessary(); return storage_area_->key(index); } NullableString16 StorageArea::GetItem(const string16& key) { + CreateWebStorageAreaIfNecessary(); return storage_area_->getItem(key); } void StorageArea::SetItem(const string16& key, const string16& value, bool* quota_exception) { + CreateWebStorageAreaIfNecessary(); storage_area_->setItem(key, value, *quota_exception); } void StorageArea::RemoveItem(const string16& key) { + CreateWebStorageAreaIfNecessary(); storage_area_->removeItem(key); } void StorageArea::Clear() { + CreateWebStorageAreaIfNecessary(); storage_area_->clear(); } + +void StorageArea::PurgeMemory() { + storage_area_.reset(); +} + +void StorageArea::CreateWebStorageAreaIfNecessary() { + if (!storage_area_.get()) + storage_area_.reset(owner_->CreateWebStorageArea(origin_)); +} diff --git a/chrome/browser/in_process_webkit/storage_area.h b/chrome/browser/in_process_webkit/storage_area.h index 7aaf73a..7875137 100644 --- a/chrome/browser/in_process_webkit/storage_area.h +++ b/chrome/browser/in_process_webkit/storage_area.h @@ -9,6 +9,8 @@ #include "base/nullable_string16.h" #include "base/scoped_ptr.h" +class StorageNamespace; + namespace WebKit { class WebStorageArea; } @@ -17,8 +19,7 @@ class WebStorageArea; // with DOMStorageContext. class StorageArea { public: - StorageArea(const string16& origin, WebKit::WebStorageArea* storage_area, - int64 id); + StorageArea(const string16& origin, int64 id, StorageNamespace* owner); ~StorageArea(); unsigned Length(); @@ -28,10 +29,14 @@ class StorageArea { bool* quota_xception); void RemoveItem(const string16& key); void Clear(); + void PurgeMemory(); int64 id() const { return id_; } private: + // Creates the underlying WebStorageArea on demand. + void CreateWebStorageAreaIfNecessary(); + // The origin this storage area represents. string16 origin_; @@ -41,6 +46,9 @@ class StorageArea { // Our storage area id. Unique to our parent WebKitContext. int64 id_; + // The StorageNamespace that owns us. + StorageNamespace* owner_; + DISALLOW_IMPLICIT_CONSTRUCTORS(StorageArea); }; diff --git a/chrome/browser/in_process_webkit/storage_namespace.cc b/chrome/browser/in_process_webkit/storage_namespace.cc index 7bab75f..ce24700 100644 --- a/chrome/browser/in_process_webkit/storage_namespace.cc +++ b/chrome/browser/in_process_webkit/storage_namespace.cc @@ -10,7 +10,6 @@ #include "chrome/browser/in_process_webkit/storage_area.h" #include "webkit/api/public/WebStorageArea.h" #include "webkit/api/public/WebStorageNamespace.h" -#include "webkit/api/public/WebString.h" #include "webkit/glue/webkit_glue.h" using WebKit::WebStorageArea; @@ -19,15 +18,11 @@ using WebKit::WebString; /* static */ StorageNamespace* StorageNamespace::CreateLocalStorageNamespace( - DOMStorageContext* dom_storage_context, const FilePath& dir_path) { + DOMStorageContext* dom_storage_context, const FilePath& data_dir_path) { int64 id = dom_storage_context->kLocalStorageNamespaceId; DCHECK(!dom_storage_context->GetStorageNamespace(id)); - WebString path = webkit_glue::FilePathToWebString(dir_path); - WebStorageNamespace* web_storage_namespace = - WebStorageNamespace::createLocalStorageNamespace(path, - kLocalStorageQuota); - return new StorageNamespace(dom_storage_context, web_storage_namespace, id, - DOM_STORAGE_LOCAL); + return new StorageNamespace(dom_storage_context, id, + webkit_glue::FilePathToWebString(data_dir_path), DOM_STORAGE_LOCAL); } /* static */ @@ -35,33 +30,29 @@ StorageNamespace* StorageNamespace::CreateSessionStorageNamespace( DOMStorageContext* dom_storage_context) { int64 id = dom_storage_context->AllocateStorageNamespaceId(); DCHECK(!dom_storage_context->GetStorageNamespace(id)); - WebStorageNamespace* web_storage_namespace = - WebStorageNamespace::createSessionStorageNamespace(); - return new StorageNamespace(dom_storage_context, web_storage_namespace, id, + return new StorageNamespace(dom_storage_context, id, WebString(), DOM_STORAGE_SESSION); } StorageNamespace::StorageNamespace(DOMStorageContext* dom_storage_context, - WebStorageNamespace* storage_namespace, - int64 id, DOMStorageType dom_storage_type) + int64 id, + const WebString& data_dir_path, + DOMStorageType dom_storage_type) : dom_storage_context_(dom_storage_context), - storage_namespace_(storage_namespace), id_(id), + data_dir_path_(data_dir_path), dom_storage_type_(dom_storage_type) { DCHECK(dom_storage_context_); - DCHECK(storage_namespace_.get()); dom_storage_context_->RegisterStorageNamespace(this); } StorageNamespace::~StorageNamespace() { dom_storage_context_->UnregisterStorageNamespace(this); - OriginToStorageAreaMap::iterator iter = origin_to_storage_area_.begin(); - OriginToStorageAreaMap::iterator end = origin_to_storage_area_.end(); - while (iter != end) { + for (OriginToStorageAreaMap::iterator iter(origin_to_storage_area_.begin()); + iter != origin_to_storage_area_.end(); ++iter) { dom_storage_context_->UnregisterStorageArea(iter->second); delete iter->second; - ++iter; } } @@ -74,9 +65,7 @@ StorageArea* StorageNamespace::GetStorageArea(const string16& origin) { // We need to create a new one. int64 id = dom_storage_context_->AllocateStorageAreaId(); DCHECK(!dom_storage_context_->GetStorageArea(id)); - WebStorageArea* web_storage_area = - storage_namespace_->createStorageArea(origin); - StorageArea* storage_area = new StorageArea(origin, web_storage_area, id); + StorageArea* storage_area = new StorageArea(origin, id, this); origin_to_storage_area_[origin] = storage_area; dom_storage_context_->RegisterStorageArea(storage_area); return storage_area; @@ -86,11 +75,35 @@ StorageNamespace* StorageNamespace::Copy() { DCHECK(dom_storage_type_ == DOM_STORAGE_SESSION); int64 id = dom_storage_context_->AllocateStorageNamespaceId(); DCHECK(!dom_storage_context_->GetStorageNamespace(id)); - WebStorageNamespace* new_storage_namespace = storage_namespace_->copy(); - return new StorageNamespace(dom_storage_context_, new_storage_namespace, id, - dom_storage_type_); + StorageNamespace* new_storage_namespace = new StorageNamespace( + dom_storage_context_, id, data_dir_path_, dom_storage_type_); + CreateWebStorageNamespaceIfNecessary(); + new_storage_namespace->storage_namespace_.reset(storage_namespace_->copy()); + return new_storage_namespace; } -void StorageNamespace::Close() { - storage_namespace_->close(); +void StorageNamespace::PurgeMemory() { + for (OriginToStorageAreaMap::iterator iter(origin_to_storage_area_.begin()); + iter != origin_to_storage_area_.end(); ++iter) + iter->second->PurgeMemory(); + storage_namespace_.reset(); +} + +WebStorageArea* StorageNamespace::CreateWebStorageArea(const string16& origin) { + CreateWebStorageNamespaceIfNecessary(); + return storage_namespace_->createStorageArea(origin); +} + +void StorageNamespace::CreateWebStorageNamespaceIfNecessary() { + if (storage_namespace_.get()) + return; + + if (dom_storage_type_ == DOM_STORAGE_LOCAL) { + storage_namespace_.reset( + WebStorageNamespace::createLocalStorageNamespace(data_dir_path_, + kLocalStorageQuota); + } else { + storage_namespace_.reset( + WebStorageNamespace::createSessionStorageNamespace()); + } } diff --git a/chrome/browser/in_process_webkit/storage_namespace.h b/chrome/browser/in_process_webkit/storage_namespace.h index 1a9ea54..9ee2c2b 100644 --- a/chrome/browser/in_process_webkit/storage_namespace.h +++ b/chrome/browser/in_process_webkit/storage_namespace.h @@ -9,12 +9,14 @@ #include "base/hash_tables.h" #include "base/scoped_ptr.h" #include "chrome/common/dom_storage_type.h" +#include "webkit/api/public/WebString.h" class DOMStorageContext; class FilePath; class StorageArea; namespace WebKit { +class WebStorageArea; class WebStorageNamespace; } @@ -30,7 +32,7 @@ class StorageNamespace { StorageArea* GetStorageArea(const string16& origin); StorageNamespace* Copy(); - void Close(); + void PurgeMemory(); const DOMStorageContext* dom_storage_context() const { return dom_storage_context_; @@ -38,11 +40,19 @@ class StorageNamespace { int64 id() const { return id_; } DOMStorageType dom_storage_type() const { return dom_storage_type_; } + // Creates a WebStorageArea for the given origin. This should only be called + // by an owned StorageArea. + WebKit::WebStorageArea* CreateWebStorageArea(const string16& origin); + private: // Called by the static factory methods above. StorageNamespace(DOMStorageContext* dom_storage_context, - WebKit::WebStorageNamespace* web_storage_namespace, - int64 id, DOMStorageType storage_type); + int64 id, + const WebKit::WebString& data_dir_path, + DOMStorageType storage_type); + + // Creates the underlying WebStorageNamespace on demand. + void CreateWebStorageNamespaceIfNecessary(); // All the storage areas we own. typedef base::hash_map<string16, StorageArea*> OriginToStorageAreaMap; @@ -57,6 +67,10 @@ class StorageNamespace { // Our id. Unique to our parent WebKitContext class. int64 id_; + // The path used to create us, so we can recreate our WebStorageNamespace on + // demand. + WebKit::WebString data_dir_path_; + // SessionStorage vs. LocalStorage. const DOMStorageType dom_storage_type_; diff --git a/chrome/browser/in_process_webkit/webkit_context.cc b/chrome/browser/in_process_webkit/webkit_context.cc index 0768284..a46d53f 100644 --- a/chrome/browser/in_process_webkit/webkit_context.cc +++ b/chrome/browser/in_process_webkit/webkit_context.cc @@ -5,7 +5,6 @@ #include "chrome/browser/in_process_webkit/webkit_context.h" #include "chrome/browser/chrome_thread.h" -#include "chrome/browser/in_process_webkit/dom_storage_context.h" WebKitContext::WebKitContext(const FilePath& data_path, bool is_incognito) : data_path_(data_path), @@ -23,3 +22,24 @@ WebKitContext::~WebKitContext() { if (webkit_loop) webkit_loop->DeleteSoon(FROM_HERE, dom_storage_context_.release()); } + +void WebKitContext::PurgeMemory() { + // DOMStorageContext::PurgeMemory() should only be called on the WebKit + // thread. + // + // Note that if there is no WebKit thread, then there's nothing in + // LocalStorage and it's OK to no-op here. Further note that in a unittest, + // there may be no threads at all, in which case MessageLoop::current() will + // also be NULL and we'll go ahead and call PurgeMemory() directly, which is + // probably what the test wants. + MessageLoop* webkit_loop = ChromeThread::GetMessageLoop(ChromeThread::WEBKIT); + if (MessageLoop::current() == webkit_loop) { + dom_storage_context_->PurgeMemory(); + } else if (webkit_loop) { + // Since we're not on the WebKit thread, proxy the call over to it. We + // can't post a task to call DOMStorageContext::PurgeMemory() directly + // because that class is not refcounted. + webkit_loop->PostTask(FROM_HERE, + NewRunnableMethod(this, &WebKitContext::PurgeMemory)); + } +} diff --git a/chrome/browser/in_process_webkit/webkit_context.h b/chrome/browser/in_process_webkit/webkit_context.h index b359ab0..d83d530 100644 --- a/chrome/browser/in_process_webkit/webkit_context.h +++ b/chrome/browser/in_process_webkit/webkit_context.h @@ -8,12 +8,15 @@ #include "base/file_path.h" #include "base/ref_counted.h" #include "base/scoped_ptr.h" +#include "chrome/browser/in_process_webkit/dom_storage_context.h" -class DOMStorageContext; class WebKitThread; // There's one WebKitContext per profile. Various DispatcherHost classes // have a pointer to the Context to store shared state. +// +// This class is created on the UI thread and accessed on the UI, IO, and WebKit +// threads. class WebKitContext : public base::RefCountedThreadSafe<WebKitContext> { public: WebKitContext(const FilePath& data_path, bool is_incognito); @@ -24,6 +27,17 @@ class WebKitContext : public base::RefCountedThreadSafe<WebKitContext> { return dom_storage_context_.get(); } +#ifdef UNIT_TEST + // For unit tests, allow specifying a DOMStorageContext directly so it can be + // mocked. + void set_dom_storage_context(DOMStorageContext* dom_storage_context) { + dom_storage_context_.reset(dom_storage_context); + } +#endif + + // Tells the DOMStorageContext to purge any memory it does not need. + void PurgeMemory(); + private: friend class base::RefCountedThreadSafe<WebKitContext>; ~WebKitContext(); diff --git a/chrome/browser/in_process_webkit/webkit_context_unittest.cc b/chrome/browser/in_process_webkit/webkit_context_unittest.cc index e5f446d..98ccfd0 100644 --- a/chrome/browser/in_process_webkit/webkit_context_unittest.cc +++ b/chrome/browser/in_process_webkit/webkit_context_unittest.cc @@ -2,19 +2,56 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "chrome/browser/chrome_thread.h" +#include "chrome/browser/in_process_webkit/dom_storage_context.h" #include "chrome/browser/in_process_webkit/webkit_context.h" #include "testing/gtest/include/gtest/gtest.h" -TEST(WebKitContextTest, BasicsTest1) { +class MockDOMStorageContext : public DOMStorageContext { + public: + explicit MockDOMStorageContext(WebKitContext* webkit_context) + : DOMStorageContext(webkit_context), + purge_count_(0) { + } + + virtual void PurgeMemory() { + EXPECT_FALSE(ChromeThread::CurrentlyOn(ChromeThread::UI)); + EXPECT_TRUE(ChromeThread::CurrentlyOn(ChromeThread::WEBKIT)); + ++purge_count_; + } + + int purge_count() const { return purge_count_; } + + private: + int purge_count_; +}; + +TEST(WebKitContextTest, Basic) { FilePath file_path; - scoped_refptr<WebKitContext> context = new WebKitContext(file_path, true); - ASSERT_TRUE(file_path == context->data_path()); - ASSERT_EQ(true, context->is_incognito()); + scoped_refptr<WebKitContext> context1(new WebKitContext(file_path, true)); + EXPECT_TRUE(file_path == context1->data_path()); + EXPECT_TRUE(context1->is_incognito()); + + scoped_refptr<WebKitContext> context2(new WebKitContext(file_path, false)); + EXPECT_TRUE(file_path == context2->data_path()); + EXPECT_FALSE(context2->is_incognito()); } -TEST(WebKitContextTest, BasicsTest2) { - FilePath file_path; - scoped_refptr<WebKitContext> context = new WebKitContext(file_path, false); - ASSERT_TRUE(file_path == context->data_path()); - ASSERT_EQ(false, context->is_incognito()); +TEST(WebKitContextTest, PurgeMemory) { + // Start up a WebKit thread for the WebKitContext to call the + // DOMStorageContext on. + ChromeThread webkit_thread(ChromeThread::WEBKIT); + webkit_thread.Start(); + + // Create the contexts. + scoped_refptr<WebKitContext> context(new WebKitContext(FilePath(), false)); + MockDOMStorageContext* mock_context = + new MockDOMStorageContext(context.get()); + context->set_dom_storage_context(mock_context); // Takes ownership. + + // Ensure PurgeMemory() calls our mock object on the right thread. + EXPECT_EQ(0, mock_context->purge_count()); + context->PurgeMemory(); + webkit_thread.Stop(); // Blocks until all tasks are complete. + EXPECT_EQ(1, mock_context->purge_count()); } |