Regression: User-friendly name for the GPUProcess no longer gets set #15356

cdumez · 2023-06-28T04:10:39Z

`73df24e`

Regression: User-friendly name for the GPUProcess no longer gets set
https://bugs.webkit.org/show_bug.cgi?id=258600

Reviewed by Per Arne Vollan and Brent Fulgham.

Access to launch services has been blocked by the GPUProcess' sandbox. As a
result, the GPUProcess no longer gets a user-friendly name in Activity Monitor.

To address the issue, we now:
1. Have the UIProcess create a sandbox extension for launch services and send it
   to the GPUProcess to temporarily consume it during initialization
2. The GPUProcess, on initialization now consumes this extension
3. The GPUProcess checks in with Launch Services and sets the process name
4. The GPUProcess closes connections to launch services
5. The GPUProcess revokes the sandbox extension

* Source/WebKit/GPUProcess/GPUProcess.cpp:
(WebKit::GPUProcess::initializeGPUProcess):
* Source/WebKit/GPUProcess/GPUProcess.h:
* Source/WebKit/GPUProcess/GPUProcessCreationParameters.cpp:
(WebKit::GPUProcessCreationParameters::encode const):
(WebKit::GPUProcessCreationParameters::decode):
* Source/WebKit/GPUProcess/GPUProcessCreationParameters.h:
* Source/WebKit/GPUProcess/cocoa/GPUProcessCocoa.mm:
(WebKit::GPUProcess::platformInitializeGPUProcess):
* Source/WebKit/GPUProcess/mac/GPUProcessMac.mm:
(WebKit::GPUProcess::initializeProcessName):
(WebKit::GPUProcess::updateProcessName):
* Source/WebKit/GPUProcess/mac/com.apple.WebKit.GPUProcess.sb.in:
* Source/WebKit/UIProcess/Cocoa/GPUProcessProxyCocoa.mm:
(WebKit::GPUProcessProxy::platformInitializeGPUProcessParameters):

Canonical link: https://commits.webkit.org/265610@main

566993e

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🛠 gtk
	✅ 🧪 ios-wk2-wpt		✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2
	✅ 🛠 tv-sim
✅ 🛠 🧪 merge	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2023-06-28T04:10:49Z

EWS run on previous version of this PR (hash 1bdbbe4)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🛠 gtk
	✅ 🧪 ios-wk2-wpt		✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv	❌ 🧪 mac-AS-debug-wk2
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

pvollan · 2023-06-28T14:51:34Z

Source/WebKit/GPUProcess/mac/GPUProcessMac.mm

