Implement the Platform::sharedOffscreenGraphicsContext3D method.

Implement the Platform::sharedOffscreenGraphicsContext3D method.

This method is being added to the Platform API for WebKit, and
when it is added, DumpRenderTree will use it for its shared main thread context.

R=jamesr,piman
BUG=177768
TBR=darin

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=185357

[danakj, 2013-02-09 06:00:19]

This is a followup to https://codereview.chromium.org/12212007/ that will allow
the WebKit::Platform API change cleanly.

[jamesr, 2013-02-09 06:01:34]

lgtm

[piman, 2013-02-11 18:19:46]

https://codereview.chromium.org/12217099/diff/1/webkit/support/test_webkit_pl...
File webkit/support/test_webkit_platform_support.cc (right):

https://codereview.chromium.org/12217099/diff/1/webkit/support/test_webkit_pl...
webkit/support/test_webkit_platform_support.cc:444: static
WebKit::WebGraphicsContext3D* context = NULL;
/Cry. Can't we make this a member?

[danakj, 2013-02-11 18:22:06]

https://codereview.chromium.org/12217099/diff/1/webkit/support/test_webkit_pl...
File webkit/support/test_webkit_platform_support.cc (right):

https://codereview.chromium.org/12217099/diff/1/webkit/support/test_webkit_pl...
webkit/support/test_webkit_platform_support.cc:444: static
WebKit::WebGraphicsContext3D* context = NULL;
On 2013/02/11 18:19:46, piman wrote:
> /Cry. Can't we make this a member?

Uhm, yah I guess so!

[danakj, 2013-02-11 18:33:56]

https://codereview.chromium.org/12217099/diff/1/webkit/support/test_webkit_pl...
File webkit/support/test_webkit_platform_support.cc (right):

https://codereview.chromium.org/12217099/diff/1/webkit/support/test_webkit_pl...
webkit/support/test_webkit_platform_support.cc:444: static
WebKit::WebGraphicsContext3D* context = NULL;
On 2013/02/11 18:22:06, danakj wrote:
> On 2013/02/11 18:19:46, piman wrote:
> > /Cry. Can't we make this a member?
> 
> Uhm, yah I guess so!

Done.

[piman, 2013-02-11 18:46:06]

lgtm

[danakj, 2013-02-13 06:20:01]

I've added the methods to get the shared GrContext* from the embedder.

[danakj, 2013-02-13 06:20:02]

I've added the methods to get the shared GrContext* from the embedder.

[jamesr, 2013-02-13 06:30:19]

still lgtm

[danakj, 2013-02-25 21:04:19]

I've updated this patch to not require WebSharedGraphicsContext3D for the
WLTVImplForTesting. Now both WLTVIForTesting and TestWKPlatformSupport go to the
same source to get an inprocess WGC3D - TestContextProviderFactory.

The factory produces context providers for them both. The WLTVIForTesting just
passes them on. The TestWKPlatformSupport unwraps the context pointers to give
them to webkit.

[danakj, 2013-02-26 16:27:03]

Dropped the was_created here also.

[danakj, 2013-02-26 17:49:11]

Jamesr: Could you verify that you're okay with the
webkit/gpu/test_context_provider_factory.h idea here and how I've hooked it up
to both WLTVIForTesting and TestWKPlatformSupport? Thanks :)

[jamesr, 2013-02-26 19:05:58]

On 2013/02/26 17:49:11, danakj wrote:
> Jamesr: Could you verify that you're okay with the
> webkit/gpu/test_context_provider_factory.h idea here and how I've hooked it up
> to both WLTVIForTesting and TestWKPlatformSupport? Thanks :)

Yeah, seems ok. LGTM

[danakj, 2013-02-26 19:08:02]

Hooray, thank you

[commit-bot: I haz the power, 2013-02-26 19:09:24]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/12217099/18011

[commit-bot: I haz the power, 2013-02-26 19:09:27]

Failed to apply patch for
webkit/compositor_bindings/web_layer_tree_view_impl_for_testing.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file
webkit/compositor_bindings/web_layer_tree_view_impl_for_testing.cc
  Hunk #1 FAILED at 23.
  Hunk #2 FAILED at 225.
  2 out of 2 hunks FAILED -- saving rejects to file
webkit/compositor_bindings/web_layer_tree_view_impl_for_testing.cc.rej

