Bug 31440 – Add Chromium's DatabaseTracker implementation to WebKit

• Component: New Bugs
• Status: RESOLVED FIXED

[Dumitru Daniliuc]

Add Chromium's DatabaseTracker implementation.

[Dumitru Daniliuc]

Created attachment 43103 [details]
patch

Somebody from Apple (Brady, Anders?) should REALLY take a look at the changes to DatabaseTracker.h. Chromium doesn't need a lot of things declared in DatabaseTracker.h, so I #ifdef'd them out.

[Michael Nordman]

I would prefer to maintain the symmetry between these two method names...


class QuotaTracker
    void getDatabaseSizeAndSpaceAvailableToOrigin(
        const String& originIdentifier, const String& databaseName,
        unsigned long long* databaseSize, unsigned long long* spaceAvailable);
    void updateDatabaseSizeAndSpaceAvailableToOrigin(
        const String& originIdentifier, const String& databaseName,
        unsigned long long databaseSize, unsigned long long spaceAvailable);


... otherwise lgtm

[Dumitru Daniliuc]

Created attachment 43133 [details]
patch

Addressed Michael's comment regarding naming inconsistency in QuotaTracker.{h|cpp}.

[Dimitri Glazkov (Google)]

Comment on attachment 43133 [details]
patch

I don't like how DatabaseTracker is so fragmented by the ifdefs. It's fine for now, but ideally this really needs to be abstracted by purpose and the lines of implementation drawn along that. Can you file a bug on this?

[Dumitru Daniliuc]

Created attachment 43175 [details]
patch

Commented out DatabaseTrackerChromium::removeOpenDatabase(). Its current implementation is causing some subtle problems when running layout tests in test_shell, and we don't even use this information yet anyway.

[Dumitru Daniliuc]

(In reply to comment #4)
> (From update of attachment 43133 [details])
> I don't like how DatabaseTracker is so fragmented by the ifdefs. It's fine for
> now, but ideally this really needs to be abstracted by purpose and the lines of
> implementation drawn along that. Can you file a bug on this?

Done. Filed bug 31482.

[Dumitru Daniliuc]

Landed as r50961.

[Eric Seidel (no email)]

Comment on attachment 43175 [details]
patch

clearing r=? since it looks like this was landed.