DomStorageArea -> DomStorageDatabase indirection; add *StorageDatabaseAdapter.

DomStorageArea -> DomStorageDatabase indirection; add *StorageDatabaseAdapter.

(Dom|Local|Session)StorageDatabaseAdapter add an extra layer which unifies handling
the backends for localStorage and sessionStorage.

This will be useful when backing sessionStorage on disk.

BUG=104292
TEST=Updated DomStorageAreaTest.*


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=143173

[marja, 2012-06-15 13:24:52]

Hi michaeln, this is another spin off of http://codereview.chromium.org/9963107/
, ptal.

[michaeln, 2012-06-15 19:02:43]

just some nits, i see you'll have to rebase/merge with the other change

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
File webkit/dom_storage/dom_storage_area.cc (right):

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
webkit/dom_storage/dom_storage_area.cc:17: #include
"webkit/dom_storage/local_storage_database_adapter.h"
depending on what happens in the .h file, this may not be needed

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
File webkit/dom_storage/dom_storage_area.h (right):

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
webkit/dom_storage/dom_storage_area.h:21: class DomStorageDatabaseAdapter;
either the #include or this forward isn't really needed

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
File webkit/dom_storage/dom_storage_context.cc (right):

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
webkit/dom_storage/dom_storage_context.cc:13: #include
"webkit/dom_storage/dom_storage_database.h"
is this needed?

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
File webkit/dom_storage/dom_storage_database_adapter.h (right):

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
webkit/dom_storage/dom_storage_database_adapter.h:19:
DomStorageDatabaseAdapter();
is this ctor needed?

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
webkit/dom_storage/dom_storage_database_adapter.h:20: virtual
~DomStorageDatabaseAdapter();
would an emtpy method body work here? if so we may not need the .cc file at all
for this interface.

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
webkit/dom_storage/dom_storage_database_adapter.h:24: virtual void DeleteFiles()
{ }
nit: no space required in {}

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
webkit/dom_storage/dom_storage_database_adapter.h:27:
DISALLOW_COPY_AND_ASSIGN(DomStorageDatabaseAdapter);
is this meaningful on an abstract class?

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
File webkit/dom_storage/local_storage_database_adapter.h (right):

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
webkit/dom_storage/local_storage_database_adapter.h:27: virtual void Reset()
OVERRIDE;
nit: add a blank line here?

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
File webkit/dom_storage/session_storage_database_adapter.h (right):

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
webkit/dom_storage/session_storage_database_adapter.h:20: const std::string&
namespace_id,
might be nice to name this param and the data member as persistent_namespace_id

[marja, 2012-06-19 10:43:30]

Thanks for comments!

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
File webkit/dom_storage/dom_storage_area.cc (right):

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
webkit/dom_storage/dom_storage_area.cc:17: #include
"webkit/dom_storage/local_storage_database_adapter.h"
On 2012/06/15 19:02:43, michaeln wrote:
> depending on what happens in the .h file, this may not be needed

Include removed from the .h -> kept this.

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
File webkit/dom_storage/dom_storage_area.h (right):

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
webkit/dom_storage/dom_storage_area.h:21: class DomStorageDatabaseAdapter;
On 2012/06/15 19:02:43, michaeln wrote:
> either the #include or this forward isn't really needed

Done (forward declaration is enough).

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
File webkit/dom_storage/dom_storage_context.cc (right):

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
webkit/dom_storage/dom_storage_context.cc:13: #include
"webkit/dom_storage/dom_storage_database.h"
On 2012/06/15 19:02:43, michaeln wrote:
> is this needed?

Yes, this uses DomStorageDatabase::GetJournalFilePath. It used to get the
include indirectly but that path no longer exists.

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
File webkit/dom_storage/dom_storage_database_adapter.h (right):

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
webkit/dom_storage/dom_storage_database_adapter.h:19:
DomStorageDatabaseAdapter();
On 2012/06/15 19:02:43, michaeln wrote:
> is this ctor needed?

No if DISALLOW_.. is removed. Removed the DISALLOW_.., removed this.

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
webkit/dom_storage/dom_storage_database_adapter.h:20: virtual
~DomStorageDatabaseAdapter();
On 2012/06/15 19:02:43, michaeln wrote:
> would an emtpy method body work here? if so we may not need the .cc file at
all
> for this interface.

Done.

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
webkit/dom_storage/dom_storage_database_adapter.h:24: virtual void DeleteFiles()
{ }
On 2012/06/15 19:02:43, michaeln wrote:
> nit: no space required in {}

Done.

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
webkit/dom_storage/dom_storage_database_adapter.h:27:
DISALLOW_COPY_AND_ASSIGN(DomStorageDatabaseAdapter);
On 2012/06/15 19:02:43, michaeln wrote:
> is this meaningful on an abstract class?

Removed.

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
File webkit/dom_storage/local_storage_database_adapter.h (right):

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
webkit/dom_storage/local_storage_database_adapter.h:27: virtual void Reset()
OVERRIDE;
On 2012/06/15 19:02:43, michaeln wrote:
> nit: add a blank line here?

Done.

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
File webkit/dom_storage/session_storage_database_adapter.h (right):

https://chromiumcodereview.appspot.com/10556009/diff/6001/webkit/dom_storage/...
webkit/dom_storage/session_storage_database_adapter.h:20: const std::string&
namespace_id,
On 2012/06/15 19:02:43, michaeln wrote:
> might be nice to name this param and the data member as
persistent_namespace_id 

Done.

[michaeln, 2012-06-20 03:25:14]

lgtm

[commit-bot: I haz the power, 2012-06-20 12:53:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marja@chromium.org/10556009/14001

[commit-bot: I haz the power, 2012-06-20 14:46:28]

Change committed as 143173