Bug 223121

Summary: Simulated WebGL context screen change events should work with GPU process
Product: WebKit Reporter: Kimmo Kinnunen <kkinnunen>
Component: WebGLAssignee: Kimmo Kinnunen <kkinnunen>
Status: RESOLVED FIXED    
Severity: Normal CC: darin, dino, kbr, kkinnunen, pvollan, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 217211, 219669    
Attachments:
Description Flags
Patch
none
Patch
ews-feeder: commit-queue-
Patch
none
Patch none

Description Kimmo Kinnunen 2021-03-12 07:50:36 PST
Simulated WebGL context screen change events should work with GPU process

Bug 219669 tried to fix it, but did not succeed

fails:
     fast/canvas/webgl/webglcontextchangedevent.html
Comment 1 Kimmo Kinnunen 2021-03-12 07:54:24 PST
Created attachment 423048 [details]
Patch
Comment 2 Kimmo Kinnunen 2021-03-12 10:33:46 PST
Created attachment 423060 [details]
Patch
Comment 3 Kimmo Kinnunen 2021-03-12 11:07:10 PST
Created attachment 423067 [details]
Patch
Comment 4 Darin Adler 2021-03-12 15:10:36 PST
Comment on attachment 423067 [details]
Patch

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

> Source/WebKit/GPUProcess/GPUConnectionToWebProcess.h:118
> +#if ENABLE(WEBGL)
> +    void dispatchDisplayWasReconfiguredForTesting() { dispatchDisplayWasReconfigured(); };
> +#endif

Style-wise, a while back we found that using boolean expressions and separate #if was clearer, and easier to keep correct when refactoring than nested #if. So this would be nice as:

#endif
#if PLATFORM(MAC) && ENABLE(WEBGL)
    // ...
#endif

Please consider that form. I’d suggest that above in the .cpp file too for the same reason.

> Source/WebKit/GPUProcess/graphics/RemoteGraphicsContextGL.cpp:281
> +        callOnMainRunLoop([gpuConnectionToWebProcess = m_gpuConnectionToWebProcess]() {
> +            if (auto connectionToWeb = gpuConnectionToWebProcess.get())
> +                connectionToWeb->dispatchDisplayWasReconfiguredForTesting();
> +        });

You don’t have to take my suggestions, but I think the best names for these two would be: weakConnection (the capture) and connection (the local).
Comment 5 Kimmo Kinnunen 2021-03-15 03:26:12 PDT
Created attachment 423152 [details]
Patch
Comment 6 EWS 2021-03-15 04:33:34 PDT
Committed r274412: <https://commits.webkit.org/r274412>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 423152 [details].
Comment 7 Radar WebKit Bug Importer 2021-03-15 04:34:16 PDT
<rdar://problem/75425294>