Bug 60985

Summary: [Chromium] WebKit/WebCore changes to support WebSQLDatabase integration with Chrome's unified quota management system.
Product: WebKit Reporter: Michael Nordman <michaeln>
Component: WebKit APIAssignee: Michael Nordman <michaeln>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dimich, fishd, kinuko, levin, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
split
none
split
none
unified
none
unified
none
unified none

Michael Nordman
Reported 2011-05-17 13:49:10 PDT
Split WebDatabase::updateDatabaseSizeAndSpaceAvailable() into two methods to support unified quota management. Previously, in the silo'd quota system, the only changes to databases could change the space available. In the new system changes in other systems can affect how much space is available for databases. So it's useful to split this method into two.
Attachments
split (6.10 KB, patch)
2011-05-17 14:05 PDT, Michael Nordman 		no flags
DetailsFormatted DiffDiff
split (5.92 KB, patch)
2011-05-17 16:06 PDT, Michael Nordman 		no flags
DetailsFormatted DiffDiff
unified (11.67 KB, patch)
2011-05-19 14:40 PDT, Michael Nordman 		no flags
DetailsFormatted DiffDiff
unified (11.67 KB, patch)
2011-05-19 14:48 PDT, Michael Nordman 		no flags
DetailsFormatted DiffDiff
unified (11.66 KB, patch)
2011-05-20 12:42 PDT, Michael Nordman 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Michael Nordman
Comment 1 2011-05-17 14:05:17 PDT
Created attachment 93814 [details] split
WebKit Review Bot
Comment 2 2011-05-17 14:06:57 PDT
Attachment 93814 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/storage/chromium/QuotaTracker.h:54: Missing space after , [whitespace/comma] [3] Source/WebKit/chromium/public/WebDatabase.h:63: WEBKIT_API should not be used on a function with a body. [readability/webkit_api] [5] Total errors found: 2 in 6 files If any of these errors are false positives, please file a bug against check-webkit-style.
Michael Nordman
Comment 3 2011-05-17 16:06:02 PDT
Created attachment 93841 [details] split fix style
Kinuko Yasuda
Comment 4 2011-05-17 22:51:22 PDT
Comment on attachment 93841 [details] split Looks good to me. View in context: https://bugs.webkit.org/attachment.cgi?id=93841&action=review > Source/WebCore/storage/chromium/QuotaTracker.h:52 > + unsigned long long databaseSize); style nit: do we format this file chromium-style since it's under chromium directory? (I often see files in chromium-style format under WebKit/chromium but not often under WebCore) > Source/WebKit/chromium/public/WebDatabase.h:59 > + // Deprecated This should go away after chrome side change? Maybe we should put a brief comment about that?
Michael Nordman
Comment 5 2011-05-18 12:05:23 PDT
I have some other changes to make in this area before this is ready. Removing review flag for now.
Michael Nordman
Comment 6 2011-05-19 14:40:37 PDT
Created attachment 94119 [details] unified
WebKit Review Bot
Comment 7 2011-05-19 14:43:28 PDT
Attachment 94119 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebKit/chromium/public/WebKitClient.h:143: origin_identifier is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.
Michael Nordman
Comment 8 2011-05-19 14:48:20 PDT
Created attachment 94122 [details] unified Fixed style. The chrome-side can be seen here... http://codereview.chromium.org/7037018/
Michael Nordman
Comment 9 2011-05-19 19:40:53 PDT
adding some reviewers to the cc list... we need get this in for M13
Kinuko Yasuda
Comment 10 2011-05-20 00:08:46 PDT
Comment on attachment 94122 [details] unified This looks good to me except for some nits in ChangeLogs. View in context: https://bugs.webkit.org/attachment.cgi?id=94122&action=review > Source/WebCore/ChangeLog:9 > + No new tests. (OOPS!) I think here we need to say 'No functionality change therefore no new tests.' or something like that. > Source/WebKit/chromium/ChangeLog:5 > + Changes to allow the WebDatabase system to participate in Chrome's unified quoata typo: s/quoata/quota/
Darin Fisher (:fishd, Google)
Comment 11 2011-05-20 11:01:04 PDT
Comment on attachment 94122 [details] unified View in context: https://bugs.webkit.org/attachment.cgi?id=94122&action=review > Source/WebKit/chromium/src/WebDatabase.cpp:98 > + WebCore::QuotaTracker::instance().updateDatabaseSize(originIdentifier, name, size); WebCore:: prefixes not needed in this .cpp file
Michael Nordman
Comment 12 2011-05-20 12:42:23 PDT
Created attachment 94261 [details] unified made all suggested changes
David Levin
Comment 13 2011-05-20 14:42:58 PDT
Comment on attachment 94261 [details] unified View in context: https://bugs.webkit.org/attachment.cgi?id=94261&action=review A few minor comments. Nothing that really needs to be fixed before submitting. (As I was writing this Darin r+'ed.) > Source/WebCore/platform/chromium/PlatformBridge.h:171 > + // Returns the space available for the origin Comment seems redundant with the method name. > Source/WebKit/chromium/ChangeLog:10 > + - WebDatabase::updateDatababaseSize() typo: updateDatababaseSize But ok. > Source/WebKit/chromium/public/WebKitClient.h:142 > + // Returns the space available for the given origin This comment seems redundant with the method name. > Source/WebKit/chromium/src/WebDatabase.cpp:99 > +#endif // ENABLE(DATABASE) You really don't need "// ENABLE(DATABASE)" in WebKit especially for such a small if/endif region.
WebKit Commit Bot
Comment 14 2011-05-20 16:47:20 PDT
The commit-queue encountered the following flaky tests while processing attachment 94261 [details]: http/tests/websocket/tests/multiple-connections.html bug 53825 (author: abarth@webkit.org) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 15 2011-05-20 16:48:58 PDT
Comment on attachment 94261 [details] unified Clearing flags on attachment: 94261 Committed r87001: <http://trac.webkit.org/changeset/87001>
WebKit Commit Bot
Comment 16 2011-05-20 16:49:04 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.