Patch:       webkit/compositor_bindings/web_layer_tree_view_impl_for_testing.cc
Index: webkit/compositor_bindings/web_layer_tree_view_impl_for_testing.cc
diff --git a/webkit/compositor_bindings/web_layer_tree_view_impl_for_testing.cc
b/webkit/compositor_bindings/web_layer_tree_view_impl_for_testing.cc
index
5dd58feeee7903744b080ae17009f12b2e43cd2e..57fe98ac50c064b108ee6a180f938c755b09ec5b
100644
--- a/webkit/compositor_bindings/web_layer_tree_view_impl_for_testing.cc
+++ b/webkit/compositor_bindings/web_layer_tree_view_impl_for_testing.cc
@@ -23,23 +23,27 @@
 #include
"third_party/WebKit/Source/Platform/chromium/public/WebLayerTreeView.h"
 #include
"third_party/WebKit/Source/Platform/chromium/public/WebLayerTreeViewClient.h"
 #include
"third_party/WebKit/Source/Platform/chromium/public/WebRenderingStats.h"
-#include
"third_party/WebKit/Source/Platform/chromium/public/WebSharedGraphicsContext3D.h"
 #include "third_party/WebKit/Source/Platform/chromium/public/WebSize.h"
 #include "webkit/compositor_bindings/web_compositor_support_impl.h"
 #include
"webkit/compositor_bindings/web_compositor_support_software_output_device.h"
 #include "webkit/compositor_bindings/web_layer_impl.h"
 #include "webkit/compositor_bindings/web_rendering_stats_impl.h"
 #include "webkit/compositor_bindings/web_to_ccinput_handler_adapter.h"
