Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(59)

Issue 7037018: DB quota (Closed)

Created:
8 years, 6 months ago by michaeln
Modified:
8 years, 6 months ago
CC:
chromium-reviews, darin-cc_chromium.org, jam, dgrogan
Visibility:
Public.

Description

Use the QuotaManager to determine the space available to WebSQLDatabases. Since activity outside of the database system now affect how much space is available, our strategy for informing the client side of the new limit needed to change. It's not enough to just send the new limit when a change within the DB system has occurred. This change depends on a webkit/webcore change. https://bugs.webkit.org/show_bug.cgi?id=60985 In this CL the renderer will ask the browser for the limit when needed. But also in this CL are ipc plumbing additions to support notifying the renderer as changes occur. That plumbing will be utilized in a later CL. * [Chrome] DatabaseMessageFilter uses the QuotaManager respond to the SpaceAvailable query. * [DRT] SimpleDatabaseSystem uses quota_per_origin_ data member to respond to the SpaceAvailable query. * Mostly mind numbing plumbing for WebKit API additions. // Sync getter for use on webcore's background db threads to // call out to chrome to get the value. long long WebKitClient::databaseGetSpaceAvailableForOrigin(origin_identifier); // Split the existing updateDatabaseSize method into three methods. Chrome calls into these. static void WebDatabase::updateDatabaseSize(originIdentifier, dbname, spaceAvailable); static void WebDatabase::updateSpaceAvailable(originIdentifier, spaceAvailable); static void WebDatabase::resetSpaceAvailable(originIdentifier); Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=86537

Patch Set 1 #

Patch Set 2 : '' #

Patch Set 3 : '' #

Patch Set 4 : '' #

Total comments: 3

Patch Set 5 : '' #

Patch Set 6 : '' #

Patch Set 7 : '' #

Total comments: 2

Patch Set 8 : '' #

Total comments: 6

Patch Set 9 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+234 lines, -50 lines) Patch
M content/browser/renderer_host/database_message_filter.h View 1 2 3 4 5 6 7 8 1 chunk +4 lines, -0 lines 0 comments Download
M content/browser/renderer_host/database_message_filter.cc View 1 2 3 4 5 6 7 8 6 chunks +71 lines, -12 lines 0 comments Download
M content/common/database_messages.h View 1 2 3 4 5 6 7 8 2 chunks +15 lines, -2 lines 0 comments Download
M content/common/database_util.h View 1 2 3 4 5 6 7 8 1 chunk +9 lines, -5 lines 0 comments Download
M content/common/database_util.cc View 1 2 3 4 5 6 7 8 4 chunks +13 lines, -4 lines 0 comments Download
M content/common/db_message_filter.h View 1 2 3 4 5 6 7 8 1 chunk +4 lines, -2 lines 0 comments Download
M content/common/db_message_filter.cc View 1 2 3 4 5 6 7 8 2 chunks +18 lines, -3 lines 0 comments Download
M content/renderer/renderer_webkitclient_impl.h View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -0 lines 0 comments Download
M content/renderer/renderer_webkitclient_impl.cc View 1 2 3 4 5 6 7 8 1 chunk +9 lines, -4 lines 0 comments Download
M content/worker/worker_webkitclient_impl.h View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -0 lines 0 comments Download
M content/worker/worker_webkitclient_impl.cc View 1 2 3 4 5 6 7 8 1 chunk +9 lines, -4 lines 0 comments Download
M webkit/database/database_tracker.h View 1 2 3 4 5 6 7 8 3 chunks +13 lines, -7 lines 0 comments Download
M webkit/glue/webkitclient_impl.h View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -0 lines 0 comments Download
M webkit/glue/webkitclient_impl.cc View 1 2 3 4 5 6 7 8 1 chunk +5 lines, -0 lines 0 comments Download
M webkit/support/simple_database_system.h View 1 2 3 4 5 6 7 8 3 chunks +4 lines, -0 lines 0 comments Download
M webkit/support/simple_database_system.cc View 1 2 3 4 5 6 7 8 7 chunks +36 lines, -6 lines 0 comments Download
M webkit/support/test_webkit_client.h View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -0 lines 0 comments Download
M webkit/support/test_webkit_client.cc View 1 2 3 4 5 6 7 8 2 chunks +8 lines, -1 line 0 comments Download
M webkit/tools/test_shell/test_shell_webkit_init.h View 1 2 3 4 5 6 7 8 1 chunk +2 lines, -0 lines 0 comments Download
M webkit/tools/test_shell/test_shell_webkit_init.cc View 1 2 3 4 5 6 7 8 1 chunk +6 lines, -0 lines 0 comments Download

Messages

