Bug 236965 - After losing context due to too many contexts, getError() does not return CONTEXT_LOST_WEBGL
Summary: After losing context due to too many contexts, getError() does not return CON...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kimmo Kinnunen
URL:
Keywords: InRadar
Depends on:
Blocks: 236964
  Show dependency tree / graph
 
Reported: 2022-02-21 03:44 PST by Kimmo Kinnunen
Modified: 2022-03-23 10:13 PDT (History)
11 users (show)

See Also:

Attachments
Patch (33.17 KB, patch)
2022-03-17 12:01 PDT, Kimmo Kinnunen 		no flags Details | Formatted Diff | Diff
Patch (20.77 KB, patch)
2022-03-22 07:02 PDT, Kimmo Kinnunen 		no flags Details | Formatted Diff | Diff
Patch (22.20 KB, patch)
2022-03-22 07:10 PDT, Kimmo Kinnunen 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Kimmo Kinnunen 2022-02-21 03:44:14 PST 
After losing context due to too many contexts, getError() does not return CONTEXT_LOST_WEBGL
Comment 1 Radar WebKit Bug Importer 2022-02-28 03:45:26 PST 
<rdar://problem/89550905>
Comment 2 Kimmo Kinnunen 2022-03-17 12:01:38 PDT 
Created attachment 454999 [details]
Patch
Comment 3 Kimmo Kinnunen 2022-03-22 07:02:49 PDT 
Created attachment 455371 [details]
Patch
Comment 4 Kimmo Kinnunen 2022-03-22 07:10:12 PDT 
Created attachment 455373 [details]
Patch
Comment 5 Kenneth Russell 2022-03-22 17:17:36 PDT 
Comment on attachment 455373 [details]
Patch

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

Seems fine given that the tests progress. If it's possible to add more WebGL conformance tests covering this area, please do. r+

> Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:7754
> +        ASSERT_NOT_REACHED();

Just checking - this assert is not hit if the WEBGL_lose_context extension is used incorrectly? (restoreContext() is called, but preventDefault() wasn't called against the lost context event?) I assume that WEBGL_lose_context's validation will catch this misuse - and that the EWS bots are running enough WebGL conformance tests using the WEBGL_lose_context extension.
Comment 6 Kimmo Kinnunen 2022-03-23 09:38:39 PDT 
Comment on attachment 455373 [details]
Patch

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

thanks fro spending the time!

> Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:6489
>          return;

this should prevent the case of calling lose_context.restoreContext() without preventDefault()

>> Source/WebCore/html/canvas/WebGLRenderingContextBase.cpp:7754
>> +        ASSERT_NOT_REACHED();
> 
> Just checking - this assert is not hit if the WEBGL_lose_context extension is used incorrectly? (restoreContext() is called, but preventDefault() wasn't called against the lost context event?) I assume that WEBGL_lose_context's validation will catch this misuse - and that the EWS bots are running enough WebGL conformance tests using the WEBGL_lose_context extension.

Yeah, that's the intention.
E.g. the timer invocation to call maybeRestoreContext should always be prefixed with the check "restoreRequested".
Your question should be handled above..
Comment 7 EWS 2022-03-23 10:13:22 PDT 
Committed r291749 (248781@main): <https://commits.webkit.org/248781@main>

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