Bug 215433

Summary:

[WebGL2] Pass user-defined-properties-on-context.html layout test

Product:

WebKit

Reporter:

Kenneth Russell <kbr>

Component:

WebGL

Assignee:

Kenneth Russell <kbr>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cdumez, changseok, darin, dino, esprehn+autocc, ews-watchlist, graouts, gyuyoung.kim, hi, jdarpinian, joepeck, kondapallykalyan, mmaxfield, webkit-bug-importer, ysuzuki

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

214765

Bug Blocks:

126404, 189641

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none

Kenneth Russell

Reported 2020-08-12 16:41:17 PDT

With the fix for Bug 214765, in which expando properties are being preserved correctly for objects latched into the WebGL context, the similar layout test: webgl/2.0.0/conformance/context/user-defined-properties-on-context.html is still failing. In order to fix it, HTMLCanvasElements will have to keep the JavaScript wrapper for their context alive.

Attachments
Patch (10.16 KB, patch) 2020-08-17 18:50 PDT, Kenneth Russell	no flags	Details Formatted Diff Diff
Patch (12.33 KB, patch) 2020-08-17 18:56 PDT, Kenneth Russell	no flags	Details Formatted Diff Diff
Patch (17.56 KB, patch) 2020-08-18 13:39 PDT, Kenneth Russell	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Kenneth Russell

Comment 1 2020-08-12 17:04:45 PDT

Also check the following layout test: webgl/2.0.0/resources/webgl_test_files/conformance/context/context-lost-restored.html which looks for expando properties on extensions, and which theoretically should already be passing.

Kenneth Russell

Comment 2 2020-08-17 18:50:38 PDT

Created attachment 406762 [details] Patch

Kenneth Russell

Comment 3 2020-08-17 18:53:08 PDT

ysuzuki@: could you please review this? I'm not sure this is completely correct. My first attempt put the HTMLCanvasElement::renderingContext() into the opaque root set, and that didn't work - JSHTMLCanvasElement::visitAdditionalChildren seemed to be called after the rendering context's isReachableFromOpaqueRoots.

Kenneth Russell

Comment 4 2020-08-17 18:56:07 PDT

Created attachment 406763 [details] Patch

Darin Adler

Comment 5 2020-08-18 08:38:00 PDT

Comment on attachment 406763 [details] Patch I understand how we are taking advantage of the test that’s already present for WebGL 2. But for the project, it would be better if we had test coverage for all three context types.

Yusuke Suzuki

Comment 6 2020-08-18 12:03:02 PDT

Comment on attachment 406763 [details] Patch r=me

Kenneth Russell

Comment 7 2020-08-18 12:31:08 PDT

Thanks darin@ and ysuzuki@ for your reviews. I'll add 2d and webgpu context expando tests. Note that HTMLCanvasElement.getContext("gpu") doesn't exist in the WebGPU spec any more, so I'll add a FIXME in that test about updating it once the implementation matches the spec.

Kenneth Russell

Comment 8 2020-08-18 13:39:30 PDT

Created attachment 406805 [details] Patch

Kenneth Russell

Comment 9 2020-08-18 13:40:01 PDT

Comment on attachment 406805 [details] Patch Added new tests in the latest patch. CQ'ing.

EWS

Comment 10 2020-08-18 14:24:56 PDT

Committed r265833: <https://trac.webkit.org/changeset/265833> All reviewed patches have been landed. Closing bug and clearing flags on attachment 406805 [details].

Radar WebKit Bug Importer

Comment 11 2020-08-18 14:25:24 PDT

<rdar://problem/67352972>

Note You need to log in before you can comment on or make changes to this bug.