commit 7fdb7ea872246b1d36308d1f2d016b8fffff7e40
author Jean-Philippe Gravel <jpgravel@chromium.org> Mon Apr 07 16:23:31 2025
committer Chromium LUCI CQ <chromium-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon Apr 07 16:23:31 2025
tree 92c95020a276a34ffcaa076c0366f7d7499bfd2c
parent 557c11c3411c0f4f4db83f77756facd6e665278b

Remove the CcLayer check in RecreateCRPFor2DContext

The function RecreateCanvasResourceProviderFor2DContext uses a very
cryptic condition where it aborts if CcLayer() is set. This check
originates from https://codereview.chromium.org/1372463002, where the
presence of m_layer (as it was called backed then) was used as a
proxy to check if the canvas was GPU accelerated. The idea was that
if m_layer was valid but m_surface was not, it was because the
context was lost and a surface shouldn't be recreated until the
context is later restored.

The concern still exists, but the surrounding code change a lot since
then. We now clear the layer as soon as we lose the context, via
LoseContext() -> DiscardResourceProvider() -> ResetLayer() [1]. We
also don't recreate the resource provider (formally known as
m_surface) if the context is lost [2].

There is still a way we can hit the CcLayer() early abort though. If
the GPU context was lost in the middle of render task, the provider
could become invalid. Until NotifyGpuContextLost is later called in a
following task, we'll have an invalid provider but a valid CcLayer.
It turns out that everything would still work even without the CcLayer
check tough, because if we removed it, the following call to
GetOrCreateCanvasResourceProviderImpl would simply return because
a provider already exists, and RecreateCRPFor2DContext would then
return nullptr because the provider would still be invalid.

Regardless, it's worth bringing attention to this particular edge
case. Instead of a cryptic CcLayer check though, the intent is
clearer if we abort when context isn't lost but the provider is
invalid (with an appropriate comment describing the situation). In
the future, we could decide to change this logic to lose canvas
context and start the restore sequence as soon as we observe the
provider go invalid.

[1]: https://crsrc.org/c/third_party/blink/renderer/modules/canvas/canvas2d/canvas_rendering_context_2d.cc;l=226;drc=78883e6c824fd669688b9943096a7b01f87c1cb5

[2]: https://crsrc.org/c/third_party/blink/renderer/core/html/canvas/html_canvas_element.cc;l=2127;drc=78883e6c824fd669688b9943096a7b01f87c1cb5

Bug: 346766781
Change-Id: I3846b92a369f2d21dfeed66ac1e5aa3f911c82c7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6299497
Reviewed-by: Colin Blundell <blundell@chromium.org>
Commit-Queue: Jean-Philippe Gravel <jpgravel@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1443532}