If this is indeed dead code, then this change is not web exposed, and hence no LGTMs are needed
On Thu, Nov 17, 2022 at 11:12 AM 'Christopher Cameron' via blink-dev < blink-dev@chromium.org> wrote: > On Wed, Nov 16, 2022 at 8:57 PM Mike Taylor <miketa...@chromium.org> > wrote: > >> If we're not 100% confident in the safety of removal then I'd suggest >>> landing a simple UseCounter now, and removing in the next milestone. You >>> could probably even get the UseCounter merged into M109 if you want, then >>> land removal for M110 now. But I'm open to an argument that this is >>> definitely unnecessary extra work. >>> >> > I was about to write up a case-by-case discussion, but I wanted to make > sure that I was accurately representing things. So I started writing tests > for this, and found that it's actually all dead code. Removing these > parameters from the public API is a no-op. > > The only function that takes ImageEncodeOptions is > OffscreenCanvas::convertToBlob. But, we only pay attention to > ImageEncodeOptions if the calling function identifies itself > as kHTMLCanvasConvertToBlobPromise > <https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/canvas/canvas_async_blob_creator.cc;l=216;drc=b75fd5075a90ada27a3360df23fa08c83e24e995> > (that > is, not that one function). When we trace this back, we see that offscreen > canvases do identify themselves correctly > <https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/canvas/canvas_rendering_context_host.cc;l=377;drc=b75fd5075a90ada27a3360df23fa08c83e24e995> > . > > And, in fact, we have at least one WPT test > <https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/html/canvas/offscreen/manual/wide-gamut-canvas/2d.color.space.p3.convertToBlobp3.canvas.html;l=43;drc=271b65d96f95b73b8678a5fd90b9ac9c7dc6d225> > that > verifies that we ignore these parameters. > Can you outline how this test is testing that we're ignoring these parameters? I'm not sure I get it.. Also, Safari seems to be failing <https://wpt.fyi/results/html/canvas/offscreen/manual/wide-gamut-canvas/2d.color.space.p3.convertToBlobp3.canvas.html?label=master&label=experimental&aligned> that test.. May be nice to add dedicated tests for both of these parameters to ensure they are ignored before deleting them. > -- > You received this message because you are subscribed to the Google Groups > "blink-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to blink-dev+unsubscr...@chromium.org. > To view this discussion on the web visit > https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAGnfxj9rOCB-Fy6obWWJB_AeFKZ6cCAkfZf16v8s0jgiAtDyTg%40mail.gmail.com > <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAGnfxj9rOCB-Fy6obWWJB_AeFKZ6cCAkfZf16v8s0jgiAtDyTg%40mail.gmail.com?utm_medium=email&utm_source=footer> > . > -- You received this message because you are subscribed to the Google Groups "blink-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+unsubscr...@chromium.org. To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAL5BFfULAevDnwp3DLnR%3De4rU34Xu%3DpHBmoAAGx3BNhE9AZ1mA%40mail.gmail.com.
