Bug 58784

Summary: [Chromium] Enable QUOTA API at runtime if enable-quota flag is given
Product: WebKit Reporter: Kinuko Yasuda <kinuko>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, fishd, levin, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 60355    
Attachments:
Description Flags
Patch levin: review+

Description Kinuko Yasuda 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.
Comment 1 Kinuko Yasuda 2011-04-18 07:30:35 PDT
Created attachment 90031 [details]
Patch
Comment 2 David Levin 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.
Comment 3 Kinuko Yasuda 2011-04-19 05:38:36 PDT
Committed r84247: <http://trac.webkit.org/changeset/84247>
Comment 4 WebKit Review Bot 2011-04-19 05:52:06 PDT
http://trac.webkit.org/changeset/84247 might have broken Chromium Win Release
Comment 5 David Levin 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.
Comment 6 Kinuko Yasuda 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).
Comment 7 David Levin 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!
Comment 8 Kinuko Yasuda 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!
Comment 9 Darin Fisher (:fishd, Google) 2011-04-19 10:16:25 PDT
Comment on attachment 90031 [details]
Patch

API changes LGTM