-#include "webkit/gpu/webgraphicscontext3d_in_process_command_buffer_impl.h"
+#include "webkit/gpu/test_context_provider_factory.h"
+#include "webkit/support/test_webkit_platform_support.h"
 
 namespace WebKit {
 
 WebLayerTreeViewImplForTesting::WebLayerTreeViewImplForTesting(
     RenderingType type, WebKit::WebLayerTreeViewClient* client)
     : type_(type),
-      client_(client) {}
+      client_(client),
+      context_provider_factory_(NULL) {}
 
-WebLayerTreeViewImplForTesting::~WebLayerTreeViewImplForTesting() {}
+WebLayerTreeViewImplForTesting::~WebLayerTreeViewImplForTesting() {
+  if (context_provider_factory_)
+    webkit::gpu::TestContextProviderFactory::DestroyInstance();
+}
 
 bool WebLayerTreeViewImplForTesting::initialize(
       scoped_ptr<cc::Thread> compositor_thread) {
@@ -225,77 +229,23 @@ void WebLayerTreeViewImplForTesting::scheduleComposite() {
     client_->scheduleComposite();
 }
 
-class WebLayerTreeViewImplForTesting::MainThreadContextProvider
-    : public cc::ContextProvider {
- public:
-  virtual bool InitializeOnMainThread() OVERRIDE { return true; }
-  virtual bool BindToCurrentThread() OVERRIDE { return true; }
-
-  virtual WebKit::WebGraphicsContext3D* Context3d() OVERRIDE {
-    return WebSharedGraphicsContext3D::mainThreadContext();
-  }
-  virtual class GrContext* GrContext() OVERRIDE {
-    return WebSharedGraphicsContext3D::mainThreadGrContext();
-  }
-
-  virtual void VerifyContexts() OVERRIDE {}
-
- protected:
-  virtual ~MainThreadContextProvider() {}
-};
-
 scoped_refptr<cc::ContextProvider>
 WebLayerTreeViewImplForTesting::OffscreenContextProviderForMainThread() {
-  if (!contexts_main_thread_)
-    contexts_main_thread_ = new MainThreadContextProvider;
-  return contexts_main_thread_;
-}
-
-class WebLayerTreeViewImplForTesting::CompositorThreadContextProvider
-    : public cc::ContextProvider {
- public:
-  CompositorThreadContextProvider() : destroyed_(false) {}
-
-  virtual bool InitializeOnMainThread() OVERRIDE {
-    return WebSharedGraphicsContext3D::createCompositorThreadContext();
-  }
-  virtual bool BindToCurrentThread() OVERRIDE {
-    return Context3d()->makeContextCurrent();
+  if (!context_provider_factory_) {
+    context_provider_factory_ =
+        webkit::gpu::TestContextProviderFactory::GetInstance();
   }
-
-  virtual WebKit::WebGraphicsContext3D* Context3d() OVERRIDE {
-    return WebSharedGraphicsContext3D::compositorThreadContext();
-  }
-  virtual class GrContext* GrContext() OVERRIDE {
-    return WebSharedGraphicsContext3D::compositorThreadGrContext();
-  }
-
-  virtual void VerifyContexts() OVERRIDE {
-    if (Context3d() && !Context3d()->isContextLost())
-      return;
-    base::AutoLock lock(destroyed_lock_);
-    destroyed_ = true;
-  }
-
-  bool DestroyedOnMainThread() {
-    base::AutoLock lock(destroyed_lock_);
-    return destroyed_;
-  }
-
- protected:
-  virtual ~CompositorThreadContextProvider() {}
-
- private:
-  base::Lock destroyed_lock_;
-  bool destroyed_;
-};
+  return context_provider_factory_->OffscreenContextProviderForMainThread();
+}
 
 scoped_refptr<cc::ContextProvider>
 WebLayerTreeViewImplForTesting::OffscreenContextProviderForCompositorThread() {
-  if (!contexts_compositor_thread_ ||
-      contexts_compositor_thread_->DestroyedOnMainThread())
-    contexts_compositor_thread_ = new CompositorThreadContextProvider;
-  return contexts_compositor_thread_;
+  if (!context_provider_factory_) {
+    context_provider_factory_ =
+        webkit::gpu::TestContextProviderFactory::GetInstance();
+  }
+  return
+      context_provider_factory_->OffscreenContextProviderForCompositorThread();
 }
 
 }  // namespace WebKit

[jamesr, 2013-02-26 19:18:11]

https://codereview.chromium.org/12217099/diff/18011/webkit/gpu/context_provid...
File webkit/gpu/context_provider_in_process.cc (right):

https://codereview.chromium.org/12217099/diff/18011/webkit/gpu/context_provid...
webkit/gpu/context_provider_in_process.cc:34:
MemoryAllocationCallbackProxy(ContextProviderInProcess* provider)
explicit

[danakj, 2013-02-26 19:23:02]

https://codereview.chromium.org/12217099/diff/18011/webkit/gpu/context_provid...
File webkit/gpu/context_provider_in_process.cc (right):

https://codereview.chromium.org/12217099/diff/18011/webkit/gpu/context_provid...
webkit/gpu/context_provider_in_process.cc:34:
MemoryAllocationCallbackProxy(ContextProviderInProcess* provider)
On 2013/02/26 19:18:11, jamesr wrote:
> explicit

Done.

[commit-bot: I haz the power, 2013-02-26 19:23:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/12217099/14014

[danakj, 2013-02-26 19:35:18]

I guess this needs the webkit roll for the GrContext forward declaration in
Platform.h. I'll wait for that.

[commit-bot: I haz the power, 2013-02-27 02:10:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/12217099/14014

[commit-bot: I haz the power, 2013-02-27 02:27:09]

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.

[danakj, 2013-02-28 05:28:10]

I solved the mac crisis by moving webkit_gpu into its own .gyp file. The cycle
was caused by other things in webkit_support.gyp eventually getting back to
compositor bindings.

Please tell me if this is horribly wrong or wonderfully exciting. :)

All places that depended on webkit_support.gyp:webkit_gpu now depend on
webkit_gpu.gyp:webkit_gpu. The ones that depend on compositor bindings, I just
removed the webkit_gpu dependency since its implied by the bindings now.

[piman, 2013-02-28 05:48:41]

On 2013/02/28 05:28:10, danakj wrote:
> I solved the mac crisis by moving webkit_gpu into its own .gyp file. The cycle
> was caused by other things in webkit_support.gyp eventually getting back to
> compositor bindings.
> 
> Please tell me if this is horribly wrong or wonderfully exciting. :)

It looks very reasonable to me, but I may be missing something.

> 
> All places that depended on webkit_support.gyp:webkit_gpu now depend on
> webkit_gpu.gyp:webkit_gpu. The ones that depend on compositor bindings, I just
> removed the webkit_gpu dependency since its implied by the bindings now.

In general, we try to be explicit, like IncludeWhatYouUse, that is if component
A depends on component B, and component B depends on component C, but A also
uses classes from C, to not rely on the implicit dependency added, and to have A
directly depend on C. This matters for things like being able to leave flags
into direct_dependent_settings instead of export_dependent_settings that end up
bloating command lines for top-level components (among other issues). I'll let
you judge what's the right thing.

[jamesr, 2013-02-28 19:30:09]

The gyp stuff lgtm but I don't understand what you are trying to achieve with
the lifetime stuff.  In prod, the context provider is essentially just a
singleton.  Why not do that here as well?

[danakj, 2013-02-28 19:31:33]

On 2013/02/28 19:30:09, jamesr wrote:
> The gyp stuff lgtm but I don't understand what you are trying to achieve with
> the lifetime stuff.  In prod, the context provider is essentially just a
> singleton.  Why not do that here as well?

That's what I was trying to do here too. The TestContextProviderFactory is a
singleton (static pointer). It providers the context provider to both WLTVImpl
and TestWKPlatformSupport so that the same context is used by the compositor and
by webkit - the same way as in prod.

[jamesr, 2013-02-28 20:04:49]

On 2013/02/28 19:31:33, danakj wrote:
> On 2013/02/28 19:30:09, jamesr wrote:
> > The gyp stuff lgtm but I don't understand what you are trying to achieve
with
> > the lifetime stuff.  In prod, the context provider is essentially just a
> > singleton.  Why not do that here as well?
> 
> That's what I was trying to do here too. The TestContextProviderFactory is a
> singleton (static pointer). It providers the context provider to both WLTVImpl
> and TestWKPlatformSupport so that the same context is used by the compositor
and
> by webkit - the same way as in prod.

OK, then why the refcounting and ::Destroy() call?

[danakj, 2013-02-28 20:06:58]

On 2013/02/28 20:04:49, jamesr wrote:
> On 2013/02/28 19:31:33, danakj wrote:
> > On 2013/02/28 19:30:09, jamesr wrote:
> > > The gyp stuff lgtm but I don't understand what you are trying to achieve
> with
> > > the lifetime stuff.  In prod, the context provider is essentially just a
> > > singleton.  Why not do that here as well?
> > 
> > That's what I was trying to do here too. The TestContextProviderFactory is a
> > singleton (static pointer). It providers the context provider to both
WLTVImpl
> > and TestWKPlatformSupport so that the same context is used by the compositor
> and
> > by webkit - the same way as in prod.
> 
> OK, then why the refcounting and ::Destroy() call?

Oh, because both WLTVIForTesting and TestWKPlatformSupport need to have a
pointer to the TestContextProviderFactory. I wanted it to be created only once,
but to be destroyed properly during shutdown. So I did the refcounting so that
the ContextProviderFactory (and thus also the ContextProvider) can be cleaned up
properly during shutdown.

Would you prefer to just init don't cleanup on shutdown?

[jamesr, 2013-02-28 20:08:10]

> Would you prefer to just init don't cleanup on shutdown?

Yes

[danakj, 2013-02-28 20:11:50]

On 2013/02/28 20:08:10, jamesr wrote:
> > Would you prefer to just init don't cleanup on shutdown?
> 
> Yes

k, done!

[danakj, 2013-02-28 20:16:02]

On Thu, Feb 28, 2013 at 3:11 PM, <danakj@chromium.org> wrote:

> On 2013/02/28 20:08:10, jamesr wrote:
>
>> > Would you prefer to just init don't cleanup on shutdown?
>>
>
>  Yes
>>
>
> k, done!
>

Also removed all the TestContextProviderFactory member variables, and just
access it through the GetInstance() method directly all the time.


>
>
https://chromiumcodereview.**appspot.com/12217099/<https://chromiumcodereview...
>

[jamesr, 2013-02-28 20:20:54]

lgtm2 !

[danakj, 2013-02-28 20:22:33]

+darin

TBRing darin for the chrome/chrome_renderer.gypi change to use webkit_gpu.gyp
insead of webkit_support.gyp:webkit_gpu.

[commit-bot: I haz the power, 2013-02-28 20:28:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/danakj@chromium.org/12217099/41070

[danakj, 2013-02-28 23:08:24]

Committed patchset #15 manually as r185357 (presubmit successful).