cc: Detect miss-behaving fence extensions when using 1-copy rasterizer.

cc: Detect miss-behaving fence extensions when using 1-copy rasterizer.

This adds a wait_if_needed parameter to
ResourcePool::CheckBusyResources. CheckBusyResources will use existing
ResourceProvider API to wait for any read locks to complete if needed
by a resource prior to checking if it's still busy when this parameter
is true.

1-copy rasterizer will use this functionality after too many failed
attempts to check if copy operations have completed. This provides a
reliable method to detect drivers with miss-behaving fence extensions.

BUG=406404
Committed: https://crrev.com/51883faa0a6104c390251f57ebc794f714cf98d0
Cr-Commit-Position: refs/heads/master@{#301926}

[reveman, 2014-10-29 05:45:56]

reveman@chromium.org changed reviewers:
+ piman@chromium.org, vmpstr@chromium.org

[piman, 2014-10-29 17:30:25]

LGTM, but I don't know this code very well -> vmpstr

[vmpstr, 2014-10-29 17:36:25]

lgtm

https://codereview.chromium.org/683263004/diff/20001/cc/resources/one_copy_ra...
File cc/resources/one_copy_raster_worker_pool.cc (right):

https://codereview.chromium.org/683263004/diff/20001/cc/resources/one_copy_ra...
cc/resources/one_copy_raster_worker_pool.cc:92: const int
kFailedAttemptsBeforeWaitIfNeeded = 256;
Is there any way we can eliminate some of these constants going forward? Perhaps
expressing one in terms of the other in some cases? Wdyt?

[reveman, 2014-10-29 18:42:48]

https://codereview.chromium.org/683263004/diff/20001/cc/resources/one_copy_ra...
File cc/resources/one_copy_raster_worker_pool.cc (right):

https://codereview.chromium.org/683263004/diff/20001/cc/resources/one_copy_ra...
cc/resources/one_copy_raster_worker_pool.cc:92: const int
kFailedAttemptsBeforeWaitIfNeeded = 256;
On 2014/10/29 17:36:25, vmpstr wrote:
> Is there any way we can eliminate some of these constants going forward?
Perhaps
> expressing one in terms of the other in some cases? Wdyt?

Not sure. This last constant is just to detect driver bugs really. I'd like to
keep them separate for now in case I've set some of them a bit to aggressively
low so it's easy to adjust one without affecting another until we have a good
idea what good values are.

The important part is that we never expose these as settings or make them
different depending on the platform. That would be a failure.

[vmpstr, 2014-10-29 19:11:42]

On 2014/10/29 18:42:48, reveman wrote:
>
https://codereview.chromium.org/683263004/diff/20001/cc/resources/one_copy_ra...
> File cc/resources/one_copy_raster_worker_pool.cc (right):
> 
>
https://codereview.chromium.org/683263004/diff/20001/cc/resources/one_copy_ra...
> cc/resources/one_copy_raster_worker_pool.cc:92: const int
> kFailedAttemptsBeforeWaitIfNeeded = 256;
> On 2014/10/29 17:36:25, vmpstr wrote:
> > Is there any way we can eliminate some of these constants going forward?
> Perhaps
> > expressing one in terms of the other in some cases? Wdyt?
> 
> Not sure. This last constant is just to detect driver bugs really. I'd like to
> keep them separate for now in case I've set some of them a bit to aggressively
> low so it's easy to adjust one without affecting another until we have a good
> idea what good values are.
> 
> The important part is that we never expose these as settings or make them
> different depending on the platform. That would be a failure.

Understood (from this and offline discussion). Maybe add a comment to
CheckBusyResource to explain what the bool represents?

[reveman, 2014-10-29 19:43:04]

On 2014/10/29 19:11:42, vmpstr wrote:
> On 2014/10/29 18:42:48, reveman wrote:
> >
>
https://codereview.chromium.org/683263004/diff/20001/cc/resources/one_copy_ra...
> > File cc/resources/one_copy_raster_worker_pool.cc (right):
> > 
> >
>
https://codereview.chromium.org/683263004/diff/20001/cc/resources/one_copy_ra...
> > cc/resources/one_copy_raster_worker_pool.cc:92: const int
> > kFailedAttemptsBeforeWaitIfNeeded = 256;
> > On 2014/10/29 17:36:25, vmpstr wrote:
> > > Is there any way we can eliminate some of these constants going forward?
> > Perhaps
> > > expressing one in terms of the other in some cases? Wdyt?
> > 
> > Not sure. This last constant is just to detect driver bugs really. I'd like
to
> > keep them separate for now in case I've set some of them a bit to
aggressively
> > low so it's easy to adjust one without affecting another until we have a
good
> > idea what good values are.
> > 
> > The important part is that we never expose these as settings or make them
> > different depending on the platform. That would be a failure.
> 
> Understood (from this and offline discussion). Maybe add a comment to
> CheckBusyResource to explain what the bool represents?

Done

[reveman, 2014-10-29 19:43:08]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2014-10-29 19:44:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/683263004/40001

[commit-bot: I haz the power, 2014-10-29 19:48:26]

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

[commit-bot: I haz the power, 2014-10-29 19:48:27]

Try jobs failed on following builders:
  linux_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/linux_gpu/builds/...)
  mac_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/75350)
  android_arm64_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_arm64_d...)
  android_chromium_gn_compile_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)
  android_clang_dbg_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_clang_d...)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  linux_chromium_asan_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win_chromium_compile_dbg on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel/...)
  win_chromium_x64_rel on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[reveman, 2014-10-29 20:12:10]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2014-10-29 20:12:30]

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

[commit-bot: I haz the power, 2014-10-29 21:04:48]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2014-10-29 21:05:25]

Patchset 4 (id:??) landed as
https://crrev.com/51883faa0a6104c390251f57ebc794f714cf98d0
Cr-Commit-Position: refs/heads/master@{#301926}