DOMStorageContextImpl that's implemented in terms of the new dom_storage classes.

DOMStorageContextImpl that's implemented in terms of the new dom_storage classes. Also compile out existing tests that no longer apply when ENABLE_NEW_DOM_STORAGE_BACKEND is defined.

BUG=106763
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=127573

[michaeln, 2012-03-16 23:35:07]

ptal

[michaeln, 2012-03-16 23:38:39]

i have a couple of todo's to take care of in dom_storage_context_impl_new.cc and
the empty method bodies in dom_storage_context.cc to write, but otherwise i
think this is ready

[michaeln, 2012-03-17 00:53:54]

I've taken care of the todo's in dom_storage_context_impl_new.cc and think i
should hold off on changes to dom_storage_context.cc and put them in a different
CL.

So i think this is it for this CL, ptal.

[michaeln, 2012-03-17 03:52:22]

http://codereview.chromium.org/9695013/diff/21007/content/browser/browser_con...
File content/browser/browser_context.cc (right):

http://codereview.chromium.org/9695013/diff/21007/content/browser/browser_con...
content/browser/browser_context.cc:193:
GetDOMStorageContextImpl(browser_context)->SaveSessionState();
the gist of these changes is to remove the WEBKIT_DEPRECATED thread dependency,
now we just call the method and the impl class deal with threading issues

http://codereview.chromium.org/9695013/diff/21007/content/browser/browser_con...
content/browser/browser_context.cc:245:
GetDOMStorageContextImpl(this)->Shutdown();
adding an explicit shutdown method since relying on destruction of recounted
objects is squishy

(historical note: prior to content api machinations, DOMStorageContext[Impl]
employed delete semantics, but when delete semantics were change to refcounting
semantics nothing was done to tighten up the shutdown logic)

[michaeln, 2012-03-18 20:58:22]

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

https://chromiumcodereview.appspot.com/9695013/diff/17033/webkit/dom_storage/...
webkit/dom_storage/dom_storage_context.cc:60: void
DomStorageContext::GetUsageInfo(std::vector<UsageInfo>* infos) {
i've added an implementation of this method, but will leave the others for a
later CL (which should also contain much needed unittests)

[michaeln, 2012-03-19 00:36:28]

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

https://chromiumcodereview.appspot.com/9695013/diff/21014/webkit/dom_storage/...
webkit/dom_storage/dom_storage_area.cc:26: return
FilePath().AppendASCII(filename).ReplaceExtension(
Fun with FilePaths... there is no method to AddExtension afaict, this usage of
ReplaceExtension is destructive since the 'origin_id' string format includes '.'
characters which look like filename extensions. So of course... you start with
just the extension as the filename, and then InsertBefore it the filename. Makes
perfect sense.

[jsbell, 2012-03-19 20:24:29]

lgtm

https://chromiumcodereview.appspot.com/9695013/diff/17076/content/browser/dom...
File content/browser/dom_storage/dom_storage_context_impl_new.h (right):

https://chromiumcodereview.appspot.com/9695013/diff/17076/content/browser/dom...
content/browser/dom_storage/dom_storage_context_impl_new.h:9: #include <vector>
Is this include needed?

https://chromiumcodereview.appspot.com/9695013/diff/17076/content/browser/dom...
content/browser/dom_storage/dom_storage_context_impl_new.h:60: //
TODO(michaeln): Remove this method when that bug is fixed.
Is there a crbug for this, or will you fix before landing?

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

https://chromiumcodereview.appspot.com/9695013/diff/17076/webkit/dom_storage/...
webkit/dom_storage/dom_storage_area.cc:27: InsertBeforeExtensionASCII(filename);
As you note this is non-obvious; perhaps a comment: "filename looks like an
extension, so cannot use FilePath::ReplaceExtension"

... or just go ahead and add FilePath::AddExtension[ASCII]()

[michaeln, 2012-03-19 20:49:58]

thnx

https://chromiumcodereview.appspot.com/9695013/diff/17076/content/browser/dom...
File content/browser/dom_storage/dom_storage_context_impl_new.h (right):

https://chromiumcodereview.appspot.com/9695013/diff/17076/content/browser/dom...
content/browser/dom_storage/dom_storage_context_impl_new.h:9: #include <vector>
On 2012/03/19 20:24:29, jsbell wrote:
> Is this include needed?

Done, dont think it is

https://chromiumcodereview.appspot.com/9695013/diff/17076/content/browser/dom...
content/browser/dom_storage/dom_storage_context_impl_new.h:60: //
TODO(michaeln): Remove this method when that bug is fixed.
On 2012/03/19 20:24:29, jsbell wrote:
> Is there a crbug for this, or will you fix before landing?

there is no crbug for this afaik, i'll file a bug as i have no plans to fix that
before landing

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

https://chromiumcodereview.appspot.com/9695013/diff/17076/webkit/dom_storage/...
webkit/dom_storage/dom_storage_area.cc:27: InsertBeforeExtensionASCII(filename);
On 2012/03/19 20:24:29, jsbell wrote:
> As you note this is non-obvious; perhaps a comment: "filename looks like an
> extension, so cannot use FilePath::ReplaceExtension"
> 
> ... or just go ahead and add FilePath::AddExtension[ASCII]()

Added a comment since i don't care to change the base library now and lengthen
the reveiw cycle for this minor detail.