Title: [285477] trunk/Source/WebKit
- Revision
- 285477
- Author
- [email protected]
- Date
- 2021-11-08 18:11:14 -0800 (Mon, 08 Nov 2021)
Log Message
Make it possible to toggle several experimental media features when GPU Process is enabled
https://bugs.webkit.org/show_bug.cgi?id=232818
Reviewed by Tim Horton.
Refactor a bit of logic around `GPUProcessProxy::updatePreferences`, such that the experimental features that
are set underneath this method can be toggled by checking or unchecking the preference in the experimental
features menu in Safari, and then restarting Safari (or simply killing and relaunching the GPU process).
As detailed in the existing FIXME, we currently consult the global WebPageGroups' WebPreferences, which do not
account for changes to the experimental feature defaults since they initialize their own WebPreferences that
are (1) prefixed with a unique identifier, and (2) have `keyPrefix` and `globalDebugKeyPrefix` that are
different from normal WebPreferences on WebPageProxy.
Instead, we can add some plumbing to provide a WebProcessProxy to `updatePreferences()`, and use this to
determine whether we need to enable any experimental features by iterating over each WebPageProxy's preferences
and enabling each feature if it's enabled by at least one web page (roughly matching the current behavior).
While it's still not ideal, these features are all process-global and there is only one GPU process, so in lieu
of more extensive refactoring around these media feature flags, it makes sense to just turn them on if it's
enabled in any individual instance of WebPreferences.
* UIProcess/GPU/GPUProcessProxy.cpp:
(WebKit::GPUProcessProxy::getOrCreate):
Hoist the call to `updatePreferences()` out of this method and into `WebProcessPool::ensureGPUProcess` instead,
so that we can pass in WebProcessProxy.
(WebKit::GPUProcessProxy::updatePreferences):
See above for more details.
(WebKit::GPUProcessProxy::updateScreenPropertiesIfNeeded):
Split out logic to update screen properties into a separate method, so that we don't end up unnecessarily
computing and sending redundant screen properties `WebProcessPool::ensureGPUProcess()`.
* UIProcess/GPU/GPUProcessProxy.h:
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::ensureGPUProcess):
(WebKit::WebProcessPool::createWebPage):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (285476 => 285477)
--- trunk/Source/WebKit/ChangeLog 2021-11-09 01:39:12 UTC (rev 285476)
+++ trunk/Source/WebKit/ChangeLog 2021-11-09 02:11:14 UTC (rev 285477)
@@ -1,3 +1,46 @@
+2021-11-08 Wenson Hsieh <[email protected]>
+
+ Make it possible to toggle several experimental media features when GPU Process is enabled
+ https://bugs.webkit.org/show_bug.cgi?id=232818
+
+ Reviewed by Tim Horton.
+
+ Refactor a bit of logic around `GPUProcessProxy::updatePreferences`, such that the experimental features that
+ are set underneath this method can be toggled by checking or unchecking the preference in the experimental
+ features menu in Safari, and then restarting Safari (or simply killing and relaunching the GPU process).
+
+ As detailed in the existing FIXME, we currently consult the global WebPageGroups' WebPreferences, which do not
+ account for changes to the experimental feature defaults since they initialize their own WebPreferences that
+ are (1) prefixed with a unique identifier, and (2) have `keyPrefix` and `globalDebugKeyPrefix` that are
+ different from normal WebPreferences on WebPageProxy.
+
+ Instead, we can add some plumbing to provide a WebProcessProxy to `updatePreferences()`, and use this to
+ determine whether we need to enable any experimental features by iterating over each WebPageProxy's preferences
+ and enabling each feature if it's enabled by at least one web page (roughly matching the current behavior).
+ While it's still not ideal, these features are all process-global and there is only one GPU process, so in lieu
+ of more extensive refactoring around these media feature flags, it makes sense to just turn them on if it's
+ enabled in any individual instance of WebPreferences.
+
+ * UIProcess/GPU/GPUProcessProxy.cpp:
+ (WebKit::GPUProcessProxy::getOrCreate):
+
+ Hoist the call to `updatePreferences()` out of this method and into `WebProcessPool::ensureGPUProcess` instead,
+ so that we can pass in WebProcessProxy.
+
+ (WebKit::GPUProcessProxy::updatePreferences):
+
+ See above for more details.
+
+ (WebKit::GPUProcessProxy::updateScreenPropertiesIfNeeded):
+
+ Split out logic to update screen properties into a separate method, so that we don't end up unnecessarily
+ computing and sending redundant screen properties `WebProcessPool::ensureGPUProcess()`.
+
+ * UIProcess/GPU/GPUProcessProxy.h:
+ * UIProcess/WebProcessPool.cpp:
+ (WebKit::WebProcessPool::ensureGPUProcess):
+ (WebKit::WebProcessPool::createWebPage):
+
2021-11-08 J Pascoe <[email protected]>
[WebAuthn] challenge does not get passed to -[_WKWebAuthenticationPanel getAssertionWithChallenge:origin:options:completionHandler:]
Modified: trunk/Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp (285476 => 285477)
--- trunk/Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp 2021-11-09 01:39:12 UTC (rev 285476)
+++ trunk/Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp 2021-11-09 02:11:14 UTC (rev 285477)
@@ -112,7 +112,6 @@
return *existingGPUProcess;
}
auto gpuProcess = adoptRef(*new GPUProcessProxy);
- gpuProcess->updatePreferences();
singleton() = gpuProcess;
return gpuProcess;
}
@@ -587,94 +586,69 @@
}
#endif
-void GPUProcessProxy::updatePreferences()
+void GPUProcessProxy::updatePreferences(WebProcessProxy& webProcess)
{
if (!canSendMessage())
return;
-#if ENABLE(MEDIA_SOURCE) && ENABLE(VP9)
- bool hasEnabledWebMParser = false;
-#endif
-
-#if ENABLE(WEBM_FORMAT_READER)
- bool hasEnabledWebMFormatReader = false;
-#endif
-
-#if ENABLE(OPUS)
- bool hasEnabledOpus = false;
-#endif
-
-#if ENABLE(VORBIS)
- bool hasEnabledVorbis = false;
-#endif
-
-#if ENABLE(MEDIA_SOURCE) && HAVE(AVSAMPLEBUFFERVIDEOOUTPUT)
- bool hasEnabledMediaSourceInlinePainting = false;
-#endif
-
// FIXME: We should consider consolidating these into a single struct and propagating it to the GPU process as a single IPC message,
// instead of sending one message for each preference.
- //
- // FIXME: Additionally, it seems wrong to consult preferences on WebPageGroup rather than WebPreferences corresponding to each page.
- // This effectively means that each of the below preferences will always be set to the default value and cannot be toggled through the
- // develop menu or user defaults, since the defaults for each group are prefixed with a unique identifier.
- //
- // Since each of these features apply to the entire GPU process at once (i.e. they affect all web pages using the GPU process), we
- // could instead refactor this so that we iterate through all web pages' preferences when initializing the GPU process for the first
- // time, and then update these flags when creating new web pages.
- WebPageGroup::forEach([&] (auto& group) mutable {
- if (!group.preferences().useGPUProcessForMediaEnabled())
- return;
+ // FIXME: It's not ideal that these features are controlled by preferences-level feature flags (i.e. per-web view), but there is only
+ // one GPU process and the runtime-enabled features backing these preferences are process-wide. We should refactor each of these features
+ // so that they aren't process-global, and then reimplement this feature flag propagation to the GPU Process in a way that respects the
+ // settings of the page that is hosting each media element.
+ // For the time being, each of the below features are enabled in the GPU Process if it is enabled by at least one web page's preferences.
+ // In practice, all web pages' preferences should agree on these feature flag values.
+ for (auto page : webProcess.pages()) {
+ auto& preferences = page->preferences();
+ if (!preferences.useGPUProcessForMediaEnabled())
+ continue;
#if ENABLE(OPUS)
- if (group.preferences().opusDecoderEnabled())
- hasEnabledOpus = true;
+ if (!m_hasEnabledOpus && preferences.opusDecoderEnabled()) {
+ m_hasEnabledOpus = true;
+ send(Messages::GPUProcess::SetOpusDecoderEnabled(m_hasEnabledOpus), 0);
+ }
#endif
#if ENABLE(VORBIS)
- if (group.preferences().vorbisDecoderEnabled())
- hasEnabledVorbis = true;
+ if (!m_hasEnabledVorbis && preferences.vorbisDecoderEnabled()) {
+ m_hasEnabledVorbis = true;
+ send(Messages::GPUProcess::SetVorbisDecoderEnabled(m_hasEnabledVorbis), 0);
+ }
#endif
#if ENABLE(WEBM_FORMAT_READER)
- if (group.preferences().webMFormatReaderEnabled())
- hasEnabledWebMFormatReader = true;
+ if (!m_hasEnabledWebMFormatReader && preferences.webMFormatReaderEnabled()) {
+ m_hasEnabledWebMFormatReader = true;
+ send(Messages::GPUProcess::SetWebMFormatReaderEnabled(m_hasEnabledWebMFormatReader), 0);
+ }
#endif
#if ENABLE(MEDIA_SOURCE) && ENABLE(VP9)
- if (group.preferences().webMParserEnabled())
- hasEnabledWebMParser = true;
+ if (!m_hasEnabledWebMParser && preferences.webMParserEnabled()) {
+ m_hasEnabledWebMParser = true;
+ send(Messages::GPUProcess::SetWebMParserEnabled(m_hasEnabledWebMParser), 0);
+ }
#endif
#if ENABLE(MEDIA_SOURCE) && HAVE(AVSAMPLEBUFFERVIDEOOUTPUT)
- if (group.preferences().mediaSourceInlinePaintingEnabled())
- hasEnabledMediaSourceInlinePainting = true;
+ if (!m_hasEnabledMediaSourceInlinePainting && preferences.mediaSourceInlinePaintingEnabled()) {
+ m_hasEnabledMediaSourceInlinePainting = true;
+ send(Messages::GPUProcess::SetMediaSourceInlinePaintingEnabled(m_hasEnabledMediaSourceInlinePainting), 0);
+ }
#endif
- });
+ }
+}
-#if ENABLE(MEDIA_SOURCE) && ENABLE(VP9)
- send(Messages::GPUProcess::SetWebMParserEnabled(hasEnabledWebMParser), 0);
-#endif
+void GPUProcessProxy::updateScreenPropertiesIfNeeded()
+{
+#if PLATFORM(MAC)
+ if (!canSendMessage())
+ return;
-#if ENABLE(WEBM_FORMAT_READER)
- send(Messages::GPUProcess::SetWebMFormatReaderEnabled(hasEnabledWebMFormatReader), 0);
+ setScreenProperties(collectScreenProperties());
#endif
-
-#if ENABLE(OPUS)
- send(Messages::GPUProcess::SetOpusDecoderEnabled(hasEnabledOpus), 0);
-#endif
-
-#if ENABLE(VORBIS)
- send(Messages::GPUProcess::SetVorbisDecoderEnabled(hasEnabledVorbis), 0);
-#endif
-
-#if ENABLE(MEDIA_SOURCE) && HAVE(AVSAMPLEBUFFERVIDEOOUTPUT)
- send(Messages::GPUProcess::SetMediaSourceInlinePaintingEnabled(hasEnabledMediaSourceInlinePainting), 0);
-#endif
-
-#if PLATFORM(MAC)
- setScreenProperties(WebCore::collectScreenProperties());
-#endif
}
void GPUProcessProxy::didBecomeUnresponsive()
Modified: trunk/Source/WebKit/UIProcess/GPU/GPUProcessProxy.h (285476 => 285477)
--- trunk/Source/WebKit/UIProcess/GPU/GPUProcessProxy.h 2021-11-09 01:39:12 UTC (rev 285476)
+++ trunk/Source/WebKit/UIProcess/GPU/GPUProcessProxy.h 2021-11-09 02:11:14 UTC (rev 285477)
@@ -93,7 +93,8 @@
void setScreenProperties(const WebCore::ScreenProperties&);
#endif
- void updatePreferences();
+ void updatePreferences(WebProcessProxy&);
+ void updateScreenPropertiesIfNeeded();
void terminateForTesting();
void webProcessConnectionCountForTesting(CompletionHandler<void(uint64_t)>&&);
@@ -151,6 +152,27 @@
bool m_hasSentMicrophoneSandboxExtension { false };
bool m_hasSentNetworkProcessXPCEndpoint { false };
#endif
+
+#if ENABLE(MEDIA_SOURCE) && ENABLE(VP9)
+ bool m_hasEnabledWebMParser { false };
+#endif
+
+#if ENABLE(WEBM_FORMAT_READER)
+ bool m_hasEnabledWebMFormatReader { false };
+#endif
+
+#if ENABLE(OPUS)
+ bool m_hasEnabledOpus { false };
+#endif
+
+#if ENABLE(VORBIS)
+ bool m_hasEnabledVorbis { false };
+#endif
+
+#if ENABLE(MEDIA_SOURCE) && HAVE(AVSAMPLEBUFFERVIDEOOUTPUT)
+ bool m_hasEnabledMediaSourceInlinePainting { false };
+#endif
+
HashSet<PAL::SessionID> m_sessionIDs;
};
Modified: trunk/Source/WebKit/UIProcess/WebProcessPool.cpp (285476 => 285477)
--- trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2021-11-09 01:39:12 UTC (rev 285476)
+++ trunk/Source/WebKit/UIProcess/WebProcessPool.cpp 2021-11-09 02:11:14 UTC (rev 285477)
@@ -437,8 +437,12 @@
#if ENABLE(GPU_PROCESS)
GPUProcessProxy& WebProcessPool::ensureGPUProcess()
{
- if (!m_gpuProcess)
+ if (!m_gpuProcess) {
m_gpuProcess = GPUProcessProxy::getOrCreate();
+ for (auto& process : m_processes)
+ m_gpuProcess->updatePreferences(process);
+ m_gpuProcess->updateScreenPropertiesIfNeeded();
+ }
return *m_gpuProcess;
}
@@ -1105,8 +1109,10 @@
m_webProcessCache->updateCapacity(*this);
#if ENABLE(GPU_PROCESS)
- if (auto* gpuProcess = GPUProcessProxy::singletonIfCreated())
- gpuProcess->updatePreferences();
+ if (auto* gpuProcess = GPUProcessProxy::singletonIfCreated()) {
+ gpuProcess->updatePreferences(*process);
+ gpuProcess->updateScreenPropertiesIfNeeded();
+ }
#endif
return page;
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
