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

Issue 632722 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Aug 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Remove blink's dependency on GrTexture::set/getCustomData

Project Member Reported by junov@chromium.org, Jul 29 2016

Issue description

Skia want to remove this unsafe API and remove Grtexture from the public API.  To do so, we need to find alternative to the current usage in Canvas2DLayerBridge, where setCustomData is used to save mailbox name associations.
 

Comment 1 by junov@chromium.org, Jul 29 2016

Cc: xidac...@chromium.org fmalita@chromium.org bsalomon@chromium.org
Project Member

Comment 2 by bugdroid1@chromium.org, Aug 2 2016

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

commit 23b4e88d6090ae9f2b2fa9aa062152900c09f7e9
Author: xidachen <xidachen@chromium.org>
Date: Tue Aug 02 18:04:15 2016

Add a new class AcceleratedStaticBitmap : StaticBitmapImage

At this moment, the StaticBitmapImage holds RefPtr to a SkImage, or a
WebExternalTextureMailbox. So there are in total of three cases:
1). The SkImage is not texture backed.
2). The SkImage is GPU texture backed.
3). The SkImage is null, but has a valid mailbox.

It makes more sense to break this class into two classes. So this CL
adds a new class called AcceleratedStaticBitmap, inherited from the
StaticBitmapImage class, is in charge of case 2 and 3. Case 1 is handled
by the StaticBitmapImage class.

We can use the bots (including the GPU_Optional bots) to make sure this
change doesn't break anything.

BUG= 632722 

Review-Url: https://codereview.chromium.org/2196963002
Cr-Commit-Position: refs/heads/master@{#409238}

[modify] https://crrev.com/23b4e88d6090ae9f2b2fa9aa062152900c09f7e9/third_party/WebKit/Source/core/frame/ImageBitmap.cpp
[modify] https://crrev.com/23b4e88d6090ae9f2b2fa9aa062152900c09f7e9/third_party/WebKit/Source/platform/blink_platform.gypi
[add] https://crrev.com/23b4e88d6090ae9f2b2fa9aa062152900c09f7e9/third_party/WebKit/Source/platform/graphics/AcceleratedStaticBitmapImage.cpp
[add] https://crrev.com/23b4e88d6090ae9f2b2fa9aa062152900c09f7e9/third_party/WebKit/Source/platform/graphics/AcceleratedStaticBitmapImage.h
[modify] https://crrev.com/23b4e88d6090ae9f2b2fa9aa062152900c09f7e9/third_party/WebKit/Source/platform/graphics/StaticBitmapImage.cpp
[modify] https://crrev.com/23b4e88d6090ae9f2b2fa9aa062152900c09f7e9/third_party/WebKit/Source/platform/graphics/StaticBitmapImage.h

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 19 2016

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

commit 645b3cc4697033ab68738dc68d9dfd8db200c008
Author: junov <junov@chromium.org>
Date: Fri Aug 19 13:58:10 2016

Stop using GrTexture::get/setCustomData in Canvas2DLayerBridge

Canvas2DLayerBridge was using custom data to cache mailbox names
associated with recycled textures managed by skia.  This helped
prevent leaking mailboxes.  This CL uses a different approach
that consists in disassociating mailboxes to prevent the mailbox
leak.  The goal is to allow the skia team to proceed with the
deprecation of texture custom data attachments.

BUG= 632722 

Review-Url: https://codereview.chromium.org/2262533002
Cr-Commit-Position: refs/heads/master@{#413129}

[modify] https://crrev.com/645b3cc4697033ab68738dc68d9dfd8db200c008/third_party/WebKit/Source/platform/graphics/Canvas2DLayerBridge.cpp

Comment 4 by junov@chromium.org, Aug 19 2016

Status: Fixed (was: Assigned)
bsalomon: Now that this is done, feel free to go ahead and remove custom data from GrTexture!

Comment 5 by bsalo...@google.com, Aug 19 2016

Thanks!!

Sign in to add a comment