Total messages: 16 (0 generated)
michaeln
I think this is ready for looking at now, mostly plumbing but SimpleDatabaseSystem and DatabaseMessageFilter ...
8 years, 6 months ago (2011-05-19 06:47:04 UTC) #1
michaeln
http://codereview.chromium.org/7037018/diff/5009/content/browser/renderer_host/database_message_filter.cc File content/browser/renderer_host/database_message_filter.cc (right): http://codereview.chromium.org/7037018/diff/5009/content/browser/renderer_host/database_message_filter.cc#newcode285 content/browser/renderer_host/database_message_filter.cc:285: quota_manager->GetUsageAndQuota( Hi Kinuko, my local build got into an ...
8 years, 6 months ago (2011-05-19 07:03:34 UTC) #2
kinuko
Geez that's bad. GetUsageAndQuota in turn calls client's GetUsageForOrigin, could there be some re-entrancy issue? ...
8 years, 6 months ago (2011-05-19 07:18:29 UTC) #3
kinuko
LGTM http://codereview.chromium.org/7037018/diff/5009/content/common/database_util.h File content/common/database_util.h (right): http://codereview.chromium.org/7037018/diff/5009/content/common/database_util.h#newcode23 content/common/database_util.h:23: static long long databaseGetSpaceAvaialble( typo: Avaialble -> Available ...
8 years, 6 months ago (2011-05-19 16:33:38 UTC) #4
michaeln
http://codereview.chromium.org/7037018/diff/5009/content/common/database_util.h File content/common/database_util.h (right): http://codereview.chromium.org/7037018/diff/5009/content/common/database_util.h#newcode23 content/common/database_util.h:23: static long long databaseGetSpaceAvaialble( On 2011/05/19 16:33:38, kinuko wrote: ...
8 years, 6 months ago (2011-05-19 22:19:47 UTC) #5
michaeln
On 2011/05/19 07:18:29, kinuko wrote: > Geez that's bad. GetUsageAndQuota in turn calls client's GetUsageForOrigin, ...
8 years, 6 months ago (2011-05-20 05:49:31 UTC) #6
kinuko
On 2011/05/20 05:49:31, michaeln wrote: > On 2011/05/19 07:18:29, kinuko wrote: > > Geez that's ...
8 years, 6 months ago (2011-05-20 05:56:58 UTC) #7
michaeln
> Great, ...I see that could happen. Seems like we should use a separate flag ...
8 years, 6 months ago (2011-05-20 23:56:48 UTC) #8
michaeln
i ran the perf tests with this vs M12... no significance difference (some slighly faster, ...
8 years, 6 months ago (2011-05-22 21:24:13 UTC) #9
michaeln
@darin for content owners This is the chrome-side of the webkit patch you reviewed last ...
8 years, 6 months ago (2011-05-22 21:49:01 UTC) #10
darin (slow to review)
http://codereview.chromium.org/7037018/diff/25024/content/browser/renderer_host/database_message_filter.cc File content/browser/renderer_host/database_message_filter.cc (right): http://codereview.chromium.org/7037018/diff/25024/content/browser/renderer_host/database_message_filter.cc#newcode44 content/browser/renderer_host/database_message_filter.cc:44: // Params are <status, usage, quota> nit: create local ...
8 years, 6 months ago (2011-05-24 04:56:10 UTC) #11
michaeln
http://codereview.chromium.org/7037018/diff/25024/webkit/support/simple_database_system.cc File webkit/support/simple_database_system.cc (right): http://codereview.chromium.org/7037018/diff/25024/webkit/support/simple_database_system.cc#newcode268 webkit/support/simple_database_system.cc:268: done_event->Signal(); On 2011/05/24 04:56:11, darin wrote: > are you ...
8 years, 6 months ago (2011-05-24 05:10:54 UTC) #12
michaeln
> right... i remember that too... i'll check, the other uses like this haven't > ...
8 years, 6 months ago (2011-05-24 07:34:04 UTC) #13
darin (slow to review)
On Tue, May 24, 2011 at 12:34 AM, <michaeln@chromium.org> wrote: > right... i remember that ...
8 years, 6 months ago (2011-05-24 16:30:05 UTC) #14
michaeln
@darin, ptal http://codereview.chromium.org/7037018/diff/25024/content/browser/renderer_host/database_message_filter.cc File content/browser/renderer_host/database_message_filter.cc (right): http://codereview.chromium.org/7037018/diff/25024/content/browser/renderer_host/database_message_filter.cc#newcode44 content/browser/renderer_host/database_message_filter.cc:44: // Params are <status, usage, quota> On ...
8 years, 6 months ago (2011-05-24 19:43:10 UTC) #15
darin (slow to review)
8 years, 6 months ago (2011-05-24 20:56:17 UTC) #16
LGTM

On Tue, May 24, 2011 at 12:43 PM, <michaeln@chromium.org> wrote:

> @darin, ptal
>
>
>
>
>
http://codereview.chromium.org/7037018/diff/25024/content/browser/renderer_ho...
> File content/browser/renderer_host/database_message_filter.cc (right):
>
>
>
http://codereview.chromium.org/7037018/diff/25024/content/browser/renderer_ho...
> content/browser/renderer_host/database_message_filter.cc:44: // Params
> are <status, usage, quota>
> On 2011/05/24 04:56:11, darin wrote:
>
>> nit: create local variables with these names?  it will help make the
>>
> code below
>
>> more readable.
>>
>
> Done.
>
>
>
>
http://codereview.chromium.org/7037018/diff/25024/content/common/database_util.h
> File content/common/database_util.h (right):
>
>
>
http://codereview.chromium.org/7037018/diff/25024/content/common/database_uti...
> content/common/database_util.h:15: static
> WebKit::WebKitClient::FileHandle databaseOpenFile(
> On 2011/05/24 04:56:11, darin wrote:
>
>> nit: these method names should start with an uppercase letter.
>>
>
> Done.
>
>
> http://codereview.chromium.org/7037018/
>

Powered by Google App Engine
This is Rietveld 408576698