Add IPC plumbing code for Quota API

Add IPC plumbing code for Quota API

BUG=61676
TEST=No new tests; just IPC plumbing (manually tested)

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

[jam, 2011-04-07 16:58:29]

is this only used in renderer processes (i.e. not in workers)?  If so, then the
dispatcher should belong on RenderThread, and should be in content\renderer, not
content\common.  ResourceDisaptcher is in common, and is held in ChildThread,
because it's used in multiple types of child processes

[michaeln, 2011-04-07 20:57:31]

lgtm

I have the same question about whether the quota APIs should be accessible via
workers (in particular shared workers). Depending on the answer, the
Dispatcher/DispatcherHost should be moved or left where it is.

http://codereview.chromium.org/6811022/diff/17/content/browser/renderer_host/...
File content/browser/renderer_host/quota_dispatcher_host.h (right):

http://codereview.chromium.org/6811022/diff/17/content/browser/renderer_host/...
content/browser/renderer_host/quota_dispatcher_host.h:26: const GURL& gurl,
'origin' may be a better param name

[ericu, 2011-04-08 03:31:18]

I don't see any reason that this should be restricted to renderers.

http://codereview.chromium.org/6811022/diff/17/content/browser/renderer_host/...
File content/browser/renderer_host/quota_dispatcher_host.h (right):

http://codereview.chromium.org/6811022/diff/17/content/browser/renderer_host/...
content/browser/renderer_host/quota_dispatcher_host.h:26: const GURL& gurl,
On 2011/04/07 20:57:31, michaeln wrote:
> 'origin' may be a better param name

+1

http://codereview.chromium.org/6811022/diff/17/content/common/quota_dispatche...
File content/common/quota_dispatcher.cc (right):

http://codereview.chromium.org/6811022/diff/17/content/common/quota_dispatche...
content/common/quota_dispatcher.cc:38: DidHandleUsageAndQuotaQuery);
DidHandleRequestUsageAndQuota?

http://codereview.chromium.org/6811022/diff/17/content/common/quota_dispatcher.h
File content/common/quota_dispatcher.h (right):

http://codereview.chromium.org/6811022/diff/17/content/common/quota_dispatche...
content/common/quota_dispatcher.h:15: #include
"third_party/WebKit/Source/WebKit/chromium/public/WebStorageQuotaType.h"
Huh.  I've not previously hit this problem.  I don't think the style guide
covers #includes over 80 chars.  Do we wrap between the directive and the file
path?

http://codereview.chromium.org/6811022/diff/17/content/common/quota_messages.h
File content/common/quota_messages.h (right):

http://codereview.chromium.org/6811022/diff/17/content/common/quota_messages....
content/common/quota_messages.h:23:
IPC_MESSAGE_CONTROL4(QuotaMsg_DidHandleQueryUsageAndQuotaQuery,
The last Query seems redundant.  But if you don't want "Storage" here, perhaps
remove it from the QuotaHostMessage as well?

http://codereview.chromium.org/6811022/diff/17/content/common/quota_messages....
content/common/quota_messages.h:40: int64 /* requesting_size */);
s/requesting/requested/

[kinuko, 2011-04-08 09:02:53]

Thanks for reviewing,
In the current implementation it'll be only called by the renderer but we'll be
adding the worker support too, so I want to keep them under common for now.

http://codereview.chromium.org/6811022/diff/17/content/browser/renderer_host/...
File content/browser/renderer_host/quota_dispatcher_host.h (right):

http://codereview.chromium.org/6811022/diff/17/content/browser/renderer_host/...
content/browser/renderer_host/quota_dispatcher_host.h:26: const GURL& gurl,
On 2011/04/07 20:57:31, michaeln wrote:
> 'origin' may be a better param name

Done.

http://codereview.chromium.org/6811022/diff/17/content/common/quota_dispatche...
File content/common/quota_dispatcher.cc (right):

http://codereview.chromium.org/6811022/diff/17/content/common/quota_dispatche...
content/common/quota_dispatcher.cc:38: DidHandleUsageAndQuotaQuery);
On 2011/04/08 03:31:18, ericu wrote:
> DidHandleRequestUsageAndQuota?

