Bug 114809 - [WK2][GTK] ExceededDatabaseQuota method support in WebKitUIClient
Summary: [WK2][GTK] ExceededDatabaseQuota method support in WebKitUIClient
Status: UNCONFIRMED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-18 06:01 PDT by Danilo Cesar Lemes de Paula
Modified: 2017-03-11 11:01 PST (History)
14 users (show)

See Also:


Attachments
patch (11.03 KB, patch)
2013-04-18 06:01 PDT, Danilo Cesar Lemes de Paula
no flags Details | Formatted Diff | Diff
Patch (13.07 KB, patch)
2013-04-19 10:52 PDT, Danilo Cesar Lemes de Paula
no flags Details | Formatted Diff | Diff
Patch (12.20 KB, patch)
2013-04-23 14:29 PDT, Danilo Cesar Lemes de Paula
no flags Details | Formatted Diff | Diff
Patch (19.52 KB, patch)
2013-04-29 20:04 PDT, Danilo Cesar Lemes de Paula
no flags Details | Formatted Diff | Diff
Patch (19.78 KB, patch)
2013-04-29 20:20 PDT, Danilo Cesar Lemes de Paula
no flags Details | Formatted Diff | Diff
Patch (19.68 KB, patch)
2013-04-29 20:45 PDT, Danilo Cesar Lemes de Paula
no flags Details | Formatted Diff | Diff
Patch (19.68 KB, patch)
2013-04-29 20:49 PDT, Danilo Cesar Lemes de Paula
mcatanzaro: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Danilo Cesar Lemes de Paula 2013-04-18 06:01:00 PDT
Created attachment 198727 [details]
patch

it's a draft implementation of exceededDatabaseQuota

When the securityOrigin reaches it's max quota, a signal will be emited and the return value of this signal should be the new quota.
When there's no one connected to that signal, the default value (5MB) will be returned.
Comment 1 Danilo Cesar Lemes de Paula 2013-04-18 06:04:16 PDT
(In reply to comment #0)
> Created an attachment (id=198727) [details]
> patch
> 
> it's a draft implementation of exceededDatabaseQuota
> 
> When the securityOrigin reaches it's max quota, a signal will be emited and the return value of this signal should be the new quota.
> When there's no one connected to that signal, the default value (5MB) will be returned.

My concern now is that if the WebContext is the right place to put the set/get quota methods.
Comment 2 Danilo Cesar Lemes de Paula 2013-04-19 10:52:21 PDT
Created attachment 198898 [details]
Patch
Comment 3 WebKit Commit Bot 2013-04-19 10:53:47 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 4 WebKit Commit Bot 2013-04-19 10:53:59 PDT
Attachment 198898 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/UIProcess/API/gtk/WebKitUIClient.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.h', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebViewPrivate.h', u'Source/WebKit2/UIProcess/API/gtk/docs/webkit2gtk-sections.txt', u'Source/WebKit2/UIProcess/API/gtk/webkit2marshal.list']" exit_code: 1
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1344:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1345:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1346:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1347:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1348:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1349:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1350:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1351:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1352:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1353:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1354:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1355:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1356:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WARNING: File exempt from style guide. Skipping: "Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.h"
Total errors found: 13 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Danilo Cesar Lemes de Paula 2013-04-19 11:07:59 PDT
(In reply to comment #3)
> Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

That's not the final version. I would like to discuss the API property.
Comment 6 Martin Robinson 2013-04-19 11:33:42 PDT
Comment on attachment 198898 [details]
Patch

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

The signal is per-WebView, but the setting you are adding is per WebContext. I think this might be a better candidate for a WebSetting (applies to all WebViews in a WebViewGroup) or something more fine-grained. Having a setting on the WebContext does not allow you to create WebViews with different quotas.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebContext.h:200
> +                                                     guint64                       *defaultQuota);

You should use default_quota here, defaultQuota in the C++ implementation file, and default_quota for the documentation.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1356
>> +                     G_TYPE_UINT64);
> 
> Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]

