Move webkit/{browser,common}/dom_storage into content/

Move webkit/{browser,common}/dom_storage into content/

Mechanical changes only, directory moving +
* renamed all DomStorage* to DOMStorage* for consistency
* renamed DOMStorageContextImpl to DOMStorageContextProxy, and renamed DomStorageContext to DOMStorageContextImpl
* other minor cleanups

Diff for dom_storage_context_impl.* may look a bit messy
due to chained renames (it's just replaced with former
webkit/browser/dom_storage/dom_storage_context.*)

BUG=265769
NOTRY=true

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

[kinuko, 2013-08-06 13:46:52]

Started with a relatively lightweight one.  Mostly mechanical changes only,
except for renames around dom_storage_context_{impl,proxy}. (michaeln: you may
have comments on this renaming)

PTL thanks

[jam, 2013-08-06 16:13:49]

lgtm with nits

https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
File content/browser/dom_storage/dom_storage_context_impl.h (right):

https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
content/browser/dom_storage/dom_storage_context_impl.h:74: const
base::NullableString16& old_value,  // may be null on initial insert
nit: 80 chars, also below in lines 90 and 91

https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
File content/browser/dom_storage/dom_storage_message_filter.h (right):

https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
content/browser/dom_storage/dom_storage_message_filter.h:20: namespace content {
nit: combine with namespace below

https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
File content/browser/dom_storage/dom_storage_session.h (right):

https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
content/browser/dom_storage/dom_storage_session.h:1: // Copyright (c) 2012 The
Chromium Authors. All rights reserved.
hmm, why is git not seeing this file move? it would be good to preserve history

https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
File content/browser/dom_storage/local_storage_database_adapter.h (right):

https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
content/browser/dom_storage/local_storage_database_adapter.h:1: // Copyright (c)
2012 The Chromium Authors. All rights reserved.
ditto re file history

https://codereview.chromium.org/22297005/diff/52001/content/common/dom_storag...
File content/common/dom_storage/OWNERS (right):

https://codereview.chromium.org/22297005/diff/52001/content/common/dom_storag...
content/common/dom_storage/OWNERS:2: michaeln@chromium.org
should this list match content/browser/dom_storage/OWNERS? what about
content/renderer/dom_storage/OWNERS? it seems that they should all be the same
or am I mistaken?

https://codereview.chromium.org/22297005/diff/52001/content/public/browser/do...
File content/public/browser/dom_storage_context.h (right):

https://codereview.chromium.org/22297005/diff/52001/content/public/browser/do...
content/public/browser/dom_storage_context.h:73: struct CONTENT_EXPORT
LocalStorageUsageInfo {
per content api conventions
(http://www.chromium.org/developers/content-module/content-api), each
struct/class/enum needs to be in its own file.

so please put LocalStorageUsageInfo and SessionStorageUsageInfo each in their
own own header and cc file.

For SessionStorageUsageInfo do you need a constructor or destructor? it doesn't
look like clang will complain if you don't. in general we avoid adding a .cc
file uneess we have to for clang.

https://codereview.chromium.org/22297005/diff/52001/content/renderer/dom_stor...
File content/renderer/dom_storage/dom_storage_cached_area.h (right):

https://codereview.chromium.org/22297005/diff/52001/content/renderer/dom_stor...
content/renderer/dom_storage/dom_storage_cached_area.h:17: class DOMStorageMap;
nit: just move this to line 22

[michaeln, 2013-08-06 22:44:59]

The FooProxy class name did throw me? I'd vote for something like FooWrapper or
FooApiImpl instead.

This looks to be a straight move to me (which is great) such that the same
number of classes get instanced and used at runtime in exactly the same way as
before, just their names are a little different.

Is that right, if that's not right, can you point out where any runtime
differences are?

assuming there are no runtime differences... lgtm... but if there are i'd like
to take a closer look at those changes specifically

https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
File content/browser/dom_storage/dom_storage_context_impl.cc (right):

https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
content/browser/dom_storage/dom_storage_context_impl.cc:21: #include
"webkit/browser/quota/special_storage_policy.h"
i see, it's computing the diff against the wrong thing

https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
File content/browser/dom_storage/dom_storage_context_proxy.h (right):

https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
content/browser/dom_storage/dom_storage_context_proxy.h:16: class
DOMStorageContextImpl;
nit: this can go down into the namespace below

https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
content/browser/dom_storage/dom_storage_context_proxy.h:27: class CONTENT_EXPORT
DOMStorageContextProxy :
nit: I see why you're using the term 'proxy' here, but it took some looking
since we usually use 'proxy' in the context of ipc interfaces. I wonder if a
less loaded term would be better for this use. "<<>>Wrapper"? Or maybe
"<<>>APIImpl"?

[kinuko, 2013-08-07 02:43:16]

On 2013/08/06 22:44:59, michaeln wrote:
> The FooProxy class name did throw me? I'd vote for something like FooWrapper
or
> FooApiImpl instead.
> 
> This looks to be a straight move to me (which is great) such that the same
> number of classes get instanced and used at runtime in exactly the same way as
> before, just their names are a little different.
> 
> Is that right, if that's not right, can you point out where any runtime
> differences are?
> 
> assuming there are no runtime differences... lgtm... but if there are i'd like
> to take a closer look at those changes specifically

No runtime differences, code moving + renames only. I added you to reviewer
because I wanted to hear your opinion on the proxy naming.

>
https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
> File content/browser/dom_storage/dom_storage_context_impl.cc (right):
> 
>
https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
> content/browser/dom_storage/dom_storage_context_impl.cc:21: #include
> "webkit/browser/quota/special_storage_policy.h"
> i see, it's computing the diff against the wrong thing
> 
>
https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
> File content/browser/dom_storage/dom_storage_context_proxy.h (right):
> 
>
https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
> content/browser/dom_storage/dom_storage_context_proxy.h:16: class
> DOMStorageContextImpl;
> nit: this can go down into the namespace below
> 
>
https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
> content/browser/dom_storage/dom_storage_context_proxy.h:27: class
CONTENT_EXPORT
> DOMStorageContextProxy :
> nit: I see why you're using the term 'proxy' here, but it took some looking
> since we usually use 'proxy' in the context of ipc interfaces. I wonder if a
> less loaded term would be better for this use. "<<>>Wrapper"? Or maybe
> "<<>>APIImpl"?

Sg, I'm going to use 'wrapper' here.

[kinuko, 2013-08-07 14:29:52]

Updated, will be submitting

https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
File content/browser/dom_storage/dom_storage_context_impl.h (right):

https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
content/browser/dom_storage/dom_storage_context_impl.h:74: const
base::NullableString16& old_value,  // may be null on initial insert
On 2013/08/06 16:13:49, jam wrote:
> nit: 80 chars, also below in lines 90 and 91

Done.

https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
File content/browser/dom_storage/dom_storage_context_proxy.h (right):

https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
content/browser/dom_storage/dom_storage_context_proxy.h:16: class
DOMStorageContextImpl;
On 2013/08/06 22:44:59, michaeln wrote:
> nit: this can go down into the namespace below

Done.

https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
content/browser/dom_storage/dom_storage_context_proxy.h:27: class CONTENT_EXPORT
DOMStorageContextProxy :
On 2013/08/06 22:44:59, michaeln wrote:
> nit: I see why you're using the term 'proxy' here, but it took some looking
> since we usually use 'proxy' in the context of ipc interfaces. I wonder if a
> less loaded term would be better for this use. "<<>>Wrapper"? Or maybe
> "<<>>APIImpl"?

Done, renamed to Wrapper.

https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
File content/browser/dom_storage/dom_storage_message_filter.h (right):

https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
content/browser/dom_storage/dom_storage_message_filter.h:20: namespace content {
On 2013/08/06 16:13:49, jam wrote:
> nit: combine with namespace below

Done.

https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
File content/browser/dom_storage/dom_storage_session.h (right):

https://codereview.chromium.org/22297005/diff/52001/content/browser/dom_stora...
content/browser/dom_storage/dom_storage_session.h:1: // Copyright (c) 2012 The
Chromium Authors. All rights reserved.
On 2013/08/06 16:13:49, jam wrote:
> hmm, why is git not seeing this file move? it would be good to preserve
history

Tweaked git cl upload --similarity flag, now most files track their originals.

https://codereview.chromium.org/22297005/diff/52001/content/common/dom_storag...
File content/common/dom_storage/OWNERS (right):

https://codereview.chromium.org/22297005/diff/52001/content/common/dom_storag...
content/common/dom_storage/OWNERS:2: michaeln@chromium.org
On 2013/08/06 16:13:49, jam wrote:
> should this list match content/browser/dom_storage/OWNERS? what about
> content/renderer/dom_storage/OWNERS? it seems that they should all be the same
> or am I mistaken?

Hmm... ok, done. I feel michaeln@ and marja@ are the right owners, so I left
them but removed others in all OWNERS files.

https://codereview.chromium.org/22297005/diff/52001/content/public/browser/do...
File content/public/browser/dom_storage_context.h (right):

https://codereview.chromium.org/22297005/diff/52001/content/public/browser/do...
content/public/browser/dom_storage_context.h:73: struct CONTENT_EXPORT
LocalStorageUsageInfo {
On 2013/08/06 16:13:49, jam wrote:
> per content api conventions
> (http://www.chromium.org/developers/content-module/content-api), each
> struct/class/enum needs to be in its own file.
> 
> so please put LocalStorageUsageInfo and SessionStorageUsageInfo each in their
> own own header and cc file.
> 
> For SessionStorageUsageInfo do you need a constructor or destructor? it
doesn't
> look like clang will complain if you don't. in general we avoid adding a .cc
> file uneess we have to for clang.

Done.

https://codereview.chromium.org/22297005/diff/52001/content/renderer/dom_stor...
File content/renderer/dom_storage/dom_storage_cached_area.h (right):

https://codereview.chromium.org/22297005/diff/52001/content/renderer/dom_stor...
content/renderer/dom_storage/dom_storage_cached_area.h:17: class DOMStorageMap;
On 2013/08/06 16:13:49, jam wrote:
> nit: just move this to line 22

Done.

[commit-bot: I haz the power, 2013-08-07 16:54:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kinuko@chromium.org/22297005/58005

[commit-bot: I haz the power, 2013-08-07 16:59:45]

Change committed as 216211