New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 796838 link

Starred by 2 users

Issue metadata

Status: Verified
Owner:
Last visit > 30 days ago
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Regression



Sign in to add a comment

Regression: Black flashes are seen in Fishbowl page

Project Member Reported by rkalavakuntla@chromium.org, Dec 21 2017

Issue description

Chrome Version: 65.0.3299.0/10234.0.0 dev channel  Daisy,Kip & Reks
OS:Chrome OS

What steps will reproduce the problem?
(1)Sign into user >>Open https://testdrive-archive.azurewebsites.net/performance/fishbowl/default.html and Observe Black flash(please refer video)

Actual:Screen flashes continuosly
Expected:No such black flash should be seen.

This is a Regression issue as same is working fine in 65.0.3286.0/10191.0.0 dev channel daisy

Note:Issue is not seen in Linux,Windows OS



 
Actual.mp4
5.7 MB View Download
Cc: marc...@chromium.org
Labels: Needs-Bisect
Need someone to bisect this ChromeOS issue.

Comment 2 by piman@chromium.org, Jan 12 2018

Owner: marc...@chromium.org
Status: Assigned (was: Untriaged)
->marcheu to find a Chrome OS gfx owner to triage / bisect.
note to self: a good repro + bisect target is braswell/cyan (ARM simple chrome build is broken ATM) with 1 fishbowlie and 1 maps zooming in and out.
2e4a432454ac3f04d6c5fa8f17c49ce3026217d6 is the first bad commit
commit 2e4a432454ac3f04d6c5fa8f17c49ce3026217d6
Author: Justin Novosad <junov@chromium.org>
Date:   Thu Dec 14 22:37:30 2017 +0000

    Replace CanvasResource_Skia with CanvasResource_Bitmap
    
    This change refactors the existing CanvasResource_Skia to make it
    store a StaticBitmapImage instead of an SkImage intenally. This
    simplifies the gpu mailbox management code by re-using the
    functionality provided by StaticBitmapImage. It also bring the code
    closer to unifying OffscreenCanvasResourceProvider with
    CanvasResourceProvider
    
    BUG=788439, 776801 
    TBR=bajones@chromium.org
    
    Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
    Change-Id: Ia57a68e1a61763daa158403e258b89b56a82a6f7
    Reviewed-on: https://chromium-review.googlesource.com/820251
    Commit-Queue: Justin Novosad <junov@chromium.org>
    Reviewed-by: Olivia Lai <xlai@chromium.org>
    Cr-Commit-Position: refs/heads/master@{#524204}

:040000 040000 cf64701c1fa6ef43e5f0db42c4430ac75fe92e01 73aed1adeb81f2c8d433eaf8d881012b82567a74 M      third_party

Cc: piman@chromium.org
Owner: junov@chromium.org
Doesn't seem to revert cleanly... Justin, can you take a look? Thanks!

Comment 6 by junov@chromium.org, Jan 19 2018

Status: Started (was: Assigned)

Comment 7 by junov@chromium.org, Jan 22 2018

Status: Assigned (was: Started)
Am able to repro on kevin with 1000 fish + zoom-in/zoom-out.

Project Member

Comment 8 by bugdroid1@chromium.org, Jan 23 2018

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

commit 86e1b7f9556ded0dbcd745f079a0cb92610c5f6b
Author: Justin Novosad <junov@chromium.org>
Date: Tue Jan 23 21:23:48 2018

Fix 2D canvas flashing black

When CanvasResource was refactored by this CL:
https://chromium.googlesource.com/chromium/src/+/2e4a432454ac3f04d6c5fa8f17c49ce3026217d6
We when from using SkImage to using StaticBitmapImage.  For tracking
texture resources. StaticBitmapImage benefits from a mailbox caching
mechanism implemented in GraphicsContext3DUtils, so when the underlying
GrTexture object is recycled, GraphicsContext3DUtils retrieves the
old mailbox name from its cache.  When CanvasResource was transition
to use StaticBitmapImage we needed to stop disassociating mailbox names
during tear down because it invalidate the mailboxes cached by
GraphicsContext3DUtils.

The test that verifies that mailbox names are not leaked was modified
to expect mailbox disassociation to be triggered by GrTexture
destruction rather than CanvasResource destruction.

BUG= 796838 

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Idff59a57a5acee08a7b7573e3b34593aa1938fa5
Reviewed-on: https://chromium-review.googlesource.com/881304
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Commit-Queue: Justin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/heads/master@{#531352}
[modify] https://crrev.com/86e1b7f9556ded0dbcd745f079a0cb92610c5f6b/third_party/WebKit/Source/platform/graphics/CanvasResource.cpp
[modify] https://crrev.com/86e1b7f9556ded0dbcd745f079a0cb92610c5f6b/third_party/WebKit/Source/platform/graphics/CanvasResourceTest.cpp

Comment 9 by junov@chromium.org, Jan 23 2018

Status: Fixed (was: Assigned)
Labels: Merge-TBD
[Auto-generated comment by a script] We noticed that this issue is targeted for M-65; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-65 label, otherwise remove Merge-TBD label. Thanks.
Cc: bhthompson@chromium.org
Cc: hsiangc@chromium.org vsu...@chromium.org
The issue still reproduce on M-65
Labels: Merge-Approved-65
Put up a cherrypick of this at https://chromium-review.googlesource.com/#/c/chromium/src/+/907208/

If this is good to merge back please land it.
Project Member

Comment 15 by bugdroid1@chromium.org, Feb 7 2018

Labels: -merge-approved-65 merge-merged-3325
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3ffbc5859a88276f0c64d2448772f267fd958f6c

commit 3ffbc5859a88276f0c64d2448772f267fd958f6c
Author: Justin Novosad <junov@chromium.org>
Date: Wed Feb 07 18:13:34 2018

Fix 2D canvas flashing black

When CanvasResource was refactored by this CL:
https://chromium.googlesource.com/chromium/src/+/2e4a432454ac3f04d6c5fa8f17c49ce3026217d6
We when from using SkImage to using StaticBitmapImage.  For tracking
texture resources. StaticBitmapImage benefits from a mailbox caching
mechanism implemented in GraphicsContext3DUtils, so when the underlying
GrTexture object is recycled, GraphicsContext3DUtils retrieves the
old mailbox name from its cache.  When CanvasResource was transition
to use StaticBitmapImage we needed to stop disassociating mailbox names
during tear down because it invalidate the mailboxes cached by
GraphicsContext3DUtils.

The test that verifies that mailbox names are not leaked was modified
to expect mailbox disassociation to be triggered by GrTexture
destruction rather than CanvasResource destruction.

BUG= 796838 

(cherry picked from commit 86e1b7f9556ded0dbcd745f079a0cb92610c5f6b)

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Idff59a57a5acee08a7b7573e3b34593aa1938fa5
Reviewed-on: https://chromium-review.googlesource.com/881304
Reviewed-by: Fernando Serboncini <fserb@chromium.org>
Commit-Queue: Justin Novosad <junov@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#531352}
Reviewed-on: https://chromium-review.googlesource.com/907208
Reviewed-by: Justin Novosad <junov@chromium.org>
Cr-Commit-Position: refs/branch-heads/3325@{#363}
Cr-Branched-From: bc084a8b5afa3744a74927344e304c02ae54189f-refs/heads/master@{#530369}
[modify] https://crrev.com/3ffbc5859a88276f0c64d2448772f267fd958f6c/third_party/WebKit/Source/platform/graphics/CanvasResource.cpp
[modify] https://crrev.com/3ffbc5859a88276f0c64d2448772f267fd958f6c/third_party/WebKit/Source/platform/graphics/CanvasResourceTest.cpp

Labels: -Merge-TBD
Labels: Merge-TBD

Comment 18 by junov@chromium.org, Feb 12 2018

Why Merge-TBD?  I think the work is done here.
Status: Verified (was: Fixed)
Issue is fixed and verified with M-65 build 10323.30.0/65.0.3325.65 on kip, daisy and reks

Comment 20 by junov@chromium.org, Feb 13 2018

Cc: y...@chromium.org junov@chromium.org mkarkada@chromium.org shenghao@google.com abod...@chromium.org dhadd...@chromium.org
 Issue 810115  has been merged into this issue.

Comment 21 by junov@chromium.org, Feb 19 2018

Labels: -Merge-TBD

Sign in to add a comment