Bug 189801 – WebSQL: User cannot grant quota increase if the JS provides an expected usage value that is too low

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Chris Dumez]

User cannot grant quota increase for WebSQL if the JS provides an expected usage value that is too low.

[Chris Dumez]

<rdar://problem/43592498>

[Chris Dumez]

Created attachment 350258 [details]
Patch

[EWS Watchlist]

Comment on attachment 350258 [details]
Patch

Attachment 350258 [details] did not pass mac-ews (mac):
Output: https://webkit-queues.webkit.org/results/9286897

New failing tests:
storage/websql/transaction-database-expand-quota.html

[EWS Watchlist]

Created attachment 350267 [details]
Archive of layout-test-results from ews102 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-sierra  Platform: Mac OS X 10.12.6

[Geoffrey Garen]

Regressions: Unexpected text-only failures (1)
  storage/websql/transaction-database-expand-quota.html [ Failure ]

[Chris Dumez]

(In reply to Geoffrey Garen from comment #5)
> Regressions: Unexpected text-only failures (1)
>   storage/websql/transaction-database-expand-quota.html [ Failure ]

Yes, WK1 ignores the estimatedSize parameter and gets it from the DatabaseTracker instead. I will have a fix shortly.

[Chris Dumez]

Created attachment 350275 [details]
Patch

[youenn fablet]

Comment on attachment 350275 [details]
Patch

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

> Source/WebCore/Modules/webdatabase/Database.cpp:797
> +    if (estimatedSize() <= oldQuota) {

Is the name oldQuota misleading?
It seems like oldQuota is the current database size while estimatedSize is the 'quota' defined by the page.

> Source/WebCore/Modules/webdatabase/Database.cpp:800
> +        setEstimatedSize(oldQuota + 5 * 1024. * 1024.);

Should we move this as a macro with a name like QUOTA_SIZE_INCREMENT?

> Source/WebCore/Modules/webdatabase/Database.h:92
> +    void setEstimatedSize(unsigned);

Can we make setEstimatedSize private?
And maybe estimatedSize as well.

[Chris Dumez]

Comment on attachment 350275 [details]
Patch

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

>> Source/WebCore/Modules/webdatabase/Database.cpp:797
>> +    if (estimatedSize() <= oldQuota) {
> 
> Is the name oldQuota misleading?
> It seems like oldQuota is the current database size while estimatedSize is the 'quota' defined by the page.

I think oldQuota is correct. It is really the current quota. However, because we now we exceeded the quota, we know the database size has reached the quota.

>> Source/WebCore/Modules/webdatabase/Database.cpp:800
>> +        setEstimatedSize(oldQuota + 5 * 1024. * 1024.);
> 
> Should we move this as a macro with a name like QUOTA_SIZE_INCREMENT?

If you mean a quotaSizeIncrement global variable, then yes.

>> Source/WebCore/Modules/webdatabase/Database.h:92
>> +    void setEstimatedSize(unsigned);
> 
> Can we make setEstimatedSize private?
> And maybe estimatedSize as well.

Certainly.

[Chris Dumez]

Created attachment 350380 [details]
Patch

[Chris Dumez]

Created attachment 350388 [details]
Patch

[Chris Dumez]

Created attachment 350397 [details]
Patch

[Chris Dumez]

Created attachment 350400 [details]
Patch

[Chris Dumez]

Created attachment 350401 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 350401 [details]
Patch

Clearing flags on attachment: 350401

Committed r236348: <https://trac.webkit.org/changeset/236348>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Truitt Savell]

Looks like the new test added in https://trac.webkit.org/changeset/236348/webkit

is a flakey timeout: storage/websql/transaction-database-expand-quota.html

History:
https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=storage%2Fwebsql%2Ftransaction-database-expand-quota.html

[Chris Dumez]

(In reply to Truitt Savell from comment #17)
> Looks like the new test added in
> https://trac.webkit.org/changeset/236348/webkit
> 
> is a flakey timeout: storage/websql/transaction-database-expand-quota.html
> 
> History:
> https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> html#showAllRuns=true&tests=storage%2Fwebsql%2Ftransaction-database-expand-
> quota.html

Looking...

[Chris Dumez]

(In reply to Chris Dumez from comment #18)
> (In reply to Truitt Savell from comment #17)
> > Looks like the new test added in
> > https://trac.webkit.org/changeset/236348/webkit
> > 
> > is a flakey timeout: storage/websql/transaction-database-expand-quota.html
> > 
> > History:
> > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> > html#showAllRuns=true&tests=storage%2Fwebsql%2Ftransaction-database-expand-
> > quota.html
> 
> Looking...

I think the test is just slow in debug because of constructing the 8MB array. I'll see how I can speed it up.

[Chris Dumez]

(In reply to Chris Dumez from comment #19)
> (In reply to Chris Dumez from comment #18)
> > (In reply to Truitt Savell from comment #17)
> > > Looks like the new test added in
> > > https://trac.webkit.org/changeset/236348/webkit
> > > 
> > > is a flakey timeout: storage/websql/transaction-database-expand-quota.html
> > > 
> > > History:
> > > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.
> > > html#showAllRuns=true&tests=storage%2Fwebsql%2Ftransaction-database-expand-
> > > quota.html
> > 
> > Looking...
> 
> I think the test is just slow in debug because of constructing the 8MB
> array. I'll see how I can speed it up.

Sped up the test in <https://trac.webkit.org/changeset/236370> by using a much smaller array.