Renamed to DidGrantStorageQuota and introduced DidFail separately for handling
failures (to make it match with the WebKit API's callback method names).

http://codereview.chromium.org/6811022/diff/17/content/common/quota_dispatcher.h
File content/common/quota_dispatcher.h (right):

http://codereview.chromium.org/6811022/diff/17/content/common/quota_dispatche...
content/common/quota_dispatcher.h:15: #include
"third_party/WebKit/Source/WebKit/chromium/public/WebStorageQuotaType.h"
On 2011/04/08 03:31:18, ericu wrote:
> Huh.  I've not previously hit this problem.  I don't think the style guide
> covers #includes over 80 chars.  Do we wrap between the directive and the file
> path?

I've seen this happening in several files, like in
content/renderer/render_view.cc.  Seems like our convention is just leaving the
long header line as is.

http://codereview.chromium.org/6811022/diff/17/content/common/quota_messages.h
File content/common/quota_messages.h (right):

http://codereview.chromium.org/6811022/diff/17/content/common/quota_messages....
content/common/quota_messages.h:23:
IPC_MESSAGE_CONTROL4(QuotaMsg_DidHandleQueryUsageAndQuotaQuery,
On 2011/04/08 03:31:18, ericu wrote:
> The last Query seems redundant.  But if you don't want "Storage" here, perhaps
> remove it from the QuotaHostMessage as well?

Renamed this to DidQueryStorageUsageAndQuota too.

http://codereview.chromium.org/6811022/diff/17/content/common/quota_messages....
content/common/quota_messages.h:40: int64 /* requesting_size */);
On 2011/04/08 03:31:18, ericu wrote:
> s/requesting/requested/

Done.

[ericu, 2011-04-09 02:22:33]

LGTM with one nit.

http://codereview.chromium.org/6811022/diff/7002/content/browser/renderer_hos...
File content/browser/renderer_host/quota_dispatcher_host.cc (right):

http://codereview.chromium.org/6811022/diff/7002/content/browser/renderer_hos...
content/browser/renderer_host/quota_dispatcher_host.cc:27: void
QuotaDispatcherHost::OnQueryUsageAndQuota(
OnQueryUsageAndQuota is now the only method without Storage in its name.

[kinuko, 2011-04-11 06:32:57]

@jam could you take a look at the patch again?

There's a plan to support the API in worker so it belongs to ChildThread.  (If
you want the code in RendererThread while we don't support workers I can also
update the code as you say.)

http://codereview.chromium.org/6811022/diff/7002/content/browser/renderer_hos...
File content/browser/renderer_host/quota_dispatcher_host.cc (right):

http://codereview.chromium.org/6811022/diff/7002/content/browser/renderer_hos...
content/browser/renderer_host/quota_dispatcher_host.cc:27: void
QuotaDispatcherHost::OnQueryUsageAndQuota(
On 2011/04/09 02:22:33, ericu wrote:
> OnQueryUsageAndQuota is now the only method without Storage in its name.

Done.

[kinuko, 2011-04-12 01:49:33]

Adding more content owners.
Could someone take a look at the patch?

[darin (slow to review), 2011-04-12 17:41:50]

LGTM

[jam, 2011-04-12 17:53:52]

lgtm

http://codereview.chromium.org/6811022/diff/11036/content/browser/renderer_ho...
File content/browser/renderer_host/quota_dispatcher_host.h (right):

http://codereview.chromium.org/6811022/diff/11036/content/browser/renderer_ho...
content/browser/renderer_host/quota_dispatcher_host.h:15: class Message;
not needed

http://codereview.chromium.org/6811022/diff/11036/content/common/quota_dispat...
File content/common/quota_dispatcher.cc (right):

http://codereview.chromium.org/6811022/diff/11036/content/common/quota_dispat...
content/common/quota_dispatcher.cc:10: #include "ipc/ipc_message.h"
nit: this isn't needed