Bug 240219 – foo(optional unsigned long long offset = 0) IDL functions don't work when called like foo(undefined) or foo()

• Component: Bindings
• Status: NEW

[Myles C. Maxfield]

In WebGPU, see mapAsync() and getMappedRange(). The C++ version of these functions never get called when the optional parameters with default values are specified by JS as undefined or omitted.

[Chris Dumez]

I find his very surprising. Pretty sure we already have similar IDL in the code Base. I’d expect our generated code to call the implementation function with 0 if the parameter is omitted or undefined.

[Chris Dumez]

I have just double-checked here and things seem to work as expected. I validated with the following patch:
```
diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
index 0636bd5b4363..370a789c0a9e 100644
--- a/Source/WebCore/dom/Document.cpp
+++ b/Source/WebCore/dom/Document.cpp
@@ -9202,6 +9202,11 @@ bool Document::isSameSiteForCookies(const URL& url) const
     return domain.matches(url);
 }
 
+void Document::testOptionalUnsignedLongLong(uint64_t param)
+{
+    WTFLogAlways("CHRIS: Document::testOptionalUnsignedLongLong(%llu)", param);
+}
+
 } // namespace WebCore
 
 #undef DOCUMENT_RELEASE_LOG
diff --git a/Source/WebCore/dom/Document.h b/Source/WebCore/dom/Document.h
index 866abc6d7986..fcc1319cadb0 100644
--- a/Source/WebCore/dom/Document.h
+++ b/Source/WebCore/dom/Document.h
@@ -699,6 +699,8 @@ public:
 
     void cancelParsing();
 
+    void testOptionalUnsignedLongLong(uint64_t);
+
     ExceptionOr<void> write(Document* entryDocument, SegmentedString&&);
     WEBCORE_EXPORT ExceptionOr<void> write(Document* entryDocument, FixedVector<String>&&);
     WEBCORE_EXPORT ExceptionOr<void> writeln(Document* entryDocument, FixedVector<String>&&);
diff --git a/Source/WebCore/dom/Document.idl b/Source/WebCore/dom/Document.idl
index b5cc4978066a..db9726d66df7 100644
--- a/Source/WebCore/dom/Document.idl
+++ b/Source/WebCore/dom/Document.idl
@@ -94,6 +94,8 @@ typedef (HTMLScriptElement or SVGScriptElement) HTMLOrSVGScriptElement;
 
     // Non standard: It has been dropped from Blink already.
     RenderingContext? getCSSCanvasContext(DOMString contextId, DOMString name, long width, long height);
+
+    undefined testOptionalUnsignedLongLong(optional unsigned long long parameter = 0);
 };
 
 enum DocumentReadyState { "loading", "interactive", "complete" };
```

document.testOptionalUnsignedLongLong() -> implementation called with 0
document.testOptionalUnsignedLongLong(0) -> implementation called with 0
document.testOptionalUnsignedLongLong(1) -> implementation called with 1
document.testOptionalUnsignedLongLong(undefined) -> implementation called with 0

Works as expected IMO.

[Myles C. Maxfield]

Cool, thanks for trying it out. I'll try to figure out why the WebGPU stuff isn't working then.

[Myles C. Maxfield]

Created attachment 459381 [details]
Test file

Here is a test file demonstrating the problem. (You have to open it in DumpRenderTree; it's not hooked up into WK2 yet, and is intentionally disabled in Minibrowser.) The IDL has:

Promise<undefined> mapAsync(GPUMapModeFlags mode, optional GPUSize64 offset = 0, optional GPUSize64 size);

And if you call it like

buffer.mapAsync(GPUMapMode.WRITE);

then our generated bindings code does this:

auto offset = convert<IDLEnforceRangeAdaptor<IDLUnsignedLongLong>>(*lexicalGlobalObject, argument1.value());

which gets sent to:

double JSValue::toNumberSlowCase(JSGlobalObject* globalObject) const
{
    ...
    return isUndefined() ? PNaN : 0; // null and false both convert to 0.
}

and then sent to

static double enforceRange(JSGlobalObject& lexicalGlobalObject, double x, double minimum, double maximum)
{
    ...
    if (std::isnan(x) || std::isinf(x)) {
        throwTypeError(&lexicalGlobalObject, scope, rangeErrorString(x, minimum, maximum));
        return 0;
    }
    ...
}

[Radar WebKit Bug Importer]

<rdar://problem/93317646>

[Myles C. Maxfield]

Pull request: https://github.com/WebKit/WebKit/pull/710

[Ahmad Saleem]

In Pull Request from Comment 06, there is mention that it was landed as part of following:

https://trac.webkit.org/changeset/294373/webkit

Is anything else needed here?

BY THE WAY - the test file show "failure" in all browsers (Chrome Canary 107, Firefox Nightly 106 and Safari Technology Preview 152).

[Myles C. Maxfield]

Yes, there is more to do here. The bug itself isn't fixed; that pull request just worked around the problem.