| Summary: | foo(optional unsigned long long offset = 0) IDL functions don't work when called like foo(undefined) or foo() | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Myles C. Maxfield <mmaxfield> | ||||
| Component: | Bindings | Assignee: | Myles C. Maxfield <mmaxfield> | ||||
| Status: | NEW --- | ||||||
| Severity: | Normal | CC: | ahmad.saleem792, cdumez, webkit-bug-importer | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=240441 | ||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 232558 | ||||||
| Attachments: |
|
||||||
|
Description
Myles C. Maxfield
2022-05-08 16:36:21 PDT
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. 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.
Cool, thanks for trying it out. I'll try to figure out why the WebGPU stuff isn't working then. 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;
}
...
}
Pull request: https://github.com/WebKit/WebKit/pull/710 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). Yes, there is more to do here. The bug itself isn't fixed; that pull request just worked around the problem. |