Bug 221396

Created attachment 419266 [details] Patch

Created attachment 419377 [details] Patch

Created attachment 419701 [details] rebase

Comment on attachment 419701 [details] rebase View in context: https://bugs.webkit.org/attachment.cgi?id=419701&action=review > Source/WebKit/GPUProcess/graphics/RemoteGraphicsContextGL.cpp:59 > + send(Messages::RemoteGraphicsContextGLProxy::WasCreated(true, extensions, requestableExtensions), m_graphicsContextGLIdentifier); What does the 'true' mean? > Source/WebKit/WebProcess/GPU/GPUProcessConnection.cpp:182 > + // Skip messages intended for already removed messageReceiverMap() destinations. > +#if ENABLE(WEBGL) > + if (decoder.messageReceiverName() == Messages::RemoteGraphicsContextGLProxy::messageReceiverName()) > + return true; > +#endif This seems pretty unusual. Do we do this elsewhere? > Source/WebKit/WebProcess/GPU/graphics/RemoteGraphicsContextGLProxy.h:357 > + void disconnectGpuProcessIfNeeded(); > + void abandonGpuProcess(); What's the difference between these? What does "abandon" imply? > Source/WebKit/WebProcess/GPU/graphics/RemoteGraphicsContextGLProxyFunctionsGenerated.cpp:39 > + if (!isContextLost()) { > + auto sendResult = send(Messages::RemoteGraphicsContextGL::SetFailNextGPUStatusCheck()); > + if (!sendResult) > + markContextLost(); > + } There's lot of boilerplate here. I wonder if we can factor this to avoid future mistakes where we forget to check context lost.

Thanks for the review! (In reply to Simon Fraser (smfr) from comment #4) >> + send(Messages::RemoteGraphicsContextGLProxy::WasCreated(true, extensions, requestableExtensions), m_graphicsContextGLIdentifier); > What does the 'true' mean? That the creation was success. I can improve this in future patches. > > Source/WebKit/WebProcess/GPU/GPUProcessConnection.cpp:182 > > + // Skip messages intended for already removed messageReceiverMap() destinations. > > +#if ENABLE(WEBGL) > > + if (decoder.messageReceiverName() == Messages::RemoteGraphicsContextGLProxy::messageReceiverName()) > > + return true; > > +#endif > > This seems pretty unusual. Do we do this elsewhere? No. All sites are pretty buggy wrt various timeout, connection lost and graceful connection shutdown scenarios. > > Source/WebKit/WebProcess/GPU/graphics/RemoteGraphicsContextGLProxy.h:357 > > + void disconnectGpuProcessIfNeeded(); > > + void abandonGpuProcess(); > > What's the difference between these? What does "abandon" imply? One is where we disconnect the connection between the proxy and the object it proxies. Other is where the disconnect has happened (e.g. a timeout) and we just forget the object we proxied. > > > Source/WebKit/WebProcess/GPU/graphics/RemoteGraphicsContextGLProxyFunctionsGenerated.cpp:39 > > + if (!isContextLost()) { > > + auto sendResult = send(Messages::RemoteGraphicsContextGL::SetFailNextGPUStatusCheck()); > > + if (!sendResult) > > + markContextLost(); > > + } > > There's lot of boilerplate here. I wonder if we can factor this to avoid > future mistakes where we forget to check context lost. Good idea, I'll change this in future commit.

Committed r272645: <https://commits.webkit.org/r272645> All reviewed patches have been landed. Closing bug and clearing flags on attachment 419701 [details].

<rdar://problem/74207073>