New issue
Advanced search Search tips

Issue 895551 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Oct 22
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 2
Type: Bug

Blocking:
issue 839970



Sign in to add a comment

lowLatency canvas, getContext() not idempotent

Project Member Reported by mcasas@chromium.org, Oct 15

Issue description

on ToT, with chrome://flags/#enable-experimental-web-platform-features
on, I do:

> cll =document.createElement('canvas', {lowLatency: true})
> ctxll = cll.getContext('2d')
> ctxll2 = cll.getContext('2d')

I get a console error 
"Cannot get context from a canvas that has transferred its control to offscreen."

coming from
https://cs.chromium.org/chromium/src/third_party/blink/renderer/modules/canvas/htmlcanvas/html_canvas_element_module.cc?type=cs&sq=package:chromium&g=0&l=25

but with non-lowLatency canvas contexts I can do that:

> c =document.createElement('canvas')
> ctx = c.getContext('2d')
> ctx2 = c.getContext('2d')
> ctx === ctx2
 true

It's probably a leftover from the days when low latency was welded with
offscreen code.
 
Owner: fs...@chromium.org
Status: Assigned (was: Available)
fserb, can you TAL?
I can prob fix this is ~1 week from now.
mcasas, should you be Owner on this?
Cc: -mcasas@chromium.org fs...@chromium.org
Owner: mcasas@chromium.org
Project Member

Comment 6 by bugdroid1@chromium.org, Oct 22

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f352608ae26967ef5adc15397f451652aabc2a3a

commit f352608ae26967ef5adc15397f451652aabc2a3a
Author: Miguel Casas <mcasas@chromium.org>
Date: Mon Oct 22 18:14:47 2018

Canvas LowLatency: make getContext() idempotent

Canvas.getContext() method is idempotent, i.e. successive calls return
the same context over and over. When the context is lowLatency, however,
we think it's an OffscreenCanvas context and fail. This CL fixes
that and adds a LayoutTest to verify the new functionality.

Bug:  895551 
Change-Id: I1ae140080915a277eb45ac01486e6c7c1464aefa
Reviewed-on: https://chromium-review.googlesource.com/c/1294090
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601652}
[add] https://crrev.com/f352608ae26967ef5adc15397f451652aabc2a3a/third_party/WebKit/LayoutTests/fast/canvas-api/canvas-lowlatency-getContext.html
[modify] https://crrev.com/f352608ae26967ef5adc15397f451652aabc2a3a/third_party/blink/renderer/modules/canvas/htmlcanvas/html_canvas_element_module.cc

Status: Fixed (was: Assigned)
72.0.3589.0
Labels: -Pri-3 Merge-Request-71 OS-Android OS-Chrome OS-Linux OS-Mac OS-Windows Pri-2
Requesting merge-to-71, it's a low-risk change that only affects
the lowLatency code path (on an Origin Trial 71-72-73)
Project Member

Comment 11 by sheriffbot@chromium.org, Oct 25

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
How is the change looking in canary? Is it safe to merge now? Is the change behind Finch?
#12: yes, the codepath is only applicable to lowLatency canvas contexts, 
and those are controlled by an OriginTrial.
Labels: -Merge-Review-71 Merge-Approved-71
Approving merge to M71 branch 3578 based on comment #13. Please merge ASAP.
Project Member

Comment 15 by bugdroid1@chromium.org, Oct 25

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3448078b954a637fc80ec0bea9c8c3326d1ae81c

commit 3448078b954a637fc80ec0bea9c8c3326d1ae81c
Author: Miguel Casas <mcasas@chromium.org>
Date: Thu Oct 25 16:36:55 2018

Canvas LowLatency: make getContext() idempotent

Canvas.getContext() method is idempotent, i.e. successive calls return
the same context over and over. When the context is lowLatency, however,
we think it's an OffscreenCanvas context and fail. This CL fixes
that and adds a LayoutTest to verify the new functionality.

Bug:  895551 
Change-Id: I1ae140080915a277eb45ac01486e6c7c1464aefa
Reviewed-on: https://chromium-review.googlesource.com/c/1294090
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#601652}(cherry picked from commit f352608ae26967ef5adc15397f451652aabc2a3a)
Reviewed-on: https://chromium-review.googlesource.com/c/1299437
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#322}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[add] https://crrev.com/3448078b954a637fc80ec0bea9c8c3326d1ae81c/third_party/WebKit/LayoutTests/fast/canvas-api/canvas-lowlatency-getContext.html
[modify] https://crrev.com/3448078b954a637fc80ec0bea9c8c3326d1ae81c/third_party/blink/renderer/modules/canvas/htmlcanvas/html_canvas_element_module.cc

Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/3448078b954a637fc80ec0bea9c8c3326d1ae81c

Commit: 3448078b954a637fc80ec0bea9c8c3326d1ae81c
Author: mcasas@chromium.org
Commiter: mcasas@chromium.org
Date: 2018-10-25 16:36:55 +0000 UTC

Canvas LowLatency: make getContext() idempotent

Canvas.getContext() method is idempotent, i.e. successive calls return
the same context over and over. When the context is lowLatency, however,
we think it's an OffscreenCanvas context and fail. This CL fixes
that and adds a LayoutTest to verify the new functionality.

Bug:  895551 
Change-Id: I1ae140080915a277eb45ac01486e6c7c1464aefa
Reviewed-on: https://chromium-review.googlesource.com/c/1294090
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Commit-Queue: Miguel Casas <mcasas@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#601652}(cherry picked from commit f352608ae26967ef5adc15397f451652aabc2a3a)
Reviewed-on: https://chromium-review.googlesource.com/c/1299437
Reviewed-by: Miguel Casas <mcasas@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#322}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Blocking: 839970

Sign in to add a comment