You can ensure your code passes the style bot by uploading your patches with webkit-patch.
Comment 7 Danilo Cesar Lemes de Paula 2013-04-23 14:29:04 PDT
Created attachment 199326 [details]
Patch
Comment 8 Martin Robinson 2013-04-23 14:56:42 PDT
Comment on attachment 199326 [details]
Patch

Perhaps I'm misreading your patch, but it seems you set can adjust the default database quota in WebKitSettings, but that it isn't used at all.
Comment 9 Danilo Cesar Lemes de Paula 2013-04-23 15:03:37 PDT
(In reply to comment #8)
> (From update of attachment 199326 [details])
> Perhaps I'm misreading your patch, but it seems you set can adjust the default database quota in WebKitSettings, but that it isn't used at all.

It's used on webkitWebViewExceededDatabaseQuotaPermissionRequest.

But forget about this patch for now, I'm assuming that the default value starts with 0.
What's the best way to initiate a value on WebkitSettingsPrivate now that it's being constructed automatically?
Comment 10 Martin Robinson 2013-04-23 15:15:05 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > (From update of attachment 199326 [details] [details])
> > Perhaps I'm misreading your patch, but it seems you set can adjust the default database quota in WebKitSettings, but that it isn't used at all.
> 
> It's used on webkitWebViewExceededDatabaseQuotaPermissionRequest.

Ah, thanks!

> But forget about this patch for now, I'm assuming that the default value starts with 0.
> What's the best way to initiate a value on WebkitSettingsPrivate now that it's being constructed automatically?

I think a good first patch for this API is to skip the signal for now and simply use the value that the API consumer sets on WebKitSettings. Perhaps Carlos has a different opinion, but we are trying to avoid the overwhelming amount of signals that we had in WebKit1.
Comment 11 Danilo Cesar Lemes de Paula 2013-04-25 07:14:39 PDT
(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > (From update of attachment 199326 [details] [details] [details])
> > > Perhaps I'm misreading your patch, but it seems you set can adjust the default database quota in WebKitSettings, but that it isn't used at all.
> > 
> > It's used on webkitWebViewExceededDatabaseQuotaPermissionRequest.
> 
> Ah, thanks!
> 
> > But forget about this patch for now, I'm assuming that the default value starts with 0.
> > What's the best way to initiate a value on WebkitSettingsPrivate now that it's being constructed automatically?
> 
> I think a good first patch for this API is to skip the signal for now and simply use the value that the API consumer sets on WebKitSettings. Perhaps Carlos has a different opinion, but we are trying to avoid the overwhelming amount of signals that we had in WebKit1.

Is there an alternative way to warn the user that an origin is over-using its database quota without using the webview signal? I agree with your concerns about the overwhelming amount of signals on webkit1, but does it pay the cost of removing a feature to keep the signal list clear?

Carlos, would you mind to give your opinion/suggestion in this case, please?

Anyway, since the WebkitSettings setter/getter is enough to allow the sqldatabase usage, I'm adapting the patch to include/use only that part and we can keep the signal discussion for later. Are you OK with that?
Comment 12 Martin Robinson 2013-04-25 09:22:16 PDT
(In reply to comment #11)

> Is there an alternative way to warn the user that an origin is over-using its database quota without using the webview signal? I agree with your concerns about the overwhelming amount of signals on webkit1, but does it pay the cost of removing a feature to keep the signal list clear?

This seems like a useful feature, but I think it should be added in tandem with consumers in downstream projects. We'll have a good idea when it's necessary because WebKitGTK+ browsers will need it.
Comment 13 Danilo Cesar Lemes de Paula 2013-04-29 20:04:39 PDT
Created attachment 200073 [details]
Patch
Comment 14 WebKit Commit Bot 2013-04-29 20:05:58 PDT
Attachment 200073 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitSettings.h', u'Source/WebKit2/UIProcess/API/gtk/WebKitUIClient.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebViewPrivate.h', u'Source/WebKit2/UIProcess/API/gtk/docs/webkit2gtk-sections.txt', u'Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp', u'Tools/ChangeLog', u'Tools/MiniBrowser/gtk/BrowserCellRendererVariant.c', u'Tools/MiniBrowser/gtk/BrowserSettingsDialog.c', u'Tools/MiniBrowser/gtk/main.c']" exit_code: 1
Tools/MiniBrowser/gtk/main.c:155:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Tools/ChangeLog:1:  ChangeLog entry has no bug number  [changelog/bugnumber] [5]
WARNING: File exempt from style guide. Skipping: "Source/WebKit2/UIProcess/API/gtk/WebKitSettings.h"
Tools/MiniBrowser/gtk/BrowserSettingsDialog.c:151:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1089:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1091:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Total errors found: 5 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 Danilo Cesar Lemes de Paula 2013-04-29 20:20:39 PDT
Created attachment 200074 [details]
Patch
Comment 16 WebKit Commit Bot 2013-04-29 20:22:57 PDT
Attachment 200074 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitSettings.h', u'Source/WebKit2/UIProcess/API/gtk/WebKitUIClient.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebViewPrivate.h', u'Source/WebKit2/UIProcess/API/gtk/docs/webkit2gtk-sections.txt', u'Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp', u'Tools/ChangeLog', u'Tools/MiniBrowser/gtk/BrowserCellRendererVariant.c', u'Tools/MiniBrowser/gtk/BrowserSettingsDialog.c', u'Tools/MiniBrowser/gtk/main.c']" exit_code: 1
Tools/MiniBrowser/gtk/main.c:155:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
WARNING: File exempt from style guide. Skipping: "Source/WebKit2/UIProcess/API/gtk/WebKitSettings.h"
Tools/MiniBrowser/gtk/BrowserSettingsDialog.c:151:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1089:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1091:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Total errors found: 4 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 17 Martin Robinson 2013-04-29 20:28:38 PDT
(In reply to comment #16)
> Attachment 200074 [details] did not pass style-queue:
> 
> Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitSettings.h', u'Source/WebKit2/UIProcess/API/gtk/WebKitUIClient.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebViewPrivate.h', u'Source/WebKit2/UIProcess/API/gtk/docs/webkit2gtk-sections.txt', u'Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp', u'Tools/ChangeLog', u'Tools/MiniBrowser/gtk/BrowserCellRendererVariant.c', u'Tools/MiniBrowser/gtk/BrowserSettingsDialog.c', u'Tools/MiniBrowser/gtk/main.c']" exit_code: 1
> Tools/MiniBrowser/gtk/main.c:155:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
> WARNING: File exempt from style guide. Skipping: "Source/WebKit2/UIProcess/API/gtk/WebKitSettings.h"
> Tools/MiniBrowser/gtk/BrowserSettingsDialog.c:151:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1089:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1091:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
> Total errors found: 4 in 12 files
> 

If this churn bothers you you should upload your patch with webkit-patch or run check-webkit-style first. Everyone CC'd gets an email when this happens. Obviously it isn't a big deal (and I do it sometimes too), but it's less work just to use webkit-patch, I think.
Comment 18 Danilo Cesar Lemes de Paula 2013-04-29 20:45:21 PDT
Created attachment 200075 [details]
Patch
Comment 19 Danilo Cesar Lemes de Paula 2013-04-29 20:47:16 PDT
There's no signal anymore
Comment 20 WebKit Commit Bot 2013-04-29 20:47:49 PDT
Attachment 200075 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitSettings.h', u'Source/WebKit2/UIProcess/API/gtk/WebKitUIClient.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitWebViewPrivate.h', u'Source/WebKit2/UIProcess/API/gtk/docs/webkit2gtk-sections.txt', u'Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp', u'Tools/ChangeLog', u'Tools/MiniBrowser/gtk/BrowserCellRendererVariant.c', u'Tools/MiniBrowser/gtk/BrowserSettingsDialog.c', u'Tools/MiniBrowser/gtk/main.c']" exit_code: 1
WARNING: File exempt from style guide. Skipping: "Source/WebKit2/UIProcess/API/gtk/WebKitSettings.h"
Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1091:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Total errors found: 1 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 21 Danilo Cesar Lemes de Paula 2013-04-29 20:49:59 PDT
Created attachment 200076 [details]
Patch
Comment 22 Martin Robinson 2013-05-03 06:41:25 PDT
Comment on attachment 200076 [details]
Patch

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

Looks good to me. Since this adds new API it needs review from two GTK+ reviewers and also a WebKit2 owner.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:68
> +    guint64 defaultWebDatabaseQuota;

You shoult use int64_t here, if possible.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1086
> +     * The default quota for Web SQL Databases

You might want to talk briefly about what happens when the quota is reached.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1173
> +void webkit_settings_set_default_web_database_quota(WebKitSettings* settings, guint64 default_quota)

Here, but not in the doc or the header, you should rename default_quota to defaultQuota.

> Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1568
> +    guint64 quota = webkit_settings_get_default_web_database_quota(settings);
> +
> +    return quota;

You could just do 

return webkit_settings_get_default_web_database_quota(settings);

here.

> Tools/MiniBrowser/gtk/BrowserSettingsDialog.c:146
> +                uIntProperty->maximum, 1, 1, 1);

It's okay to make this one line.

> Tools/MiniBrowser/gtk/BrowserSettingsDialog.c:151
> +            adjustment = gtk_adjustment_new(uInt64Property->default_value, uInt64Property->minimum,
> +                uInt64Property->maximum, 1, 1, 1);

Ditto.

> Tools/MiniBrowser/gtk/main.c:155
> -            || gParamType == G_TYPE_FLOAT);
> +        || gParamType == G_TYPE_UINT64 || gParamType == G_TYPE_FLOAT);

Ditto.

> Tools/MiniBrowser/gtk/main.c:253
> +

Looks like this snuck in by accident.
Comment 23 Martin Robinson 2013-05-03 06:41:47 PDT
Carlos, can you review this patch as well?
Comment 24 Gustavo Noronha (kov) 2013-05-03 12:18:15 PDT
Comment on attachment 200076 [details]
Patch

I think the API is OK in general, I think there's value in letting the user agent decide to increase the quota once it's reached, but I'm in favour of not doing before we get a better feeling of how that should work UI-wise, maybe we can even add a request for increasing the quote by a fixed amount that is presented by webkit2 itself rather than by the browser, like for http basic auth…
Comment 25 Carlos Garcia Campos 2013-05-06 09:45:01 PDT
Comment on attachment 200076 [details]
Patch

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

I don't know much about web database, but I wonder if it would make more sense to expose web database API and expose methods to get/set the web database quota per security origin, using a default value like webkit1 did.

>> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:68
>> +    guint64 defaultWebDatabaseQuota;
> 
> You shoult use int64_t here, if possible.

Actually uint64_t

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1087
> +     */

Since: 2.2

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1093
> +            0, G_MAXUINT64, 5 * 1024 * 1024,

We could probably use a global static var or a macro for the default value.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1159
> + **/

Use single * here, and add Since: 2.2 tag

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1172
> + **/

Ditto.

> Source/WebKit2/UIProcess/API/gtk/WebKitUIClient.cpp:199
> -        0, // exceededDatabaseQuota
> +        decidePolicyForExceededDatabaseQuotaPermissionRequest,

We don't usually rename the callbacks, it makes easier to match callback name and its implementation,
Comment 26 Carlos Garcia Campos 2013-05-06 09:45:52 PDT
Adding wk2 owners.
Comment 27 Carlos Garcia Campos 2013-05-06 09:48:11 PDT
(In reply to comment #19)
> There's no signal anymore

To avoid adding another signal to web view, we can use the generic permission request signal, adding a new permission request object for web database quota permission.
Comment 28 Martin Robinson 2013-05-06 10:01:35 PDT
(In reply to comment #27)
> (In reply to comment #19)
> > There's no signal anymore
> 
> To avoid adding another signal to web view, we can use the generic permission request signal, adding a new permission request object for web database quota permission.

One issue is that you don't actually respond with allow or deny, but are expected to adjust the quota setting.
Comment 29 Gustavo Noronha (kov) 2013-05-06 16:03:25 PDT
Should just need API for this new permission object that lets you tell it how much to increase the quota to, I guess, instead of using the allow/deny ones? I think allowing setting the quota per origin makes sense in principle, but does any current API user actually need that at the moment?
Comment 30 Martin Robinson 2013-05-06 16:06:13 PDT
(In reply to comment #29)
> Should just need API for this new permission object that lets you tell it how much to increase the quota to, I guess, instead of using the allow/deny ones? I think allowing setting the quota per origin makes sense in principle, but does any current API user actually need that at the moment?

That is one approach, but it opens up more questions. For instance, does the API on the permissions object proxy to the WebKitSetting property or does it only apply to the WebView in question? If it's just a proxy, isn't it redundant? If it doesn't proxy, what takes precedence when the application then adjusts the WebSettings property?
Comment 31 Carlos Garcia Campos 2013-05-06 23:49:56 PDT
(In reply to comment #29)
> Should just need API for this new permission object that lets you tell it how much to increase the quota to, I guess, instead of using the allow/deny ones?

Or in addition to allow/deny. set_quota just saves the value and the allow impl actually updates the quota.

> I think allowing setting the quota per origin makes sense in principle, but does any current API user actually need that at the moment?

Good question.
Comment 32 Carlos Garcia Campos 2013-05-06 23:51:09 PDT
(In reply to comment #30)
> (In reply to comment #29)
> > Should just need API for this new permission object that lets you tell it how much to increase the quota to, I guess, instead of using the allow/deny ones? I think allowing setting the quota per origin makes sense in principle, but does any current API user actually need that at the moment?
> 
> That is one approach, but it opens up more questions. For instance, does the API on the permissions object proxy to the WebKitSetting property or does it only apply to the WebView in question? If it's just a proxy, isn't it redundant? If it doesn't proxy, what takes precedence when the application then adjusts the WebSettings property?

That was my main concern, if we are going to expose a web database api like in wk1, this setting might be redundant and confusing.
Comment 33 Gustavo Noronha (kov) 2014-01-06 09:40:00 PST
Well, the setting is for the default value, the permission would be to increase the quota for a specific origin (which would require API for managing the quotas per-origin, too).
Comment 34 Andre Moreira Magalhaes 2014-02-23 17:37:09 PST
Related: API used in wk2-efl https://bugs.webkit.org/show_bug.cgi?id=97882. They are using a callback in the view that is invoked when the quota is exceeded. If the callback is not set the default value of 5MB is used. No websettings involved.

I think we should take a similar approach by using a signal in the view but using websettings to allow setting the default value though (to be used in case no one is connected to the signal).

I don't like the idea of re-using the permission-request signal for this as I believe the API would be confusing: 1) allow/deny doesn't apply directly here 2) it would require a new PermissionRequest object 3) the quota-exceeded signal carries useful information (currentQuota, currentOriginUsage, currentDatabaseUsage, expectUsage) that would also have to be handled in the new PermissionRequest object ...

Let me know what you think and I will try to update the patch.
Comment 35 Michael Catanzaro 2016-01-06 18:55:32 PST
Comment on attachment 200076 [details]
Patch

Martin and Carlos have several requested improvements to this patch. Removing from request queue.
Comment 36 Michael Catanzaro 2016-01-06 18:58:27 PST
(In reply to comment #34)
> Let me know what you think and I will try to update the patch.

What I don't understand is: why would an application want to increase the quota when it is exceeded? I can understand why apps might need to change the default quota, but what use-case do you have for an API that tells you when the quota is exceeded and then allows you the option of increasing it?

Surely you're not going to expose database quotas to users, so there is not going to be any user interaction here. So what factor will your application use to determine the new value of the quota, which was not available when the quota was first set?