Bug 45165 - Expose Flags constructor if FileSystem API is Enabled
: Expose Flags constructor if FileSystem API is Enabled
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: WebCore Misc.
: 528+ (Nightly build)
: PC Mac OS X 10.5
: P2 Normal
Assigned To: Nobody
:
Depends on:
Blocks: 42903
  Show dependency treegraph
 
Reported: 2010-09-02 23:24 PDT by Kinuko Yasuda
Modified: 2010-09-13 16:15 PDT (History)
4 users (show)

See Also:


Attachments
Patch (2.56 KB, patch)
2010-09-03 00:06 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (12.15 KB, patch)
2010-09-09 18:41 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (12.94 KB, patch)
2010-09-09 18:51 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (13.73 KB, patch)
2010-09-09 20:22 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (21.31 KB, patch)
2010-09-09 21:49 PDT, Kinuko Yasuda
no flags Details | Formatted Diff | Diff
Patch (28.46 KB, patch)
2010-09-09 22:57 PDT, Kinuko Yasuda
dumi: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kinuko Yasuda 2010-09-02 23:24:09 PDT
Expose Flags constructor if FileSystem API is Enabled.
http://dev.w3.org/2009/dap/file-system/file-dir-sys.html#the-flags-interface
Comment 1 Kinuko Yasuda 2010-09-03 00:06:24 PDT
Created attachment 66475 [details]
Patch
Comment 2 Dumitru Daniliuc 2010-09-03 03:43:38 PDT
is there a way to reuse fileSystemEnabled()? i know that [EnableAtRuntime] requires a getter with a specific name, but i think in this case it might be better to write some custom code to reuse fileSystemEnabled() (if possible).
Comment 3 Kinuko Yasuda 2010-09-07 00:47:15 PDT
Comment on attachment 66475 [details]
Patch

Removing this from the review queue for now as we may not need to expose Flags (sent an email about this to public-webapps).
(Thanks dumi for your review comment!)
Comment 4 Kinuko Yasuda 2010-09-09 18:41:07 PDT
Created attachment 67133 [details]
Patch

Reviving this patch.
Comment 5 Kinuko Yasuda 2010-09-09 18:51:44 PDT
Created attachment 67135 [details]
Patch
Comment 6 Kinuko Yasuda 2010-09-09 20:22:12 PDT
Created attachment 67147 [details]
Patch
Comment 7 Kinuko Yasuda 2010-09-09 20:26:53 PDT
(In reply to comment #2)
> is there a way to reuse fileSystemEnabled()? i know that [EnableAtRuntime] requires a getter with a specific name, but i think in this case it might be better to write some custom code to reuse fileSystemEnabled() (if possible).

I added custom getter for V8 (since EnableAtRuntime is valid only in V8).  This doesn't stop exposing Flags constructor Window though -- I wasn't able to find a way to hook the custom code from the DOMWindow's prototype template constructor method.
Comment 8 Kinuko Yasuda 2010-09-09 21:49:58 PDT
Created attachment 67150 [details]
Patch

Modified the code generator to take additional parameter to EnableAtRuntime so that we can reuse fileSystemEnabled().
Comment 9 Kinuko Yasuda 2010-09-09 22:57:49 PDT
Created attachment 67159 [details]
Patch

Rebased.
Comment 10 Dumitru Daniliuc 2010-09-13 12:45:28 PDT
Comment on attachment 67159 [details]
Patch

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

r=me. please address the comment before landing.

> WebCore/bindings/scripts/CodeGeneratorV8.pm:3395
> +    if (length($signature->extendedAttributes->{"EnabledAtRuntime"}) > 1) {
to keep this function consistent with all other helper functions, i think we should change its format to:

return "RuntimeEnabledFeatures::" . $codeGenerator->WK_lcfirst($signature->extendedAttributes->{"EnabledAtRuntime"}) . "Enabled" if ($signature->extendedAttributes->{"EnabledAtRuntime"} ne "");
return <the default name>;
Comment 11 Kinuko Yasuda 2010-09-13 14:36:38 PDT
(In reply to comment #10)
> (From update of attachment 67159 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=67159&action=prettypatch
>
> > WebCore/bindings/scripts/CodeGeneratorV8.pm:3395
> > +    if (length($signature->extendedAttributes->{"EnabledAtRuntime"}) > 1) {
> to keep this function consistent with all other helper functions, i think we should change its format to:
> 
> return "RuntimeEnabledFeatures::" . $codeGenerator->WK_lcfirst($signature->extendedAttributes->{"EnabledAtRuntime"}) . "Enabled" if ($signature->extendedAttributes->{"EnabledAtRuntime"} ne "");
> return <the default name>;

As extendedAttributes->{"EnabledAtRuntime"} has value 1 when it's set but has no values, I'm going to make the if expr look like following.   (Could you ping me if it doesn't look good.)  Thanks,

> return ... if ($signature->extendedAttributes->{"EnabledAtRuntime"}  && $signature->extendedAttributes->{"EnabledAtRuntime"} != 1);
Comment 12 Kinuko Yasuda 2010-09-13 16:15:11 PDT
Committed r67421: <http://trac.webkit.org/changeset/67421>