@@ -63,8 +63,19 @@
 void GPUProcess::initializeProcessName(const AuxiliaryProcessInitializationParameters& parameters)
 {
 #if !PLATFORM(MACCATALYST)
+    // Register the application. This will also check in with Launch Services.
+    // This is necessary to set the process name.
+    _RegisterApplication(nullptr, nullptr);


Would it be sufficient to just check in with Launch Services? I do not think the Networking process calls _RegisterApplication?

pvollan · 2023-06-28T14:54:25Z

Source/WebKit/Shared/AuxiliaryProcess.cpp

+    // Set process name before initializing the sandbox as we may need to
+    // connect to launch services to set the name and the sandbox doesn't
+    // necessarily allow it.
+    initializeProcessName(parameters);


Perhaps you could create a sandbox extension for Launch Services, which would be revoked after setting the process name?

webkit-early-warning-system · 2023-06-28T17:57:48Z

EWS run on previous version of this PR (hash 84d791b)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	⏳ 🛠 wpe	⏳ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	⏳ 🧪 wpe-wk2
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	~~🧪 api-mac~~	⏳ 🛠 gtk
	~~🧪 ios-wk2-wpt~~		⏳ 🧪 gtk-wk2
⏳ 🛠 🧪 jsc	~~🧪 api-ios~~	~~🧪 mac-wk2~~	⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc-arm64	✅ 🛠 tv	~~🧪 mac-AS-debug-wk2~~
	✅ 🛠 tv-sim
	⏳ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2023-06-28T17:59:49Z

EWS run on previous version of this PR (hash 0376f7b)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	~~🧪 api-mac~~	✅ 🛠 gtk
	✅ 🧪 ios-wk2-wpt		✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

pvollan · 2023-06-28T20:04:32Z

Source/WebKit/GPUProcess/cocoa/GPUProcessCocoa.mm

+    _LSSetApplicationInformationItem(kLSDefaultSessionID, _LSGetCurrentApplicationASN(), _kLSPersistenceSuppressRelaunchAtLoginKey, kCFBooleanTrue, nullptr);
+
+    if (launchServicesExtension)
+        launchServicesExtension->revoke();


I think it would be good if we closed any open LS connections here.

webkit-early-warning-system · 2023-06-28T20:59:53Z

EWS run on previous version of this PR (hash d0d40ce)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	~~🛠 wincairo~~
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	~~🧪 api-mac~~	✅ 🛠 gtk
	✅ 🧪 ios-wk2-wpt		⏳ 🧪 gtk-wk2
	✅ 🧪 api-ios	~~🧪 mac-wk2~~	~~🧪 api-gtk~~
	✅ 🛠 tv	~~🧪 mac-AS-debug-wk2~~
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

pvollan · 2023-06-28T21:35:13Z

Source/WebKit/GPUProcess/cocoa/GPUProcessCocoa.mm

+    // Disable relaunch on login. This is also done from -[NSApplication init] by dispatching -[NSApplication disableRelaunchOnLogin] on a non-main thread.
+    // This will be in a race with the closing of the Launch Services connection, so call it synchronously here.
+    // The cost of calling this should be small, and it is not expected to have any impact on performance.
+    _LSSetApplicationInformationItem(kLSDefaultSessionID, _LSGetCurrentApplicationASN(), _kLSPersistenceSuppressRelaunchAtLoginKey, kCFBooleanTrue, nullptr);


I don't think this is required in the GPU process, since it was a workaround for a sandbox issue when NSApplication is being used in the WebContent process.

pvollan

Looks good! R=me.

webkit-early-warning-system · 2023-06-28T21:39:24Z

EWS run on current version of this PR (hash 566993e)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🛠 gtk
	✅ 🧪 ios-wk2-wpt		✅ 🧪 gtk-wk2
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🧪 api-gtk
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2
	✅ 🛠 tv-sim
✅ 🛠 🧪 merge	✅ 🛠 watch
	✅ 🛠 watch-sim

pvollan · 2023-06-28T21:44:14Z

Source/WebKit/GPUProcess/cocoa/GPUProcessCocoa.mm

+    // It is important to check in with launch services before setting the process name.
+    launchServicesCheckIn();
+
+    // Update process name while holding the Launch Services sandbox extension
+    updateProcessName();
+
+    // Close connection to launch services.
+#if HAVE(HAVE_LS_SERVER_CONNECTION_STATUS_RELEASE_NOTIFICATIONS_MASK)
+    _LSSetApplicationLaunchServicesServerConnectionStatus(kLSServerConnectionStatusDoNotConnectToServerMask | kLSServerConnectionStatusReleaseNotificationsMask, nullptr);
+#else
+    _LSSetApplicationLaunchServicesServerConnectionStatus(kLSServerConnectionStatusDoNotConnectToServerMask, nullptr);
+#endif


Nit: this could be moved into the if (launchServicesExtension) clause, since it will not work without the extension.

Does it hurt to keep as is?

Keeping as-is is fine :)

brentfulgham

Looks good to me, too, assuming you adopt Per Arne's requested changes.

https://bugs.webkit.org/show_bug.cgi?id=258600 Reviewed by Per Arne Vollan and Brent Fulgham. Access to launch services has been blocked by the GPUProcess' sandbox. As a result, the GPUProcess no longer gets a user-friendly name in Activity Monitor. To address the issue, we now: 1. Have the UIProcess create a sandbox extension for launch services and send it to the GPUProcess to temporarily consume it during initialization 2. The GPUProcess, on initialization now consumes this extension 3. The GPUProcess checks in with Launch Services and sets the process name 4. The GPUProcess closes connections to launch services 5. The GPUProcess revokes the sandbox extension * Source/WebKit/GPUProcess/GPUProcess.cpp: (WebKit::GPUProcess::initializeGPUProcess): * Source/WebKit/GPUProcess/GPUProcess.h: * Source/WebKit/GPUProcess/GPUProcessCreationParameters.cpp: (WebKit::GPUProcessCreationParameters::encode const): (WebKit::GPUProcessCreationParameters::decode): * Source/WebKit/GPUProcess/GPUProcessCreationParameters.h: * Source/WebKit/GPUProcess/cocoa/GPUProcessCocoa.mm: (WebKit::GPUProcess::platformInitializeGPUProcess): * Source/WebKit/GPUProcess/mac/GPUProcessMac.mm: (WebKit::GPUProcess::initializeProcessName): (WebKit::GPUProcess::updateProcessName): * Source/WebKit/GPUProcess/mac/com.apple.WebKit.GPUProcess.sb.in: * Source/WebKit/UIProcess/Cocoa/GPUProcessProxyCocoa.mm: (WebKit::GPUProcessProxy::platformInitializeGPUProcessParameters): Canonical link: https://commits.webkit.org/265610@main

