Bug 225537 - IPC testing API should have the ability to create and receive IPC::Semaphore
Summary: IPC testing API should have the ability to create and receive IPC::Semaphore
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ryosuke Niwa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-05-07 13:20 PDT by Ryosuke Niwa
Modified: 2021-05-08 23:38 PDT (History)
4 users (show)

See Also:

Attachments
Patch (24.13 KB, patch)
2021-05-07 13:35 PDT, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Patch for landing (25.05 KB, patch)
2021-05-07 14:06 PDT, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Patch for landing (24.18 KB, patch)
2021-05-07 14:30 PDT, Ryosuke Niwa 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Ryosuke Niwa 2021-05-07 13:20:02 PDT 
In order to support DisplayList fuzzing, we need the ability to create & receive IPC::Semaphore.
Comment 1 Ryosuke Niwa 2021-05-07 13:35:07 PDT 
Created attachment 428028 [details]
Patch
Comment 2 Wenson Hsieh 2021-05-07 13:51:41 PDT 
Comment on attachment 428028 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=428028&action=review

> Source/WebKit/WebProcess/WebPage/IPCTestingAPI.cpp:551
> +    RemoteRenderingBackendCreationParameters parameters { *identifier, WTFMove(semaphore), *pageProxyID, *pageID };
> +    encoder << parameters;
> +    semaphoreObject->exchange(WTFMove(semaphore));

We seem to `WTFMove(semaphore)` a couple of times here…is that intentional?
Comment 3 Ryosuke Niwa 2021-05-07 14:05:30 PDT 
Comment on attachment 428028 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=428028&action=review

>> Source/WebKit/WebProcess/WebPage/IPCTestingAPI.cpp:551
>> +    semaphoreObject->exchange(WTFMove(semaphore));
> 
> We seem to `WTFMove(semaphore)` a couple of times here…is that intentional?

As we discussed, this should have been WTFMove(parameters.resumeDisplayListSemaphore), not WTFMove(semaphore).

> Tools/TestWebKitAPI/Tests/WebKitCocoa/IPCTestingAPI.mm:225
> +        "const result = IPC.sendSyncMessage('GPU', 123, IPC.messages.RemoteRenderingBackend_SemaphoreForGetImageData.name, 100, []);"

I'd add a call to signal() here so that the above bug will be caught with a debug assertion in IPC::Semaphore::signal.
Comment 4 Ryosuke Niwa 2021-05-07 14:06:46 PDT 
Created attachment 428032 [details]
Patch for landing
Comment 5 Ryosuke Niwa 2021-05-07 14:30:21 PDT 
Comment on attachment 428032 [details]
Patch for landing

View in context: https://bugs.webkit.org/attachment.cgi?id=428032&action=review

> WebKit.xcworkspace/xcshareddata/xcschemes/All Source.xcscheme:130
> +               BuildableName = "Framework &amp; XPC Services"

Oh oops :(
Comment 6 Ryosuke Niwa 2021-05-07 14:30:33 PDT 
Created attachment 428037 [details]
Patch for landing
Comment 7 Ryosuke Niwa 2021-05-07 14:32:54 PDT 
Comment on attachment 428037 [details]
Patch for landing

Clearing flags on attachment: 428037

Committed r277199 (237471@main): <https://commits.webkit.org/237471@main>
Comment 8 Ryosuke Niwa 2021-05-07 14:32:56 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2021-05-07 14:33:18 PDT 
<rdar://problem/77673684>