- Revision
- 276212
- Author
- [email protected]
- Date
- 2021-04-17 16:22:13 -0700 (Sat, 17 Apr 2021)
Log Message
RemoteImageDecoderAVFManager should never re-launch the GPUProcess on destruction
https://bugs.webkit.org/show_bug.cgi?id=224723
Reviewed by Darin Adler.
RemoteImageDecoderAVFManager was calling ensureGPUProcessConnection() it is destructor,
just to remove itself as an IPC message receiver. This means it could unnecessarily
relaunch the GPUProcess. This patch addresses that.
This patch also makes it so that RemoteImageDecoderAVFManager registers itself as a
client of the GPUProcessConnection, so that it gets notified when the connection gets
severed. Right now, I only do very basic crash handling but this paves the way to do
better in the future. I did fix a bug where the RemoteImageDecoderAVFManager would
not re-register itself as a message receiver after a GPUProcess re-launch.
* WebProcess/GPU/media/RemoteImageDecoderAVF.cpp:
(WebKit::RemoteImageDecoderAVF::RemoteImageDecoderAVF):
* WebProcess/GPU/media/RemoteImageDecoderAVFManager.cpp:
(WebKit::RemoteImageDecoderAVFManager::createImageDecoder):
(WebKit::RemoteImageDecoderAVFManager::deleteRemoteImageDecoder):
(WebKit::RemoteImageDecoderAVFManager::~RemoteImageDecoderAVFManager):
(WebKit::RemoteImageDecoderAVFManager::gpuProcessConnectionDidClose):
(WebKit::RemoteImageDecoderAVFManager::ensureGPUProcessConnection):
(WebKit::RemoteImageDecoderAVFManager::gpuProcessConnection const): Deleted.
* WebProcess/GPU/media/RemoteImageDecoderAVFManager.h:
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (276211 => 276212)
--- trunk/Source/WebKit/ChangeLog 2021-04-17 22:55:14 UTC (rev 276211)
+++ trunk/Source/WebKit/ChangeLog 2021-04-17 23:22:13 UTC (rev 276212)
@@ -1,3 +1,31 @@
+2021-04-17 Chris Dumez <[email protected]>
+
+ RemoteImageDecoderAVFManager should never re-launch the GPUProcess on destruction
+ https://bugs.webkit.org/show_bug.cgi?id=224723
+
+ Reviewed by Darin Adler.
+
+ RemoteImageDecoderAVFManager was calling ensureGPUProcessConnection() it is destructor,
+ just to remove itself as an IPC message receiver. This means it could unnecessarily
+ relaunch the GPUProcess. This patch addresses that.
+
+ This patch also makes it so that RemoteImageDecoderAVFManager registers itself as a
+ client of the GPUProcessConnection, so that it gets notified when the connection gets
+ severed. Right now, I only do very basic crash handling but this paves the way to do
+ better in the future. I did fix a bug where the RemoteImageDecoderAVFManager would
+ not re-register itself as a message receiver after a GPUProcess re-launch.
+
+ * WebProcess/GPU/media/RemoteImageDecoderAVF.cpp:
+ (WebKit::RemoteImageDecoderAVF::RemoteImageDecoderAVF):
+ * WebProcess/GPU/media/RemoteImageDecoderAVFManager.cpp:
+ (WebKit::RemoteImageDecoderAVFManager::createImageDecoder):
+ (WebKit::RemoteImageDecoderAVFManager::deleteRemoteImageDecoder):
+ (WebKit::RemoteImageDecoderAVFManager::~RemoteImageDecoderAVFManager):
+ (WebKit::RemoteImageDecoderAVFManager::gpuProcessConnectionDidClose):
+ (WebKit::RemoteImageDecoderAVFManager::ensureGPUProcessConnection):
+ (WebKit::RemoteImageDecoderAVFManager::gpuProcessConnection const): Deleted.
+ * WebProcess/GPU/media/RemoteImageDecoderAVFManager.h:
+
2021-04-17 Tyler Wilcock <[email protected]>
Consider making CSSStyleSheet::rules() just an alias of CSSStyleSheet::cssRules().
Modified: trunk/Source/WebKit/WebProcess/GPU/media/RemoteImageDecoderAVF.cpp (276211 => 276212)
--- trunk/Source/WebKit/WebProcess/GPU/media/RemoteImageDecoderAVF.cpp 2021-04-17 22:55:14 UTC (rev 276211)
+++ trunk/Source/WebKit/WebProcess/GPU/media/RemoteImageDecoderAVF.cpp 2021-04-17 23:22:13 UTC (rev 276212)
@@ -48,7 +48,7 @@
RemoteImageDecoderAVF::RemoteImageDecoderAVF(RemoteImageDecoderAVFManager& manager, const WebCore::ImageDecoderIdentifier& identifier, const String& mimeType)
: ImageDecoder()
- , m_gpuProcessConnection(makeWeakPtr(manager.gpuProcessConnection()))
+ , m_gpuProcessConnection(makeWeakPtr(manager.ensureGPUProcessConnection()))
, m_manager(manager)
, m_identifier(identifier)
, m_mimeType(mimeType)
Modified: trunk/Source/WebKit/WebProcess/GPU/media/RemoteImageDecoderAVFManager.cpp (276211 => 276212)
--- trunk/Source/WebKit/WebProcess/GPU/media/RemoteImageDecoderAVFManager.cpp 2021-04-17 22:55:14 UTC (rev 276211)
+++ trunk/Source/WebKit/WebProcess/GPU/media/RemoteImageDecoderAVFManager.cpp 2021-04-17 23:22:13 UTC (rev 276212)
@@ -41,14 +41,9 @@
RefPtr<RemoteImageDecoderAVF> RemoteImageDecoderAVFManager::createImageDecoder(SharedBuffer& data, const String& mimeType, AlphaOption alphaOption, GammaAndColorProfileOption gammaAndColorProfileOption)
{
- if (!m_messageReceiverInitialized) {
- m_messageReceiverInitialized = true;
- gpuProcessConnection().messageReceiverMap().addMessageReceiver(Messages::RemoteImageDecoderAVFManager::messageReceiverName(), *this);
- }
-
Optional<ImageDecoderIdentifier> imageDecoderIdentifier;
IPC::SharedBufferDataReference dataReference { data };
- if (!gpuProcessConnection().connection().sendSync(Messages::RemoteImageDecoderAVFProxy::CreateDecoder(dataReference, mimeType), Messages::RemoteImageDecoderAVFProxy::CreateDecoder::Reply(imageDecoderIdentifier), 0))
+ if (!ensureGPUProcessConnection().connection().sendSync(Messages::RemoteImageDecoderAVFProxy::CreateDecoder(dataReference, mimeType), Messages::RemoteImageDecoderAVFProxy::CreateDecoder::Reply(imageDecoderIdentifier), 0))
return nullptr;
if (!imageDecoderIdentifier)
@@ -63,7 +58,8 @@
void RemoteImageDecoderAVFManager::deleteRemoteImageDecoder(const ImageDecoderIdentifier& identifier)
{
m_remoteImageDecoders.take(identifier);
- gpuProcessConnection().connection().send(Messages::RemoteImageDecoderAVFProxy::DeleteDecoder(identifier), 0);
+ if (m_gpuProcessConnection)
+ m_gpuProcessConnection->connection().send(Messages::RemoteImageDecoderAVFProxy::DeleteDecoder(identifier), 0);
}
RemoteImageDecoderAVFManager::RemoteImageDecoderAVFManager(WebProcess& process)
@@ -73,17 +69,32 @@
RemoteImageDecoderAVFManager::~RemoteImageDecoderAVFManager()
{
- gpuProcessConnection().messageReceiverMap().removeMessageReceiver(Messages::RemoteImageDecoderAVFManager::messageReceiverName());
+ if (m_gpuProcessConnection)
+ m_gpuProcessConnection->messageReceiverMap().removeMessageReceiver(Messages::RemoteImageDecoderAVFManager::messageReceiverName());
}
+void RemoteImageDecoderAVFManager::gpuProcessConnectionDidClose(GPUProcessConnection& connection)
+{
+ ASSERT(m_gpuProcessConnection == &connection);
+ connection.removeClient(*this);
+ m_gpuProcessConnection->messageReceiverMap().removeMessageReceiver(Messages::RemoteImageDecoderAVFManager::messageReceiverName());
+ m_gpuProcessConnection = nullptr;
+ // FIXME: Do we need to do more when m_remoteImageDecoders is not empty to re-create them?
+}
+
const char* RemoteImageDecoderAVFManager::supplementName()
{
return "RemoteImageDecoderAVFManager";
}
-GPUProcessConnection& RemoteImageDecoderAVFManager::gpuProcessConnection() const
+GPUProcessConnection& RemoteImageDecoderAVFManager::ensureGPUProcessConnection()
{
- return m_process.ensureGPUProcessConnection();
+ if (!m_gpuProcessConnection) {
+ m_gpuProcessConnection = makeWeakPtr(m_process.ensureGPUProcessConnection());
+ m_gpuProcessConnection->addClient(*this);
+ m_gpuProcessConnection->messageReceiverMap().addMessageReceiver(Messages::RemoteImageDecoderAVFManager::messageReceiverName(), *this);
+ }
+ return *m_gpuProcessConnection;
}
void RemoteImageDecoderAVFManager::setUseGPUProcess(bool useGPUProcess)
Modified: trunk/Source/WebKit/WebProcess/GPU/media/RemoteImageDecoderAVFManager.h (276211 => 276212)
--- trunk/Source/WebKit/WebProcess/GPU/media/RemoteImageDecoderAVFManager.h 2021-04-17 22:55:14 UTC (rev 276211)
+++ trunk/Source/WebKit/WebProcess/GPU/media/RemoteImageDecoderAVFManager.h 2021-04-17 23:22:13 UTC (rev 276212)
@@ -28,6 +28,7 @@
#if ENABLE(GPU_PROCESS) && HAVE(AVASSETREADER)
#include "Connection.h"
+#include "GPUProcessConnection.h"
#include "MessageReceiver.h"
#include "WebProcessSupplement.h"
#include <WebCore/ImageDecoderIdentifier.h>
@@ -38,12 +39,12 @@
namespace WebKit {
-class GPUProcessConnection;
class RemoteImageDecoderAVF;
class WebProcess;
-class RemoteImageDecoderAVFManager
+class RemoteImageDecoderAVFManager final
: public WebProcessSupplement
+ , private GPUProcessConnection::Client
, private IPC::MessageReceiver {
WTF_MAKE_FAST_ALLOCATED;
public:
@@ -55,11 +56,14 @@
static const char* supplementName();
void setUseGPUProcess(bool);
- GPUProcessConnection& gpuProcessConnection() const;
+ GPUProcessConnection& ensureGPUProcessConnection();
private:
RefPtr<RemoteImageDecoderAVF> createImageDecoder(WebCore::SharedBuffer& data, const String& mimeType, WebCore::AlphaOption, WebCore::GammaAndColorProfileOption);
+ // GPUProcessConnection::Client.
+ void gpuProcessConnectionDidClose(GPUProcessConnection&) final;
+
void didReceiveMessage(IPC::Connection&, IPC::Decoder&) final;
void encodedDataStatusChanged(const WebCore::ImageDecoderIdentifier&, size_t frameCount, const WebCore::IntSize&, bool hasTrack);
@@ -66,7 +70,7 @@
HashMap<WebCore::ImageDecoderIdentifier, WeakPtr<RemoteImageDecoderAVF>> m_remoteImageDecoders;
WebProcess& m_process;
- bool m_messageReceiverInitialized { false };
+ WeakPtr<GPUProcessConnection> m_gpuProcessConnection;
};
}