webkit-commit-queue · 2023-06-29T03:34:48Z

Committed 265610@main (73df24e): https://commits.webkit.org/265610@main

Reviewed commits have been landed. Closing PR #15356 and removing active labels.

cdumez self-assigned this Jun 28, 2023

cdumez added the WebKit2 Bugs relating to the WebKit2 API layer label Jun 28, 2023

cdumez requested a review from pvollan June 28, 2023 04:11

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 28, 2023

pvollan reviewed Jun 28, 2023

View reviewed changes

cdumez removed the merging-blocked Applied to prevent a change from being merged label Jun 28, 2023

cdumez force-pushed the 258600_GPUProcess_name branch from 1bdbbe4 to 84d791b Compare June 28, 2023 17:57

cdumez force-pushed the 258600_GPUProcess_name branch from 84d791b to 0376f7b Compare June 28, 2023 17:59

pvollan reviewed Jun 28, 2023

View reviewed changes

cdumez force-pushed the 258600_GPUProcess_name branch from 0376f7b to d0d40ce Compare June 28, 2023 20:59

pvollan reviewed Jun 28, 2023

View reviewed changes

pvollan approved these changes Jun 28, 2023

View reviewed changes

cdumez force-pushed the 258600_GPUProcess_name branch from d0d40ce to 566993e Compare June 28, 2023 21:39

pvollan reviewed Jun 28, 2023

View reviewed changes

cdumez marked this pull request as ready for review June 28, 2023 21:53

cdumez requested a review from brentfulgham as a code owner June 28, 2023 21:53

brentfulgham approved these changes Jun 28, 2023

View reviewed changes

cdumez added the merge-queue Applied to send a pull request to merge-queue label Jun 29, 2023

webkit-commit-queue force-pushed the 258600_GPUProcess_name branch from 566993e to 73df24e Compare June 29, 2023 03:34

webkit-commit-queue merged commit 73df24e into WebKit:main Jun 29, 2023

webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jun 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Regression: User-friendly name for the GPUProcess no longer gets set #15356

Regression: User-friendly name for the GPUProcess no longer gets set #15356

cdumez commented Jun 28, 2023 •

edited by webkit-early-warning-system

webkit-early-warning-system commented Jun 28, 2023 •

edited

pvollan Jun 28, 2023

pvollan Jun 28, 2023

webkit-early-warning-system commented Jun 28, 2023 •

edited

webkit-early-warning-system commented Jun 28, 2023 •

edited

pvollan Jun 28, 2023

webkit-early-warning-system commented Jun 28, 2023 •

edited

pvollan Jun 28, 2023

pvollan left a comment

webkit-early-warning-system commented Jun 28, 2023 •

edited

pvollan Jun 28, 2023

cdumez Jun 28, 2023

pvollan Jun 29, 2023

brentfulgham left a comment

webkit-commit-queue commented Jun 29, 2023

Regression: User-friendly name for the GPUProcess no longer gets set #15356

Regression: User-friendly name for the GPUProcess no longer gets set #15356

Conversation

cdumez commented Jun 28, 2023 • edited by webkit-early-warning-system

webkit-early-warning-system commented Jun 28, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

webkit-early-warning-system commented Jun 28, 2023 • edited

webkit-early-warning-system commented Jun 28, 2023 • edited

Choose a reason for hiding this comment

webkit-early-warning-system commented Jun 28, 2023 • edited

Choose a reason for hiding this comment

pvollan left a comment

Choose a reason for hiding this comment

webkit-early-warning-system commented Jun 28, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brentfulgham left a comment

Choose a reason for hiding this comment

webkit-commit-queue commented Jun 29, 2023

cdumez commented Jun 28, 2023 •

edited by webkit-early-warning-system

webkit-early-warning-system commented Jun 28, 2023 •

edited

webkit-early-warning-system commented Jun 28, 2023 •

edited

webkit-early-warning-system commented Jun 28, 2023 •

edited

webkit-early-warning-system commented Jun 28, 2023 •

edited

webkit-early-warning-system commented Jun 28, 2023 •

edited