Determine how set() will work for "flag" mode + scope

[mgiuca]

Follow-up to Chromium bug 1000968.

The current informal definition of the set() method has two optional parameters:

Badge.set([contents], [options]): Sets the badge for the scope in options to contents (an integer), or to "flag" if contents is omitted. If contents is 0, clears the badge for the given scope.

The intention here is that if you pass one argument, if it's an integer, it's contents and the scope is default. If it's a dictionary, it's options and the badge is set to "flag". I think that would feel natural to users and it's unambiguous, but it might be difficult to express that in WebIDL, and/or be considered bad API design.

I can't think of a precedent for this sort of "arity overloading" in the Web platform. The closest thing that comes to my mind is Python's range function, which takes (start, stop), but if you only supply one argument, it is accepted as stop (not start).

Edit: Examples of the four different ways you'd be able to supply parameters to this function:

1. Badge.set() -- sets badge to "flag" with the default scope.
2. Badge.set(num) -- sets badge to num with the default scope.
3. Badge.set(num, {scope: url}) -- sets badge to num with scope url.
4. Badge.set({scope: url}) -- (in question): sets badge to "flag" with scope url.

The question is whether this is reasonable for the user to understand that if you pass a dictionary as the sole argument that it be treated as options (not content), and also whether this is acceptable from an IDL and implementation perspective.

If the above can't be done, we need another solution, such as being able to pass an explicit value meaning "flag" (e.g., true or null) to that first argument.

Currently, due to the declaration of that parameter in WebIDL as unsigned long long, false and null are converted to 0, which clears the badge, and true is converted to 1 which sets it to "1". There is no way to pass an explicit argument that sets the badge to "flag".

Possibly something to ask TAG about.

[fallaciousreasoning]

FWIW

set(optional unsigned long long contents, optional BageOptions options);

seems to work in Chromium

https://cs.chromium.org/chromium/src/third_party/blink/renderer/modules/badging/experimental_badge.idl?q=f:badge.idl&sq=package:chromium&g=0&l=15.

As to style, I think you're right, we should talk to the TAG folks.

[g-ortuno]

In bluetooth we do:

typedef (DOMString or unsigned long) BluetoothServiceUUID;
getPrimaryServices(optional BluetoothServiceUUID);

Couldn't you do something similar?

typedef (unsigned long long or BadgeOptions) SetParam;
set(optional SetParam);

[g-ortuno]

There might be a way to avoid the typedef depending on how we parse the IDL.

[mgiuca]

FWIW

set(optional unsigned long long contents, optional BageOptions options);

seems to work in Chromium

It compiles but it doesn't work as intended.

https://crbug.com/1000968

Uncaught TypeError: Failed to execute 'set' on 'ExperimentalBadge': Value is not of type 'unsigned long long'.

Both parameters are optional (as in, you can pass 0 arguments), but if you do pass one, it always goes to the contents parameter. I assume this is correct according to WebIDL (it would be a bit crazy to expect this to automatically decide which parameter to fill based on the type).

[mgiuca]

@g-ortuno Yes, you can certainly have an "or" as a parameter (and the typedef is not required). But it's more complex than simply allowing one of two types in the first parameter slot, since you also need to prevent the dictionary from being passed as the first and second parameter. The closest approximation is:

static void set(optional (([EnforceRange] unsigned long long) or BadgeOptions) contentsOrOptions,
                optional BadgeOptions options);

Statically, this would allow:

1. set()
2. set(num)
3. set({scope: url})
4. set(num, {scope: url})
5. set({scope: url1}, {scope: url2})

And then we need code inside the method to throw an error if contentsOrOptions is an object and options is supplied. It can be done, but it's a bit messy.

[mgiuca]

Wait, it turns out WebIDL has overloading. Good, then we should simply be able to define it like this:

// Set badge to "flag".
static void set();
// Set badge to a number, optionally giving an options dictionary.
static void set([EnforceRange] unsigned long long contents,
                optional BadgeOptions options);
// Set badge to "flag", with an options dictionary.
static void set(BadgeOptions options);

But we have to read this very carefully to see how the type matching would work. Thanks @g-ortuno .

[mgiuca]

Interesting, on the alternative approach where we ask for an explicit value in the first argument position, I think undefined should "just work":

Badge.set(undefined, {scope: url});

Based on a cursory reading of the (admittedly very long) JS-to-IDL overload resolution algorithm (which is the algorithm responsible for converting a JS argument list into WebIDL types), undefined should just select the default value when passed to an optional parameter, so the above should set a flag badge with a scope. But it doesn't work. Same as above:

Uncaught TypeError: Failed to execute 'set' on 'ExperimentalBadge': Value is not of type 'unsigned long long'.

This might be a bug in Chrome's IDL parser? (Edit: Filed bug on Chrome.)

Badge.set(undefined) sets a flag badge, but Badge.set(undefined, {scope: url}) gives a TypeError converting undefined to a long long, so ... maybe there's a bug in Chrome's overload resolution algorithm.

Nevertheless, I would prefer to allow you to pass the dictionary to the first parameter if that's possible.

[fallaciousreasoning]

Note: When we've made the changes in #55 this will no longer be relevant.

[mgiuca]

Scope has been removed.