Bug 123131 – Re-instate ProposedDatabases needed by detailsForNameAndOrigin()

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Mark Lam]

The WebKit1 implementation for WebSQL quota management (to determine what database size is being requested by a webpage) used to rely on ProposedDatabase records being added and queried in DatabaseTracker::detailsForNameAndOrigin().  This was removed in favor passing the database details to the DatabaseContext::databaseExceededQuota().  However, this has proved to be inadequate.

This patch will fix this issue by re-instating the use of the ProposedDatabase record in DatabaseManager which serves as the front end interface for DatabaseTracker.

[Mark Lam]

Created attachment 214808 [details]
the patch.

Layout tests are happy.  I don't any new regressions.

[Mark Lam]

<rdar://problem/14813808>.

[Geoffrey Garen]

Comment on attachment 214808 [details]
the patch.

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

> Source/WebCore/ChangeLog:19
> +        Previously, detailsForNameAndOrigin() relies on the ProposedDatabase
> +        mechanism to provide this size information. This change re-instates the
> +        ProposedDatabase mechanism so that WebKit1 client code that relies on
> +        this behavior will continue to work.

Can you add a test for this? Seems like a pretty straightforward case.

> Source/WebCore/Modules/webdatabase/DatabaseManager.h:139
> +    HashSet<ProposedDatabase*> m_proposedDatabases;

A set implies that we plan to handle lots of proposed databases concurrently or re-entrently. Do we really intend to do that?

If not, let's make this code self-documenting by switching to a single pointer instead of a set, and ASSERTing and returning early if we're wrong.

[Mark Lam]

Created attachment 214937 [details]
patch 2: added a test, and track only 1 ProposedDatabase in DatabaseManager.

I tried to update the DumpRenderTree and test runner files for all the ports needed for the new test, but the gtk one has an abstraction layer that I don't quite understand yet.  So, instead, I added a TestExpectation to fail the test for the gtk port.  The gtk port owners will have to implement the needed changes based on other ports.

The test changes have been tested on Mac.  Will need to let the EWS check if they build on the other ports.

[EFL EWS Bot]

Comment on attachment 214937 [details]
patch 2: added a test, and track only 1 ProposedDatabase in DatabaseManager.

Attachment 214937 [details] did not pass efl-ews (efl):
Output: http://webkit-queues.appspot.com/results/7748072

[Mark Lam]

Created attachment 214938 [details]
patch 3: fixed efl build failure

[Build Bot]

Comment on attachment 214938 [details]
patch 3: fixed efl build failure

Attachment 214938 [details] did not pass win-ews (win):
Output: http://webkit-queues.appspot.com/results/7728098

[Mark Lam]

Created attachment 214970 [details]
patch 4: with speculative fix for Windows build

[Geoffrey Garen]

Comment on attachment 214970 [details]
patch 4: with speculative fix for Windows build

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

r=me

> Source/WebCore/Modules/webdatabase/DatabaseManager.h:119
> +        DatabaseManager* m_manager;

This should be a reference, since it can't be null.

[Mark Lam]

Thanks for the review. Geoff's last feedback has been addressed.  Landed in r157874: <http://trac.webkit.org/r157874>.