Bug 45165 - Expose Flags constructor if FileSystem API is Enabled
: Expose Flags constructor if FileSystem API is Enabled
Status: RESOLVED FIXED
: WebKit
WebCore Misc.
: 528+ (Nightly build)
: PC Mac OS X 10.5
: P2 Normal
Assigned To:
:
:
:
: 42903
  Show dependency treegraph
 
Reported: 2010-09-02 23:24 PST by
Modified: 2010-09-13 16:15 PST (History)


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


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2010-09-02 23:24:09 PST
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 From 2010-09-03 00:06:24 PST -------
Created an attachment (id=66475) [details]
Patch
------- Comment #2 From 2010-09-03 03:43:38 PST -------
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 From 2010-09-07 00:47:15 PST -------
(From update of attachment 66475 [details])
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 From 2010-09-09 18:41:07 PST -------
Created an attachment (id=67133) [details]
Patch

Reviving this patch.
------- Comment #5 From 2010-09-09 18:51:44 PST -------
Created an attachment (id=67135) [details]
Patch
------- Comment #6 From 2010-09-09 20:22:12 PST -------
Created an attachment (id=67147) [details]
Patch
------- Comment #7 From 2010-09-09 20:26:53 PST -------
(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 From 2010-09-09 21:49:58 PST -------
Created an attachment (id=67150) [details]
Patch

Modified the code generator to take additional parameter to EnableAtRuntime so that we can reuse fileSystemEnabled().
------- Comment #9 From 2010-09-09 22:57:49 PST -------
Created an attachment (id=67159) [details]
Patch

Rebased.
------- Comment #10 From 2010-09-13 12:45:28 PST -------
(From update of attachment 67159 [details])
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 From 2010-09-13 14:36:38 PST -------
(In reply to comment #10)
> (From update of attachment 67159 [details] [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 From 2010-09-13 16:15:11 PST -------
Committed r67421: <http://trac.webkit.org/changeset/67421>