Don't hardcode the "Local Storage" directory name in DomStorageContext.

DomStorageContext: separate directories for localStorage and sessionStorage.

This is needed to eventually store sessionStorage on disk.

BUG=104292
TEST=NONE


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

[marja, 2012-04-11 12:07:21]

Hi Michael, I created this CL to be able to add a directory name for the
file-backed sessionStorage later. Ptal.

[michaeln, 2012-04-11 20:35:53]

https://chromiumcodereview.appspot.com/9999021/diff/11002/content/browser/dom...
File content/browser/dom_storage/dom_storage_context_impl_new.cc (right):

https://chromiumcodereview.appspot.com/9999021/diff/11002/content/browser/dom...
content/browser/dom_storage/dom_storage_context_impl_new.cc:85: data_path,
Another option is to give the dom_storage class two paths, one for the
local_storage_directory and another for the session_storage_directory. This
could be nice because then the caller could independently configure locations
(including an empty path for one and not the other).

2 paths might be a cleaner interface while bringing it up too... we can pass an
empty FilePath() here in the content layer for session_dir until its really
ready to be turned on... but still have unit tests that don't pass an empty
path.

wdyt?

[marja, 2012-04-12 14:58:11]

https://chromiumcodereview.appspot.com/9999021/diff/11002/content/browser/dom...
File content/browser/dom_storage/dom_storage_context_impl_new.cc (right):

https://chromiumcodereview.appspot.com/9999021/diff/11002/content/browser/dom...
content/browser/dom_storage/dom_storage_context_impl_new.cc:85: data_path,
On 2012/04/11 20:35:53, michaeln wrote:
> Another option is to give the dom_storage class two paths, one for the
> local_storage_directory and another for the session_storage_directory. This
> could be nice because then the caller could independently configure locations
> (including an empty path for one and not the other).
> 
> 2 paths might be a cleaner interface while bringing it up too... we can pass
an
> empty FilePath() here in the content layer for session_dir until its really
> ready to be turned on... but still have unit tests that don't pass an empty
> path.
> 
> wdyt?

Alright! Done.

[michaeln, 2012-04-12 18:35:38]

lgtm, thnx!

https://chromiumcodereview.appspot.com/9999021/diff/15002/webkit/dom_storage/...
File webkit/dom_storage/dom_storage_context.h (right):

https://chromiumcodereview.appspot.com/9999021/diff/15002/webkit/dom_storage/...
webkit/dom_storage/dom_storage_context.h:103: FilePath
sessionstorage_directory() { return sessionstorage_directory_; }
can these be const FilePath& return values instead?

[commit-bot: I haz the power, 2012-04-13 07:47:59]

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

[commit-bot: I haz the power, 2012-04-13 07:48:02]

Presubmit check for 9999021-19001 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    webkit

Presubmit checks took 1.0s to calculate.

Was the presubmit check useful? Please send feedback & hate mail to
maruel@chromium.org!

[marja, 2012-04-13 07:55:28]

Oops, missing OWNERS.

tony, could you review the tiny change in
webkit/tools/test_shell/simple_dom_storage_system.cc ?

(michaeln has already reviewed the parts that are under webkit/dom_storage.)

[marja, 2012-04-13 07:56:09]

https://chromiumcodereview.appspot.com/9999021/diff/15002/webkit/dom_storage/...
File webkit/dom_storage/dom_storage_context.h (right):

https://chromiumcodereview.appspot.com/9999021/diff/15002/webkit/dom_storage/...
webkit/dom_storage/dom_storage_context.h:103: FilePath
sessionstorage_directory() { return sessionstorage_directory_; }
On 2012/04/12 18:35:38, michaeln wrote:
> can these be const FilePath& return values instead?

Afaics yes. Done.

[marja, 2012-04-16 15:12:43]

Oops2, tony seems to be on vacation. jamesr, could you review the tiny change in
webkit/tools/test_shell/simple_dom_storage_system.cc ?

(michaeln has already reviewed the parts that are under webkit/dom_storage.)

[jamesr, 2012-04-16 17:57:02]

lgtm

[commit-bot: I haz the power, 2012-04-17 07:11:20]

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

[commit-bot: I haz the power, 2012-04-17 07:11:23]

Can't apply patch for file
content/browser/dom_storage/dom_storage_context_impl_new.cc.
While running patch -p1 --forward --force;
patching file content/browser/dom_storage/dom_storage_context_impl_new.cc
Hunk #1 FAILED at 67.
Hunk #2 FAILED at 82.
Hunk #3 FAILED at 109.
3 out of 3 hunks FAILED -- saving rejects to file
content/browser/dom_storage/dom_storage_context_impl_new.cc.rej

[commit-bot: I haz the power, 2012-04-17 07:37:29]

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

[commit-bot: I haz the power, 2012-04-17 07:37:31]

Can't apply patch for file
content/browser/dom_storage/dom_storage_context_impl_new.cc.
While running patch -p1 --forward --force;
patching file content/browser/dom_storage/dom_storage_context_impl_new.cc
Hunk #1 FAILED at 67.
Hunk #2 FAILED at 82.
Hunk #3 FAILED at 109.
3 out of 3 hunks FAILED -- saving rejects to file
content/browser/dom_storage/dom_storage_context_impl_new.cc.rej

[commit-bot: I haz the power, 2012-04-17 07:41:59]

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

[commit-bot: I haz the power, 2012-04-17 09:30:15]

Change committed as 132555