Skip to content

Commit 8ebae84

Browse files
kkinnunen-applekkinnunen-apple
committed
IPC testing API is unable to reply with nullopt optional
https://bugs.webkit.org/show_bug.cgi?id=275409 rdar://129669346 Reviewed by Cameron McCormack. Decoding std::optional would return empty JSValue (JSValue()) if the optional was std::nullopt. This would be interpreted as a decoding failure. Fix this by returning the more natural value, jsUndefined value. Fix the result array construction to not use jsUndefined to signify that the value should be a array buffer containing the data. Use empty JSValue to signify this. Use exception scope to detect when JS exception would cancel the decoding as a decoding failure. Decoding failures would use the already cleared exception as the result JS object, causing a crash. Fix by first constructing the JS value, then clearing the exception from the scope. Add decoder for WebCore::ExceptionData, to be used in testing the message this problem was noted. * LayoutTests/ipc/async-with-reply-optional.html: Added. * Source/WebKit/Platform/IPC/JSIPCBinding.cpp: (IPC::jsValueForDecodedArgumentValue): (IPC::putJSValueForDecodedArgumentAtIndexOrArrayBufferIfUndefined): Deleted. * Source/WebKit/Platform/IPC/JSIPCBinding.h: (IPC::jsValueForDecodedArgumentValue): (IPC::putJSValueForDecodeArgumentInArray): * Source/WebKit/Shared/IPCTester.cpp: (WebKit::IPCTester::asyncOptionalExceptionData): * Source/WebKit/Shared/IPCTester.h: * Source/WebKit/Shared/IPCTester.messages.in: * Source/WebKit/WebProcess/WebPage/IPCTestingAPI.cpp: (WebKit::IPCTestingAPI::jsSendWithAsyncReply): Canonical link: https://commits.webkit.org/279999@main
1 parent 2051bf7 commit 8ebae84

File tree

8 files changed

+92
-23
lines changed

8 files changed

+92
-23
lines changed

‎LayoutTests/ipc/async-with-reply-optional-expected.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
2+
PASS Test that async messages can reply with optionals
3+

‎LayoutTests/ipc/async-with-reply-optional.html

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
<!doctype html><!-- webkit-test-runner [ IPCTestingAPIEnabled=true ] -->
2+
<title>Test that async messages can reply with optionals</title>
3+
<script src="../resources/testharness.js"></script>
4+
<script src="../resources/testharnessreport.js"></script>
5+
<script src="../resources/ipc.js"></script>
6+
<body>
7+
<script>
8+
async function runTest() {
9+
const testerID = 0;
10+
const IPCTester_AsyncOptionalExceptionData = IPC.messages.IPCTester_AsyncOptionalExceptionData.name;
11+
const IPCTester_AsyncOptionalExceptionDataReply = IPC.messages.IPCTester_AsyncOptionalExceptionDataReply.name;
12+
13+
for (const processTarget of IPC.processTargets) {
14+
// Test starts here.
15+
let results = { };
16+
17+
let connection = IPC.connectionForProcessTarget(processTarget);
18+
let result1 = IPC.sendWithPromisedReply(processTarget, testerID, IPCTester_AsyncOptionalExceptionData, [{type: 'bool', value: 0}]);
19+
let result2 = IPC.sendWithPromisedReply(processTarget, testerID, IPCTester_AsyncOptionalExceptionData, [{type: 'bool', value: 1}]);
20+
let result = await result1;
21+
assert_equals(typeof result, "object");
22+
assert_equals(typeof result.arguments[0], "undefined");
23+
assert_equals(result.arguments[1].type, "String");
24+
assert_equals(result.arguments[1].value, "b");
25+
result = await result2;
26+
assert_equals(typeof result, "object");
27+
assert_equals(result.arguments[0].type, "ExceptionData");
28+
assert_equals(result.arguments[0].code, "WrongDocumentError");
29+
assert_equals(result.arguments[0].message, "m");
30+
assert_equals(result.arguments[1].type, "String");
31+
assert_equals(result.arguments[1].value, "a");
32+
}
33+
done();
34+
}
35+
36+
setup({ single_test: true });
37+
if (window.IPC)
38+
runTest();
39+
else
40+
done();
41+
42+
</script>
43+
</body>

‎Source/WebKit/Platform/IPC/JSIPCBinding.cpp

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030

3131
#include <JavaScriptCore/JSCInlines.h>
3232
#include <JavaScriptCore/ObjectConstructor.h>
33+
#include <WebCore/DOMException.h>
34+
#include <WebCore/ExceptionData.h>
3335
#include <WebCore/FloatRect.h>
3436
#include <WebCore/IntRect.h>
3537
#include <WebCore/RegistrableDomain.h>
@@ -194,20 +196,21 @@ JSC::JSValue jsValueForDecodedArgumentValue(JSC::JSGlobalObject* globalObject, W
194196
return jsValueForDecodedArgumentRect(globalObject, value, "FloatRect"_s);
195197
}
196198

