[webkit-reviews] review granted: [Bug 218924] [GPUProcess] Add basic GPUProcess crash handling for canvas : [Attachment 414098] Patch

[bugzilla-daemon at webkit.org]

Simon Fraser (smfr) <simon.fraser at apple.com> has granted Chris Dumez
<cdumez at apple.com>'s request for review:
Bug 218924: [GPUProcess] Add basic GPUProcess crash handling for canvas
https://bugs.webkit.org/show_bug.cgi?id=218924

Attachment 414098: Patch

https://bugs.webkit.org/attachment.cgi?id=414098&action=review




--- Comment #2 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 414098
  --> https://bugs.webkit.org/attachment.cgi?id=414098
Patch

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

> Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h:83
> +    float resolutionScale() const override { return m_resolutionScale; }

final?

> Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:66
> +    // Register itself as a MessageReceiver in the GPUProcessConnection.

Comment doesn't add anything.

> Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:70
> +    IPC::MessageReceiverMap& messageReceiverMap =
connection.messageReceiverMap();

auto?

> Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:74
> +   
send(Messages::GPUConnectionToWebProcess::CreateRenderingBackend(m_renderingBac
kendIdentifier), 0);

Comment doesn't add anything.

> Source/WebKit/WebProcess/GPU/graphics/RemoteRenderingBackendProxy.cpp:101
> +    connectToGPUProcess();
> +
> +    for (auto& pair : m_remoteResourceCacheProxy.imageBuffers()) {
> +	   if (auto& baseImageBuffer = pair.value) {
> +	       if (is<AcceleratedRemoteImageBufferProxy>(*baseImageBuffer))
> +		   recreateImageBuffer(*this,
downcast<AcceleratedRemoteImageBufferProxy>(*baseImageBuffer), pair.key,
m_renderingBackendIdentifier);
> +	       else
> +		   recreateImageBuffer(*this,
downcast<UnacceleratedRemoteImageBufferProxy>(*baseImageBuffer), pair.key,
m_renderingBackendIdentifier);
> +	   }
> +    }

It's weird to see this code in a "didClose" function. It would be nicer to move
this to a "reestablishGPUConnection" function.