Bug 233517

Summary:

GPUP GraphicsContextGL creation failure cannot be detected

Product:

WebKit

Reporter:

Kimmo Kinnunen <kkinnunen>

Component:

WebGL

Assignee:

Kimmo Kinnunen <kkinnunen>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

annulen, cdumez, changseok, cmarcelo, crzwdjk, dino, esprehn+autocc, ews-watchlist, gyuyoung.kim, kbr, kkinnunen, koivisto, kondapallykalyan, lmoura, luiz, ryuan.choi, sergio, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Local Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=233712

Bug Depends on:

Bug Blocks:

221664

Attachments:

Description	Flags
Patch	ews-feeder: commit-queue-
Patch	none
Patch	none
Patch for landing	ews-feeder: commit-queue-
Patch for landing	none
Patch for landing	none

Description Kimmo Kinnunen 2021-11-26 03:35:05 PST

GPUP GraphicsContextGL creation failure cannot be detected

The WebGLRenderingContextBase first tries to create GPUP GraphicsContextGL. If this fails, it tries to create WP GraphicsContextGL. This is not consistent.
ChromeClient should decide what type to instantiate.

Comment 1 Kimmo Kinnunen 2021-11-26 05:44:38 PST

Created attachment 445194 [details]
Patch

Comment 2 Kimmo Kinnunen 2021-11-26 06:02:14 PST

Created attachment 445197 [details]
Patch

Comment 3 Kimmo Kinnunen 2021-11-26 06:33:17 PST

Created attachment 445199 [details]
Patch

Comment 4 Antti Koivisto 2021-11-29 01:16:56 PST

Comment on attachment 445199 [details]
Patch

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

> Source/WebCore/ChangeLog:22
> +        Fix a bug where GPUP WebGL would use in-process context if the lost context would be recreated.
> +
> +        No new tests, a refactor.

Not quite a pure refactoring if it fixes a bug.

Comment 5 Antti Koivisto 2021-11-29 01:19:15 PST

Comment on attachment 445199 [details]
Patch

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

> Source/WebCore/platform/graphics/WebProcessGraphicsContextGL.h:37
> +#include "GraphicsContextGL.h"
> +
> +#include <wtf/RefCounted.h>
> +
> +namespace WebCore {
> +
> +WEBCORE_EXPORT RefPtr<GraphicsContextGL> createWebProcessGraphicsContextGL(const GraphicsContextGLAttributes&);

I guess this function could also just go to GraphicsContextGL.h?

Comment 6 Kimmo Kinnunen 2021-11-29 01:57:50 PST

Created attachment 445258 [details]
Patch for landing

Comment 7 Kimmo Kinnunen 2021-11-29 02:43:14 PST

Created attachment 445262 [details]
Patch for landing

Comment 8 Kimmo Kinnunen 2021-11-29 04:06:27 PST

Created attachment 445265 [details]
Patch for landing

Comment 9 EWS 2021-11-29 05:05:37 PST

Committed r286209 (244591@main): <https://commits.webkit.org/244591@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 445265 [details].

Comment 10 Radar WebKit Bug Importer 2021-11-29 05:06:26 PST

<rdar://problem/85811767>

Comment 11 Arcady Goldmints-Orlov 2021-12-01 09:23:46 PST

This change has caused a number of crashes in offscreen canvas tests on GTK and WPE (which only run on GTK and WPE):
  http/wpt/offscreen-canvas/getContext-webgl.html [ Crash ]
  http/wpt/offscreen-canvas/transferToImageBitmap-webgl.html [ Crash ]
  imported/w3c/web-platform-tests/html/canvas/offscreen/manual/the-offscreen-canvas/offscreencanvas.getcontext.html [ Crash ]
  imported/w3c/web-platform-tests/html/canvas/offscreen/manual/the-offscreen-canvas/offscreencanvas.getcontext.worker.html [ Crash ]
  imported/w3c/web-platform-tests/html/canvas/offscreen/manual/the-offscreen-canvas/offscreencanvas.resize.html [ Crash ]
  imported/w3c/web-platform-tests/html/canvas/offscreen/manual/the-offscreen-canvas/offscreencanvas.transfer.to.imagebitmap.html [ Crash ]
  imported/w3c/web-platform-tests/html/canvas/offscreen/manual/the-offscreen-canvas/offscreencanvas.transfer.to.imagebitmap.w.html [ Crash ]
  imported/w3c/web-platform-tests/html/canvas/offscreen/manual/the-offscreen-canvas/offscreencanvas.transferrable.html [ Crash ]
  imported/w3c/web-platform-tests/html/canvas/offscreen/manual/the-offscreen-canvas/offscreencanvas.transferrable.w.html [ Crash ]

With a backtrace like the following:
#0  0x00007f806d263347 in WebCore::WebGLRenderingContextBase::create(WebCore::CanvasBase&, WebCore::GraphicsContextGLAttributes&, WebCore::GraphicsContextGLWebGLVersion) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#1  0x00007f806d1e9ccc in WebCore::OffscreenCanvas::createContextWebGL(WebCore::OffscreenCanvas::RenderingContextType, WebCore::GraphicsContextGLAttributes&&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#2  0x00007f806d1ea02e in WebCore::OffscreenCanvas::getContext(JSC::JSGlobalObject&, WebCore::OffscreenCanvas::RenderingContextType, WTF::Vector<JSC::Strong<JSC::Unknown, (JSC::ShouldStrongDestructorGrabLock)0>, 0ul, WTF::CrashOnOverflow, 16ul, WTF::FastMalloc>&&) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#3  0x00007f806c2e0447 in WebCore::jsOffscreenCanvasPrototypeFunction_getContext(JSC::JSGlobalObject*, JSC::CallFrame*) () at /app/webkit/WebKitBuild/Release/lib/libwebkit2gtk-4.1.so.0
#4  0x00007f8021cff1d8 in  ()
#5  0x00007fff5faf3cb0 in  ()
#6  0x00007f80682dc20d in op_call_slow_return_location () at /app/webkit/WebKitBuild/Release/lib/libjavascriptcoregtk-4.1.so.0
#7  0x0000000000000000 in  ()

Not having debugged this in depth, my suspicion is that this change is not taking into account the offscreen case where hostWindow is null.