[Blink Bindings] Returning an undefined

[Rouslan Solomakhin]

Hello Bindings Team!

Quick question: Is there a way to return an "undefined" from an attribute defined in WebIDL?

This attribute for example: readonly attribute USVString topOrigin;

For some context, we would like to run an Origin Trial that removes some fields from a service worker event. However, an Origin Trial can only enable an already-enabled-feature, so an OT can only add fields with [RuntimeEnabled=FeatureFlag], not remove them.

The ideal solution would be to have something like [RuntimeDisalbed=FeatureFlag] or return v8::Undefined() from the .cc file, but neither of these options are implemented.

The most feasible option as of right now is to return an empty string from the field in the .cc file, but that means feature detection would work only with if (event.topOrigin) or if (event.topOrigin !== ""). However, feature detection with if (event.topOrigin !== undefined) would not work with this option.

More details can be found in Proposal to Fix the CanMakePayment Identity OT.

(Thank you, @Stephen Mcgruer for recommending that I reach out here.)

Cheers,

Rouslan

[Yuki Shiino]

Hi Rouslan,

Thanks for reaching us. The bindings layer just checks RuntimeEnabledFeatures::FooEnabled(). So, if you can make FooEnabled() false by default and change it to true as needed by the time the OT flags are finalized, then the bindings layer just installs the property only when FooEnabled() is true. I think chasej@ should be a good contact person to ask how to do this / when the OT flags are finalized.

About returning undefined, you can change the IDL type of the attribute to `any` so that you can return anything, including ES undefined. (Drawback: you need to perform a type check on your side because web author scripts can pass anything other than a string or undefined.)  The Blink type corresponding to `any` IDL type is ScriptValue. You can create ScriptValue(isolate, v8::Undefined(isolate)).

Cheers,

Yuki Shiino

2022年10月11日(火) 22:49 Rouslan Solomakhin <rou...@chromium.org>:

[Rouslan Solomakhin]

Thank you for the quick and awesome reply!

> The Blink type corresponding to `any` IDL type is ScriptValue. You can create ScriptValue(isolate, v8::Undefined(isolate)).

Are there preferred ways to convert WTF::String and blink::HeapVector into a ScriptValue? (For the cases where the OT is turned off and the actual value needs to be returned.)

> I think chasej@ should be a good contact person to ask how to do this / when the OT flags are finalized.

I have talked to chasej@ and that's what caused me to come up with the design doc.

Cheers,

Rouslan

[Stephen Mcgruer]

> I have talked to chasej@ and that's what caused me to come up with the design doc.

To expand on this slightly; the Origin Trial team advised us (as I understand it) that Origin Trials can only flip a flag from off --> on, and not from on --> off. In general this isn't a problem as one can just invert the logic if necessary, but for the IDL RuntimeEnabled attribute specifically that doesn't work - there's no way (as Rouslan said) to have RuntimeDisabled=FooFlag.

(Another solution would be for OTs to support on --> off, but I believe they stated it was generally hard to do).

[Rouslan Solomakhin]

> Are there preferred ways to convert WTF::String and blink::HeapVector into a ScriptValue?

Answered my own question, so recording it here for posterity:

bool success = ToV8Traits<IDLUSVString>::ToV8(script_state, string_value).ToLocal(&result);

bool success = ToV8Traits<IDLArray<PaymentMethodData>>::ToV8(script_state, heap_vector_value).ToLocal(&result);

[Yuki Shiino]

That's it > ToV8Traits. ToV8Traits::ToV8 throws a TypeError (or RangeError, etc. depending on cases) when it fails. So, you'd like to exit early in that case. I'm very happy to review type checks if you're going to use `any` type.

Other random ideas here.

a) You can tweak the code generation of RuntimeEnabledFeatures (or RuntimeEnabledFeaturesBase) and define a static member function FooRevertedEnabled() { return !FooEnabled(); }.

b) You can tweak ContextFeatureSettings (See [ContextEnabled=MojoJS] and ContextFeatureSettings::isMojoEnabled()). You can (technically) add a new member function isFooRevertedEnabled() { return ! RuntimeEnabledFeatures or something else ::isFooEnabled(); }.

Note that RuntimeEnabledFeatures(Base) is auto-generated by a script based on runtime_enabled_features.json5. It will look tricky (IMHO), but you can add something hard-coded in:

//third_party/blink/renderer/build/scripts/templates/runtime_enabled_features.h.tmpl

//third_party/blink/renderer/build/scripts/templates/runtime_enabled_features.cc.tmpl

Note that ContextFeatureSettings itself is not so welcome from my perspective (RuntimeEnabledFeatures should be used instead) and it's used only for Mojo so far. But you can technically add a hack there.

Anyway, you need to fix the flag value until when the bindings layer installs context-dependent properties. Otherwise, the bindings layer does / does not install the property based on the flag value at that moment.

Cheers,

Yuki Shiino

2022年10月12日(水) 3:08 Rouslan Solomakhin <rou...@chromium.org>:

[Rouslan Solomakhin]

Thank you very much for all of the advice! I've implemented the "any" option in https://crrev.com/c/3955818 and it's working quite well. Next I will be exploring a few of your other suggestions, timeboxed to a couple of days.

Have a great weekend!

[Rouslan Solomakhin]

Hi everyone,

After considering all alternatives, it seems most natural to return a ScriptValue(ioslate, v8::Undefined(isolate)) value in .cc file and "any" in IDL. Let's go ahead with that solution. -- Happy to discuss further if needed.

Cheers,

Rouslan