RESOLVED FIXED Bug 58784
[Chromium] Enable QUOTA API at runtime if enable-quota flag is given
https://bugs.webkit.org/show_bug.cgi?id=58784
Summary [Chromium] Enable QUOTA API at runtime if enable-quota flag is given
Kinuko Yasuda
Reported 2011-04-18 07:25:16 PDT
Enable QUOTA API (added in http://trac.webkit.org/changeset/84053) at runtime if enable-quota flag is given. For now the feature is enabled only if the quota compile-time flag is given.
Attachments
Patch (5.59 KB, patch)
2011-04-18 07:30 PDT, Kinuko Yasuda
levin: review+
Kinuko Yasuda
Comment 1 2011-04-18 07:30:35 PDT
David Levin
Comment 2 2011-04-18 09:24:26 PDT
Comment on attachment 90031 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=90031&action=review Please wait for Darin's ok since you did a chromium public api change. > Source/WebKit/chromium/src/WebRuntimeFeatures.cpp:323 > +void WebRuntimeFeatures::enableQuota(bool enable) I suspect you'll trigger a warning and thus a compile error due to unused enable when QUOTA isn't enabled.
Kinuko Yasuda
Comment 3 2011-04-19 05:38:36 PDT
WebKit Review Bot
Comment 4 2011-04-19 05:52:06 PDT
http://trac.webkit.org/changeset/84247 might have broken Chromium Win Release
David Levin
Comment 5 2011-04-19 06:29:29 PDT
(In reply to comment #2) > (From update of attachment 90031 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=90031&action=review > > Please wait for Darin's ok since you did a chromium public api change. I didn't see Darin's ok in here. > > > Source/WebKit/chromium/src/WebRuntimeFeatures.cpp:323 > > +void WebRuntimeFeatures::enableQuota(bool enable) > > I suspect you'll trigger a warning and thus a compile error due to unused enable when QUOTA isn't enabled. It doesn't look like this was handled.
Kinuko Yasuda
Comment 6 2011-04-19 06:41:59 PDT
(In reply to comment #5) > (In reply to comment #2) > > (From update of attachment 90031 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=90031&action=review > > > > Please wait for Darin's ok since you did a chromium public api change. > > I didn't see Darin's ok in here. Oops, sorry I overlooked the commend... ok I will revert the change. > > > Source/WebKit/chromium/src/WebRuntimeFeatures.cpp:323 > > > +void WebRuntimeFeatures::enableQuota(bool enable) > > > > I suspect you'll trigger a warning and thus a compile error due to unused enable when QUOTA isn't enabled. > > It doesn't look like this was handled. Sorry I didn't reply. I thought it would be ok (and seems to be ok) as the file have a lot of similar lines (i.e.g void WebRuntimeFeatures::enableXxx(bool enable) with #if ENABLE(XXX) in body).
David Levin
Comment 7 2011-04-19 06:44:23 PDT
(In reply to comment #6) > (In reply to comment #5) > > (In reply to comment #2) > > > (From update of attachment 90031 [details] [details] [details]) > > > View in context: https://bugs.webkit.org/attachment.cgi?id=90031&action=review > > > > > > Please wait for Darin's ok since you did a chromium public api change. > > > > I didn't see Darin's ok in here. > > Oops, sorry I overlooked the commend... ok I will revert the change. At this point, I wouldn't revert. Just get an ok from Darin or do any changes that he suggests. > I thought it would be ok as the file have a lot of similar lines . Ah. Thanks!
Kinuko Yasuda
Comment 8 2011-04-19 06:50:23 PDT
(In reply to comment #7) > (In reply to comment #6) > > (In reply to comment #5) > > > (In reply to comment #2) > > > > (From update of attachment 90031 [details] [details] [details] [details]) > > > > View in context: https://bugs.webkit.org/attachment.cgi?id=90031&action=review > > > > > > > > Please wait for Darin's ok since you did a chromium public api change. > > > > > > I didn't see Darin's ok in here. > > > > Oops, sorry I overlooked the commend... ok I will revert the change. > > At this point, I wouldn't revert. Just get an ok from Darin or do any changes that he suggests. Ok, will do. Darin, please let me know if anything need to be changed/reverted. Thanks! > > I thought it would be ok as the file have a lot of similar lines . > Ah. Thanks!
Darin Fisher (:fishd, Google)
Comment 9 2011-04-19 10:16:25 PDT
Comment on attachment 90031 [details] Patch API changes LGTM
Note You need to log in before you can comment on or make changes to this bug.