Fredrik Söderquist (Gerrit)

unread,

Dec 1, 2025, 8:58:57 AM (9 days ago) Dec 1

to Jean-Philippe Gravel, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Jean-Philippe Gravel

Fredrik Söderquist voted and added 1 comment

Votes added by Fredrik Söderquist

Auto-Submit

+1

1 comment

File third_party/blink/web_tests/external/wpt/html/canvas/tools/yaml/the-canvas-state.yaml

Line 160, Patchset 3:- name: 2d.state.saverestore.zerosize

Jean-Philippe Gravel . resolved

This test is essentially the same as the one above (`2d.state.saverestore`), but it's implemented differently (using the `{% for ... %}` unrolling of the test below).
Note that the only reason that the test below uses a Jinja `for` statement is because these tests were migrated from an old `meta.yaml` definition (in https://crrev.com/c/5593168) and the migration only targeted the YAML file, preserving the generated files untouched (with all their technical debt). That original test definition generated different test files for the `2d.state.saverestore` test, but bundled all of the test cases of `2d.canvas.host.initial.reset.2dstate` into a single file. This is what was reproduced in this current YAML config, using test variants in the former and a Jinja `for` loop in the latter. Ideally, we'd follow-up with a cleanup to implement both tests the same way.
That being said, it's usually preferable to have atomic tests, each validating a single behavior. For instance, test failures are harder to diagnose if multiple behaviors are bundled into the same test body. One nice thing about using a test generator is that we can generate atomic tests for a large number of test inputs, with minimal boilerplate. In that sense, test variants are generally better.
Here, what about extending the above test by adding a new dimension to the variant definition? The test generator can either put test variant dimensions in different files, or all in the same file. The following would produce test files with two `test(t => {...})` instances, one with a normal canvas size and another with a zero size:
```
- name: 2d.state.saverestore
  desc: >-
    save()/restore() works for {{ variant_names[1] }}, with a canvas size of
    {{ size }}
  code: |
    // Test that restore() undoes any modifications
    var old = ctx.{{ variant_names[1] }};
    [...]
    ctx.restore();
  variants_layout: [single_file, multi_files]
  variants:
  - non-zero-size:
      size: [100, 50]
    zero-size:
      size: [0, 0]
  - &2d_state_test_cases    
    [...]
```
(Make sure you update all `variant_names[0]` to `variant_names[1]`.)
You'll need to sync passed https://crrev.com/c/7210510 for the above to work correctly (I just submitted that CL to fix the `size` handling).
See here for more details on test variants:
https://chromium.googlesource.com/chromium/src.git/+/master/third_party/blink/web_tests/external/wpt/html/canvas/tools/README.md#test-variants

Fredrik Söderquist

Done

Open in Gerrit

Related details

Attention is currently required from:

Jean-Philippe Gravel

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Jean-Philippe Gravel (Gerrit)

unread,

Dec 1, 2025, 10:50:23 AM (9 days ago) Dec 1

to Fredrik Söderquist, AyeAye, Chromium LUCI CQ, chromium...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Attention needed from Fredrik Söderquist

Jean-Philippe Gravel voted and added 1 comment

Votes added by Jean-Philippe Gravel

Code-Review	+1
Commit-Queue	+2

1 comment

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Jean-Philippe Gravel . resolved

Looks good! Thanks.

Open in Gerrit

Related details

Attention is currently required from:

Fredrik Söderquist

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

Dec 1, 2025, 11:04:37 AM (8 days ago) Dec 1

to Fredrik Söderquist, Jean-Philippe Gravel, AyeAye, chromium...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Chromium LUCI CQ submitted the change

Change information

Commit message:

Fix CanvasRenderingContext2D.restore() on zero-size canvas

Canvas2DRecorderContext::restore() would return without restoring the
state stack if there was no PaintCanvas.

Hoist the operations on the PaintCanvas out of PopAndRestore() into the
respective callers and drop the PaintCanvas argument. Rename to
PopStateStack().

Fixed: 463867637

Change-Id: I8376c9c1622710cb942b17df9357c36effb7bfba

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7205876

Auto-Submit: Fredrik Söderquist <f...@opera.com>

Reviewed-by: Jean-Philippe Gravel <jpgr...@chromium.org>

Commit-Queue: Jean-Philippe Gravel <jpgr...@chromium.org>

Cr-Commit-Position: refs/heads/main@{#1552155}

Files:

M third_party/blink/renderer/modules/canvas/canvas2d/canvas_2d_recorder_context.cc
M third_party/blink/renderer/modules/canvas/canvas2d/canvas_2d_recorder_context.h
M third_party/blink/web_tests/external/wpt/html/canvas/element/the-canvas-state/2d.state.saverestore.fillStyle.html
M third_party/blink/web_tests/external/wpt/html/canvas/element/the-canvas-state/2d.state.saverestore.font.html
M third_party/blink/web_tests/external/wpt/html/canvas/element/the-canvas-state/2d.state.saverestore.globalAlpha.html
M third_party/blink/web_tests/external/wpt/html/canvas/element/the-canvas-state/2d.state.saverestore.globalCompositeOperation.html
M third_party/blink/web_tests/external/wpt/html/canvas/element/the-canvas-state/2d.state.saverestore.lineCap.html
M third_party/blink/web_tests/external/wpt/html/canvas/element/the-canvas-state/2d.state.saverestore.lineJoin.html
M third_party/blink/web_tests/external/wpt/html/canvas/element/the-canvas-state/2d.state.saverestore.lineWidth.html
M third_party/blink/web_tests/external/wpt/html/canvas/element/the-canvas-state/2d.state.saverestore.miterLimit.html
M third_party/blink/web_tests/external/wpt/html/canvas/element/the-canvas-state/2d.state.saverestore.shadowBlur.html
M third_party/blink/web_tests/external/wpt/html/canvas/element/the-canvas-state/2d.state.saverestore.shadowColor.html
M third_party/blink/web_tests/external/wpt/html/canvas/element/the-canvas-state/2d.state.saverestore.shadowOffsetX.html
M third_party/blink/web_tests/external/wpt/html/canvas/element/the-canvas-state/2d.state.saverestore.shadowOffsetY.html
M third_party/blink/web_tests/external/wpt/html/canvas/element/the-canvas-state/2d.state.saverestore.strokeStyle.html
M third_party/blink/web_tests/external/wpt/html/canvas/element/the-canvas-state/2d.state.saverestore.textAlign.html
M third_party/blink/web_tests/external/wpt/html/canvas/element/the-canvas-state/2d.state.saverestore.textBaseline.html
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.fillStyle.html
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.fillStyle.worker.js
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.globalAlpha.html
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.globalAlpha.worker.js
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.globalCompositeOperation.html
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.globalCompositeOperation.worker.js
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.lineCap.html
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.lineCap.worker.js
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.lineJoin.html
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.lineJoin.worker.js
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.lineWidth.html
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.lineWidth.worker.js
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.miterLimit.html
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.miterLimit.worker.js
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.shadowBlur.html
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.shadowBlur.worker.js
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.shadowColor.html
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.shadowColor.worker.js
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.shadowOffsetX.html
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.shadowOffsetX.worker.js
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.shadowOffsetY.html
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.shadowOffsetY.worker.js
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.strokeStyle.html
M third_party/blink/web_tests/external/wpt/html/canvas/offscreen/the-canvas-state/2d.state.saverestore.strokeStyle.worker.js
M third_party/blink/web_tests/external/wpt/html/canvas/tools/yaml/the-canvas-state.yaml

Change size: XL

Delta: 42 files changed, 1100 insertions(+), 476 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Jean-Philippe Gravel

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Blink W3C Test Autoroller (Gerrit)

unread,

Dec 1, 2025, 12:21:28 PM (8 days ago) Dec 1

to Chromium LUCI CQ, Fredrik Söderquist, Jean-Philippe Gravel, AyeAye, chromium...@chromium.org, blink-rev...@chromium.org, blink-revie...@chromium.org, blink-...@chromium.org

Message from Blink W3C Test Autoroller

The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/56369

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Reply all

Reply to author

Forward

Fix CanvasRenderingContext2D.restore() on zero-size canvas [chromium/src : main]

Fredrik Söderquist (Gerrit)

Fredrik Söderquist voted and added 1 comment

Votes added by Fredrik Söderquist

1 comment

Related details

Jean-Philippe Gravel (Gerrit)

Jean-Philippe Gravel voted and added 1 comment

Votes added by Jean-Philippe Gravel

1 comment

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Change information

Blink W3C Test Autoroller (Gerrit)

Message from Blink W3C Test Autoroller

Related details