cc: Lock resources directly in CALayerOverlay

cc: Lock resources directly in CALayerOverlay

When we move the CALayer tree to the browser process, we will want to
ensure that we can access the GpuMemoryBuffer for a resource before we
commit to representing it as a CALayerOverlay. This patch enables that.

BUG=604052
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

[ccameron, 2016-05-26 00:15:07]

Description was changed from

==========
cc: Lock resources directly in CALayerOverlay

When we move the CALayer tree to the browser process, we will want to
ensure that we can access the GpuMemoryBuffer for a resource before we
commit to representing it as a CALayerOverlay. This patch enables that.

BUG=604052
==========

to

==========
cc: Lock resources directly in CALayerOverlay

When we move the CALayer tree to the browser process, we will want to
ensure that we can access the GpuMemoryBuffer for a resource before we
commit to representing it as a CALayerOverlay. This patch enables that.

BUG=604052
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[ccameron, 2016-05-26 00:16:37]

ccameron@chromium.org changed reviewers:
+ erikchen@chromium.org

[ccameron, 2016-05-26 00:16:38]

ptal

[erikchen, 2016-05-26 00:32:01]

lgtm

https://codereview.chromium.org/2017473002/diff/1/cc/output/ca_layer_overlay.cc
File cc/output/ca_layer_overlay.cc (right):

https://codereview.chromium.org/2017473002/diff/1/cc/output/ca_layer_overlay....
cc/output/ca_layer_overlay.cc:41: bool SetResource(ResourceProvider*
resource_provider,
I think the conditional should live outside the function. I don't really see the
point of this function. [It's doing too many things. SetResource should set a
resource, not conditionally set a resource.]

if (!resource_provider->IsOverlayCandidate(resource_id))	
       return CA_LAYER_FAILED_STREAM_VIDEO_NOT_CANDIDATE;

ca_layer_overlay->resource_lock = new
ResourceProvider::ScopedReadLockGpuMemoryBuffer(resource_provider, resource_id);

https://codereview.chromium.org/2017473002/diff/1/cc/resources/resource_provi...
File cc/resources/resource_provider.h (right):

https://codereview.chromium.org/2017473002/diff/1/cc/resources/resource_provi...
cc/resources/resource_provider.h:349: bool IsOverlayCandidate() const;
not necessary if you choose to follow my suggestion.

[ccameron, 2016-05-26 18:49:15]

Thanks!

https://codereview.chromium.org/2017473002/diff/1/cc/output/ca_layer_overlay.cc
File cc/output/ca_layer_overlay.cc (right):

https://codereview.chromium.org/2017473002/diff/1/cc/output/ca_layer_overlay....
cc/output/ca_layer_overlay.cc:41: bool SetResource(ResourceProvider*
resource_provider,
On 2016/05/26 00:32:01, erikchen wrote:
> I think the conditional should live outside the function. I don't really see
the
> point of this function. [It's doing too many things. SetResource should set a
> resource, not conditionally set a resource.]
> 
> if (!resource_provider->IsOverlayCandidate(resource_id))	
>        return CA_LAYER_FAILED_STREAM_VIDEO_NOT_CANDIDATE;
> 
> ca_layer_overlay->resource_lock = new
> ResourceProvider::ScopedReadLockGpuMemoryBuffer(resource_provider,
resource_id);

But but ... it seems so wasteful to look the same structure up in the hash table
twice in a row!

The hope, though is that the next step will be to check
resource_lock->GetGpuMemoryBuffer, so the IsOverlayCandidate is to be short
lived. Made it not-lived.

[ccameron, 2016-05-26 23:38:05]

The CQ bit was checked by ccameron@chromium.org

[ccameron, 2016-05-26 23:38:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
erikchen@chromium.org
Link to the patchset: https://codereview.chromium.org/2017473002/#ps60001
(title: "Add operator=")

[commit-bot: I haz the power, 2016-05-26 23:38:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2017473002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2017473002/60001

[commit-bot: I haz the power, 2016-05-27 00:43:13]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-27 00:43:14]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)