Disable threaded compositing when texture sharing is blacklisted

Disable threaded compositing when texture sharing is blacklisted

When texture_sharing is blacklisted, we can't do readbacks from the
browser process and thus hit the render process to do readbacks for
the thumbnailer or the extensions getVisibleTab() API. This path is
a bit buggy in threaded compositing mode, so disable the thread.

BUG=158747
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=165866

[jamesr, 2012-11-02 23:56:58]

@vangelis - please review

@ben - could you OWNERS approve the changes to chrome/test/gpu/ and
content/browser/gpu? Thanks!

[Ben Goodger (Google), 2012-11-03 00:22:51]

owner lgtm once you have lgtm


On Fri, Nov 2, 2012 at 4:56 PM, <jamesr@chromium.org> wrote:

> Reviewers: vangelis, Ben Goodger (Google),
>
> Message:
> @vangelis - please review
>
> @ben - could you OWNERS approve the changes to chrome/test/gpu/ and
> content/browser/gpu? Thanks!
>
> Description:
> Disable threaded compositing when texture sharing is blacklisted
>
> When texture_sharing is blacklisted, we can't do readbacks from the
> browser process and thus hit the render process to do readbacks for
> the thumbnailer or the extensions getVisibleTab() API. This path is
> a bit buggy in threaded compositing mode, so disable the thread.
>
> BUG=158747
>
>
> Please review this at
https://codereview.chromium.**org/11361077/<https://codereview.chromium.org/1...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
>
> Affected files:
>   M chrome/test/gpu/gpu_feature_**browsertest.cc
>   M content/browser/gpu/**compositor_util.cc
>   M webkit/compositor_bindings/**web_compositor_support_impl.cc
>
>
> Index: chrome/test/gpu/gpu_feature_**browsertest.cc
> diff --git a/chrome/test/gpu/gpu_feature_**browsertest.cc
> b/chrome/test/gpu/gpu_feature_**browsertest.cc
> index 892facb246eb578ff09b065fa85694**4665b1f894..**
> 91cd6816ca74dac654075a1241b290**8836cbd1ad 100644
> --- a/chrome/test/gpu/gpu_feature_**browsertest.cc
> +++ b/chrome/test/gpu/gpu_feature_**browsertest.cc
> @@ -44,6 +44,7 @@ namespace {
>  const char kSwapBuffersEvent[] = "SwapBuffers";
>  const char kAcceleratedCanvasCreationEven**t[] =
> "Canvas2DLayerBridgeCreation";
>  const char kWebGLCreationEvent[] = "DrawingBufferCreation";
> +const char kThreadedCompositingEvent[] = "**
> ThreadedCompositingInitializat**ion";
>
>  class GpuFeatureTest : public InProcessBrowserTest {
>   public:
> @@ -226,6 +227,30 @@ IN_PROC_BROWSER_TEST_F(**
> AcceleratedCompositingBlockedT**est,
>    RunEventTest(url, kSwapBuffersEvent, false);
>  }
>
> +IN_PROC_BROWSER_TEST_F(**GpuFeatureTest,
ThreadedCompositingTextureShar**ing)
> {
> +  const std::string json_blacklist =
> +      "{\n"
> +      "  \"name\": \"gpu blacklist\",\n"
> +      "  \"version\": \"1.0\",\n"
> +      "  \"entries\": [\n"
> +      "    {\n"
> +      "      \"id\": 1,\n"
> +      "      \"blacklist\": [\n"
> +      "        \"texture_sharing\"\n"
> +      "      ]\n"
> +      "    }\n"
> +      "  ]\n"
> +      "}";
> +  SetupBlacklist(json_blacklist)**;
> +  GpuFeatureType type =
> +      GpuDataManager::GetInstance()-**>GetBlacklistedFeatures();
> +  type = IgnoreGpuFeatures(type);
> +  EXPECT_EQ(type, content::GPU_FEATURE_TYPE_**TEXTURE_SHARING);
> +
> +  const FilePath url(FILE_PATH_LITERAL("**feature_compositing.html"));
> +  RunEventTest(url, kThreadedCompositingEvent, false);
> +}
> +
>  class AcceleratedCompositingTest : public GpuFeatureTest {
>   public:
>    virtual void SetUpCommandLine(CommandLine* command_line) {
> Index: content/browser/gpu/**compositor_util.cc
> diff --git a/content/browser/gpu/**compositor_util.cc
> b/content/browser/gpu/**compositor_util.cc
> index d95459071877d1f8dffa3257239c22**629e145f81..**
> bef0a97fcc9ce72a32bb11b975e820**4198486a73 100644
> --- a/content/browser/gpu/**compositor_util.cc
> +++ b/content/browser/gpu/**compositor_util.cc
> @@ -47,6 +47,15 @@ bool IsThreadedCompositingEnabled() {
>    if (!CanDoAcceleratedCompositing(**))
>      return false;
>
> +  const GpuDataManager* gpu_data_manager = GpuDataManager::GetInstance();
> +  GpuFeatureType blacklisted_features =
> +      gpu_data_manager->**GetBlacklistedFeatures();
> +  // Disallow threaded compositing when texture sharing is blacklisted
> since
> +  // this triggers renderer-side readbacks for the thumbnailer /
> extensions.
> +  // http://crbug.com/158747
> +  if (blacklisted_features & GPU_FEATURE_TYPE_TEXTURE_**SHARING)
> +    return false;
> +
>    const CommandLine& command_line = *CommandLine::**ForCurrentProcess();
>
>    // Command line switches take precedence over field trials.
> Index: webkit/compositor_bindings/**web_compositor_support_impl.cc
> diff --git a/webkit/compositor_bindings/**web_compositor_support_impl.cc
> b/webkit/compositor_bindings/**web_compositor_support_impl.cc
> index 32a18408180f2ff163223c1aac85fc**c6eec1549f..**
> b05fbb3b4ec91d789ea8505e016d8f**e3f8d5a9ed 100644
> --- a/webkit/compositor_bindings/**web_compositor_support_impl.cc
> +++ b/webkit/compositor_bindings/**web_compositor_support_impl.cc
> @@ -5,6 +5,7 @@
>  #include "config.h"
>  #include "webkit/compositor_bindings/**web_compositor_support_impl.h"
>
> +#include "base/debug/tracing.h"
>  #include "base/memory/scoped_ptr.h"
>  #include "cc/settings.h"
>  #include "webkit/compositor_bindings/**web_animation_impl.h"
> @@ -56,6 +57,9 @@ WebCompositorSupportImpl::~**WebCompositorSupportImpl()
> {
>  }
>
>  void WebCompositorSupportImpl::**initialize(WebKit::WebThread* thread) {
> +  if (thread) {
> +    TRACE_EVENT_INSTANT0("test_**gpu", "**ThreadedCompositingInitializat*
> *ion");
> +  }
>    WebCompositorImpl::initialize(**thread);
>  }
>
>
>
>

[vangelis, 2012-11-03 00:52:02]

On 2012/11/02 23:56:58, jamesr wrote:
> @vangelis - please review
> 
> @ben - could you OWNERS approve the changes to chrome/test/gpu/ and
> content/browser/gpu? Thanks!

lgtm

[commit-bot: I haz the power, 2012-11-03 00:52:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/11361077/1

[commit-bot: I haz the power, 2012-11-03 01:08:23]

Sorry for I got bad news for ya.
Compile failed with a clobber build on android_dbg.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_db...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2012-11-03 20:19:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamesr@chromium.org/11361077/2003

[commit-bot: I haz the power, 2012-11-04 00:03:57]

Change committed as 165866