Bug 189729

Summary:

JS bindings generator should support EnabledAtRuntime for static methods

Product:

WebKit

Reporter:

Justin Michaud <justin_michaud>

Component:

Bindings

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, commit-queue, esprehn+autocc, ews-watchlist, kondapallykalyan, rniwa, sam, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

189694

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none

Justin Michaud

Reported 2018-09-18 19:14:59 PDT

[EnabledAtRuntime=Feature] static void blah(); should add/remove the method when Feature is enabled or disabled.

Attachments
Patch (25.69 KB, patch) 2018-09-18 19:27 PDT, Justin Michaud	no flags	Details Formatted Diff Diff
Patch (25.65 KB, patch) 2018-09-19 10:08 PDT, Justin Michaud	no flags	Details Formatted Diff Diff
Patch (25.63 KB, patch) 2018-09-19 16:59 PDT, Justin Michaud	no flags	Details Formatted Diff Diff
Patch (25.63 KB, patch) 2018-09-20 09:53 PDT, Justin Michaud	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Justin Michaud

Comment 1 2018-09-18 19:27:31 PDT

Created attachment 350088 [details] Patch

Justin Michaud

Comment 2 2018-09-19 10:08:31 PDT

Created attachment 350129 [details] Patch

Chris Dumez

Comment 3 2018-09-19 15:20:28 PDT

Comment on attachment 350129 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=350129&action=review > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:3658 > + my $globalObjectPtr = ($globalObjectIsParam? "&globalObject" : "globalObject()"); missing space before '?'. Also, I think we usually omit the parentheses. > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:3679 > + my $globalObjectPtr = ($globalObjectIsParam? "&globalObject" : "globalObject()"); Same comments as above. > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:7199 > + push(@attributes, @{$interface->mapLike->attributes}) if $interface->mapLike; Could you clarify why maplike is unconditionally treated as runtime enabled? > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:7205 > + if ($attribute->extendedAttributes->{EnabledAtRuntime}) { Same comment as below. Should probably use NeedsRuntimeCheck() > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:7211 > + push(@operations, @{$interface->iterable->operations}) if IsKeyValueIterableInterface($interface); Could you clarify why iterable is unconditionally treated as runtime enabled? > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:7212 > + push(@operations, @{$interface->mapLike->operations}) if $interface->mapLike; ditto. > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:7213 > + push(@operations, @{$interface->serializable->operations}) if $interface->serializable; ditto. > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:7221 > + if ($operation->extendedAttributes->{EnabledAtRuntime}) { We normally do not special-case EnabledAtRuntime and instead rely on NeedsRuntimeCheck(). Also note that EnabledAtRuntime is kind of deprecated in favor of EnabledBySetting, unless you need multithreaded access. > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:7293 > + my $runtimeEnableConditionalString = GenerateRuntimeEnableConditionalString($interface, $operationOrAttribute, "true"); GenerateRuntimeEnableConditionalString supports other things that EnabledAtRuntime. For e.g. it supports EnabledBySetting (which is preferred nowadays). Therefore, it is unfortunate that GetRuntimeEnabledStaticProperties() only returns EnabledAtRuntime things.

Justin Michaud

Comment 4 2018-09-19 16:59:27 PDT

Created attachment 350161 [details] Patch

Chris Dumez

Comment 5 2018-09-20 09:18:44 PDT

Comment on attachment 350161 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=350161&action=review r=me > Source/WebCore/ChangeLog:6 > + Add support for EnabledAtRuntime to static methods in the JS bindings The description should come after... > Source/WebCore/ChangeLog:9 > + Reviewed by NOBODY (OOPS!). ... the reviewed by line.

Justin Michaud

Comment 6 2018-09-20 09:53:04 PDT

Created attachment 350219 [details] Patch

WebKit Commit Bot

Comment 7 2018-09-20 10:31:51 PDT

Comment on attachment 350219 [details] Patch Clearing flags on attachment: 350219 Committed r236266: <https://trac.webkit.org/changeset/236266>

WebKit Commit Bot

Comment 8 2018-09-20 10:31:52 PDT

All reviewed patches have been landed. Closing bug.

Radar WebKit Bug Importer

Comment 9 2018-09-20 10:33:40 PDT

<rdar://problem/44646048>

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