197-
bool putJSValueForDecodedArgumentAtIndexOrArrayBufferIfUndefined(JSC::JSGlobalObject* globalObject, JSC::JSArray* array, unsigned index, JSC::JSValue value, std::span<const uint8_t> buffer)
199+
template<>
200+
JSC::JSValue jsValueForDecodedArgumentValue(JSC::JSGlobalObject* globalObject, WebCore::ExceptionData&& exceptionData)
198201
{
199-
auto scope = DECLARE_THROW_SCOPE(globalObject->vm());
200-
201-
if (value.isUndefined()) {
202-
auto arrayBuffer = JSC::ArrayBuffer::create(buffer);
203-
if (auto* structure = globalObject->arrayBufferStructure(arrayBuffer->sharingMode()))
204-
value = JSC::JSArrayBuffer::create(Ref { globalObject->vm() }, structure, WTFMove(arrayBuffer));
205-
}
206-
207-
array->putDirectIndex(globalObject, index, value);
208-
RETURN_IF_EXCEPTION(scope, false);
209-
210-
return true;
202+
auto& vm = globalObject->vm();
203+
auto scope = DECLARE_THROW_SCOPE(vm);
204+
auto* object = JSC::constructEmptyObject(globalObject, globalObject->objectPrototype());
205+
RETURN_IF_EXCEPTION(scope, JSC::JSValue());
206+
object->putDirect(vm, JSC::Identifier::fromString(vm, "type"_s), JSC::jsNontrivialString(vm, "ExceptionData"_s));
207+
RETURN_IF_EXCEPTION(scope, JSC::JSValue());
208+
auto& message = exceptionData.message;
209+
object->putDirect(vm, JSC::Identifier::fromString(vm, "message"_s), message.isNull() ? JSC::jsNull() : JSC::jsString(vm, message));
210+
RETURN_IF_EXCEPTION(scope, JSC::JSValue());
211+
object->putDirect(vm, JSC::Identifier::fromString(vm, "code"_s), JSC::jsNontrivialString(vm, WebCore::DOMException::description(exceptionData.code).name));
212+
RETURN_IF_EXCEPTION(scope, JSC::JSValue());
213+
return object;
211214
}
212215

213216
}

‎Source/WebKit/Platform/IPC/JSIPCBinding.h

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ namespace WebCore {
5050
class FloatRect;
5151
class IntRect;
5252
class RegistrableDomain;
53+
struct ExceptionData;
5354

5455
}
5556

@@ -60,7 +61,8 @@ class Semaphore;
6061
template<typename T, std::enable_if_t<!std::is_arithmetic<T>::value && !std::is_enum<T>::value>* = nullptr>
6162
JSC::JSValue jsValueForDecodedArgumentValue(JSC::JSGlobalObject*, T&&)
6263
{
63-
return JSC::jsUndefined();
64+
// Report that we don't recognize this type.
65+
return JSC::JSValue();
6466
}
6567

