Bug 54421 - Add initial support for per-origin quotas to IndexedDB
Summary: Add initial support for per-origin quotas to IndexedDB
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Jeremy Orlow
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-02-14 16:28 PST by Jeremy Orlow
Modified: 2011-02-15 16:38 PST (History)
9 users (show)

See Also:


Attachments
Patch (12.32 KB, patch)
2011-02-14 16:33 PST, Jeremy Orlow
no flags Details | Formatted Diff | Diff
Patch (12.33 KB, patch)
2011-02-15 12:56 PST, Jeremy Orlow
fishd: review+
fishd: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Orlow 2011-02-14 16:28:14 PST
Add initial support for per-origin quotas to IndexedDB
Comment 1 Jeremy Orlow 2011-02-14 16:33:26 PST
Created attachment 82381 [details]
Patch
Comment 2 Jeremy Orlow 2011-02-14 16:42:40 PST
please review
Comment 3 Jeremy Orlow 2011-02-14 16:42:57 PST
+ darin for api change
Comment 4 Hans Wennborg 2011-02-15 02:35:54 PST
Comment on attachment 82381 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=82381&action=review

> Source/WebCore/storage/IDBFactoryBackendImpl.cpp:265
> +         m_currentQuotaMap.set(fileIdentifier, maximumSize);

nit: Indented one space too much?

> Source/WebCore/storage/IDBFactoryBackendImpl.h:70
> +    typedef HashMap<String, int64_t> Quota;

hmm, could it be called QuotaMap or something instead? just "Quota" makes me think of a scalar...


lgtm
Comment 5 Jeremy Orlow 2011-02-15 12:56:48 PST
Created attachment 82509 [details]
Patch
Comment 6 Darin Fisher (:fishd, Google) 2011-02-15 14:00:36 PST
Comment on attachment 82509 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=82509&action=review

> Source/WebKit/chromium/public/WebIDBFactory.h:60
> +    static const long long DefaultQuota = -1;

nit: use variable naming: fooBar

maybe these variables should be named differently so that it is more apparent
that they should be passed in for the "maximumSize" field?

how about renaming maximumSize to quota?  alternatively, you could rename the
constants to defaultMaximumSize and unlimitedMaximumSize.

also, since this expression of quota might appear in other WebKit storage APIs,
maybe we should have some generalized expression of quota?  maybe these constants
should be pulled out to a separate header file?
Comment 7 Jeremy Orlow 2011-02-15 15:44:26 PST
(In reply to comment #6)
> (From update of attachment 82509 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=82509&action=review
> 
> > Source/WebKit/chromium/public/WebIDBFactory.h:60
> > +    static const long long DefaultQuota = -1;
> 
> nit: use variable naming: fooBar

done
 
> maybe these variables should be named differently so that it is more apparent
> that they should be passed in for the "maximumSize" field?
> 
> how about renaming maximumSize to quota?  alternatively, you could rename the
> constants to defaultMaximumSize and unlimitedMaximumSize.

renamed to quota and defaultQuota
 
> also, since this expression of quota might appear in other WebKit storage APIs,
> maybe we should have some generalized expression of quota?  maybe these constants
> should be pulled out to a separate header file?

This seems a bit premature, but I will add a fixme and I've cc'ed others who are involved in Chromium quota stuff as a FYI.
Comment 8 Jeremy Orlow 2011-02-15 16:38:17 PST
Committed r78645: <http://trac.webkit.org/changeset/78645>