6668
template<> JSC::JSValue jsValueForDecodedArgumentValue(JSC::JSGlobalObject*, String&&);
@@ -109,6 +111,7 @@ JSC::JSValue jsValueForDecodedArgumentValue(JSC::JSGlobalObject* globalObject, A
109111

110112
template<> JSC::JSValue jsValueForDecodedArgumentValue(JSC::JSGlobalObject*, WebCore::IntRect&&);
111113
template<> JSC::JSValue jsValueForDecodedArgumentValue(JSC::JSGlobalObject*, WebCore::FloatRect&&);
114+
template<> JSC::JSValue jsValueForDecodedArgumentValue(JSC::JSGlobalObject*, WebCore::ExceptionData&&);
112115

113116
template<typename U>
114117
JSC::JSValue jsValueForDecodedArgumentValue(JSC::JSGlobalObject* globalObject, OptionSet<U>&& value)
@@ -126,12 +129,10 @@ template<typename U>
126129
JSC::JSValue jsValueForDecodedArgumentValue(JSC::JSGlobalObject* globalObject, std::optional<U>&& value)
127130
{
128131
if (!value)
129-
return JSC::JSValue();
132+
return JSC::jsUndefined();
130133
return jsValueForDecodedArgumentValue(globalObject, std::forward<U>(*value));
131134
}
132135

133-
bool putJSValueForDecodedArgumentAtIndexOrArrayBufferIfUndefined(JSC::JSGlobalObject*, JSC::JSArray*, unsigned index, JSC::JSValue, std::span<const uint8_t> buffer);
134-
135136
template<typename... Elements>
136137
std::optional<JSC::JSValue> putJSValueForDecodeArgumentInArray(JSC::JSGlobalObject*, IPC::Decoder&, JSC::JSArray*, size_t currentIndex, std::tuple<Elements...>*);
137138

@@ -150,12 +151,19 @@ std::optional<JSC::JSValue> putJSValueForDecodeArgumentInArray(JSC::JSGlobalObje
150151
if (!value)
151152
return std::nullopt;
152153

154+
auto scope = DECLARE_THROW_SCOPE(globalObject->vm());
153155
auto jsValue = jsValueForDecodedArgumentValue(globalObject, WTFMove(*value));
154-
if (jsValue.isEmpty())
155-
return jsValue;
156-
157-
auto span = decoder.span().subspan(startingBufferOffset, decoder.currentBufferOffset() - startingBufferOffset);
158-
putJSValueForDecodedArgumentAtIndexOrArrayBufferIfUndefined(globalObject, array, currentIndex, jsValue, span);
156+
RETURN_IF_EXCEPTION(scope, std::nullopt);
157+
if (jsValue.isEmpty()) {
158+
// Create array buffers out of types we don't recognize.
159+
auto span = decoder.span().subspan(startingBufferOffset, decoder.currentBufferOffset() - startingBufferOffset);
160+
auto arrayBuffer = JSC::ArrayBuffer::create(span);
161+
if (auto* structure = globalObject->arrayBufferStructure(arrayBuffer->sharingMode()))
162+
jsValue = JSC::JSArrayBuffer::create(Ref { globalObject->vm() }, structure, WTFMove(arrayBuffer));
163+
RETURN_IF_EXCEPTION(scope, std::nullopt);
164+
}
165+
array->putDirectIndex(globalObject, currentIndex, jsValue);
166+
RETURN_IF_EXCEPTION(scope, std::nullopt);
159167

160168
std::tuple<Elements...>* dummyArguments = nullptr;
161169
return putJSValueForDecodeArgumentInArray<Elements...>(globalObject, decoder, array, currentIndex + 1, dummyArguments);

‎Source/WebKit/Shared/IPCTester.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include "IPCTesterReceiverMessages.h"
3636
#include "IPCUtilities.h"
3737

38+
#include <WebCore/ExceptionData.h>
3839
#include <atomic>
3940
#include <dlfcn.h>
4041
#include <stdio.h>
@@ -218,6 +219,15 @@ void IPCTester::syncPingEmptyReply(IPC::Connection&, uint32_t value, CompletionH
218219
completionHandler();
219220
}
220221

222+
void IPCTester::asyncOptionalExceptionData(IPC::Connection&, bool sendEngaged, CompletionHandler<void(std::optional<WebCore::ExceptionData>, String)>&& completionHandler)
223+
{
224+
if (sendEngaged) {
225+
completionHandler(WebCore::ExceptionData { WebCore::ExceptionCode::WrongDocumentError, "m"_s }, "a"_s);
226+
return;
227+
}
228+
completionHandler(std::nullopt, "b"_s);
229+
}
230+
221231
void IPCTester::stopIfNeeded()
222232
{
223233
if (m_testQueue) {

‎Source/WebKit/Shared/IPCTester.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ class IPCTester final : public IPC::MessageReceiver {
7474
void asyncPing(uint32_t value, CompletionHandler<void(uint32_t)>&&);
7575
void syncPing(IPC::Connection&, uint32_t value, CompletionHandler<void(uint32_t)>&&);
7676
void syncPingEmptyReply(IPC::Connection&, uint32_t value, CompletionHandler<void()>&&);
77+
void asyncOptionalExceptionData(IPC::Connection&, bool sendEngaged, CompletionHandler<void(std::optional<WebCore::ExceptionData>, String)>&&);
7778

7879
void stopIfNeeded();
7980

‎Source/WebKit/Shared/IPCTester.messages.in

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ messages -> IPCTester NotRefCounted {
4040
SyncPingEmptyReply(uint32_t value) -> () Synchronous
4141

4242
SendAsyncMessageToReceiver(uint32_t arg0)
43+
AsyncOptionalExceptionData(bool sendEngaged) -> (std::optional<WebCore::ExceptionData> exceptionData, String other)
4344
}
4445

4546
#endif

‎Source/WebKit/WebProcess/WebPage/IPCTestingAPI.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -527,8 +527,8 @@ static JSValueRef jsSendWithAsyncReply(IPC::Connection& connection, uint64_t des
527527
jsResult = jsResultFromReplyDecoder(globalObject, messageName, *replyDecoder);
528528
JSValueRef arguments[1] = { nullptr };
529529
if (auto* exception = scope.exception()) {
530-
scope.clearException();
531530
arguments[0] = toRef(globalObject, exception);
531+
scope.clearException();
532532
} else
533533
arguments[0] = toRef(globalObject, jsResult);
534534
JSObjectCallAsFunction(context, callback, callback, 1, arguments, nullptr);

0 commit comments

Comments
 (0)