Revert of Make lost context and error message callbacks on GpuControl go to client

Revert of Make lost context and error message callbacks on GpuControl go to client (patchset #18 id:360001 of https://codereview.chromium.org/1864723003/ )

Reason for revert:
This possibly causes page_cycler.intl_hi_ru and page_cycler.intl_es_fr_pt-BR to fail.
crbug.com/602801

Original issue's description:
> Make lost context and error message callbacks on GpuControl go to client
> 
> In this we introduce a GpuControlClient, that implementations of
> GpuControl will call to notify about the context becoming lost, or of
> error messages.
> 
> Users of GpuControl implement the client to hear about these states.
> The most important users of GpuControl here are:
> - GLES2Implementation, which now has 2 base::Callbacks which it forwards
>   these events to, if they have been set by the user of the
>   GLES2Implementation.
> - PPB_Graphics3D_Impl, which acts on these two events.
> - PepperVideoEncoderHost, which turns lost context events into a call
>   to NotifyPepperError(PP_ERROR_RESOURCE_FAILED).
> - mojo::GLES2Context, which forwards the lost context event to a
>   registered callback.
> 
> Other owners of GpuControl (if they exist) never set callbacks on
> CommandBufferProxyImpl or equivalent before, so they don't care to
> listen to these events and just opt out of setting the client
> altogether.
> 
> The main implementations of GpuControl here are:
> - CommandBufferProxyImpl, which will now call the client instead of
>   owning base::Callbacks for these two events.
> - mojo::CommandBufferClientImpl, which notifies its client of lost
>   contexts, but has no concept of error messages.
> - InProcessCommandBuffer, which will now call the client (on the client
>   thread) instead of owning and wrapping base::Callbacks for the lost
>   context event. It also has no error messages.
> 
> For other implementations of GpuControl there is no chance to lose the
> context or generate error messages, and they don't even bother to store
> the client since it would go unused.
> 
> WebGraphicsContext3D{CommandBuffer}Impl continues to be used for these
> callbacks, but it now goes directly to the GLES2Implementation to set
> callbacks, instead of bypassing it to the CommandBufferProxyImpl.
> 
> The next step will be to have Blink or the ContextProvider directly set
> these callbacks on the GLES2Implementation, bypassing the
> WebGraphicsContext3DImpls entirely. This isn't possible today as the
> two versions of WebGraphicsContext3DImpl (which are
> WebGraphicsContext3DCommandBufferImpl and
> WebGraphicsContext3DInProcessCommandBufferImpl) each have different
> versions of the GpuControl, while both having a GLES2Implementation,
> forcing ContextProviderCommandBuffer to route through this fork
> when setting the callbacks.
> 
> R=piman@chomium.org
> BUG=584497
> CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
> 
> Committed: https://crrev.com/10057772128ab5e67452dc736bf15752dc434ed3
> Cr-Commit-Position: refs/heads/master@{#386761}

TBR=piman@chromium.org,sievers@chromium.org,boliu@chromium.org,fsamuel@chromium.org,sky@chromium.org,wez@chromium.org,danakj@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=584497

[rnephew (Reviews Here), 2016-04-13 16:49:45]

Created Revert of Make lost context and error message callbacks on GpuControl go
to client

[commit-bot: I haz the power, 2016-04-13 16:49:59]

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

[commit-bot: I haz the power, 2016-04-13 16:50:37]

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

[commit-bot: I haz the power, 2016-04-13 16:50:40]

Failed to apply patch for
content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc:
While running git apply --index -3 -p1;
  error: patch failed:
content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc:120
  error: repository lacks the necessary blob to fall back on 3-way merge.
  error: content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc:
patch does not apply

Patch:      
content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc
Index: content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc
diff --git
a/content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc
b/content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc
index
29dc0e2384d4e29b776072eac60e2d8bfc7b208d..708a3da8ee0a8b2c93894de7eef23006bd2fa062
100644
--- a/content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc
+++ b/content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc
@@ -120,9 +120,7 @@
 WebGraphicsContext3DCommandBufferImpl::
     ~WebGraphicsContext3DCommandBufferImpl() {
   if (real_gl_) {
-    real_gl_->SetErrorMessageCallback(
-        base::Callback<void(const char*, int32_t)>());
-    real_gl_->SetLostContextCallback(base::Closure());
+    real_gl_->SetErrorMessageCallback(NULL);
   }
 
   Destroy();
@@ -149,15 +147,15 @@
     return false;
   }
 
-  real_gl_->SetErrorMessageCallback(
+  command_buffer_->SetContextLostCallback(
+      base::Bind(&WebGraphicsContext3DCommandBufferImpl::OnContextLost,
+                 weak_ptr_factory_.GetWeakPtr()));
+
+  command_buffer_->SetOnConsoleMessageCallback(
       base::Bind(&WebGraphicsContext3DCommandBufferImpl::OnErrorMessage,
-                 // The callback is unset in the destructor.
-                 base::Unretained(this)));
-  real_gl_->SetLostContextCallback(
-      base::Bind(&WebGraphicsContext3DCommandBufferImpl::OnContextLost,
-                 // The callback is unset in the destructor.
-                 base::Unretained(this)));
-
+                 weak_ptr_factory_.GetWeakPtr()));
+
+  real_gl_->SetErrorMessageCallback(getErrorMessageCallback());
   real_gl_->TraceBeginCHROMIUM("WebGraphicsContext3D",
                                "CommandBufferContext");
 
@@ -364,12 +362,4 @@
   UmaRecordContextLost(context_type_, state.error, state.context_lost_reason);
 }
 
-void WebGraphicsContext3DCommandBufferImpl::OnErrorMessage(const char* message,
-                                                           int id) {
-  if (error_message_callback_) {
-    blink::WebString str = blink::WebString::fromUTF8(message);
-    error_message_callback_->onErrorMessage(str, id);
-  }
-}
-
 }  // namespace content

[danakj, 2016-04-13 18:15:05]

This doesn't apply any more, but the fix is very small:
https://codereview.chromium.org/1881423004/

[Wez, 2016-04-14 16:47:32]

Description was changed from

==========
Revert of Make lost context and error message callbacks on GpuControl go to
client (patchset #18 id:360001 of https://codereview.chromium.org/1864723003/ )

Reason for revert:
This possibly causes page_cycler.intl_hi_ru and page_cycler.intl_es_fr_pt-BR to
fail.
crbug.com/602801

Original issue's description:
> Make lost context and error message callbacks on GpuControl go to client
> 
> In this we introduce a GpuControlClient, that implementations of
> GpuControl will call to notify about the context becoming lost, or of
> error messages.
> 
> Users of GpuControl implement the client to hear about these states.
> The most important users of GpuControl here are:
> - GLES2Implementation, which now has 2 base::Callbacks which it forwards
>   these events to, if they have been set by the user of the
>   GLES2Implementation.
> - PPB_Graphics3D_Impl, which acts on these two events.
> - PepperVideoEncoderHost, which turns lost context events into a call
>   to NotifyPepperError(PP_ERROR_RESOURCE_FAILED).
> - mojo::GLES2Context, which forwards the lost context event to a
>   registered callback.
> 
> Other owners of GpuControl (if they exist) never set callbacks on
> CommandBufferProxyImpl or equivalent before, so they don't care to
> listen to these events and just opt out of setting the client
> altogether.
> 
> The main implementations of GpuControl here are:
> - CommandBufferProxyImpl, which will now call the client instead of
>   owning base::Callbacks for these two events.
> - mojo::CommandBufferClientImpl, which notifies its client of lost
>   contexts, but has no concept of error messages.
> - InProcessCommandBuffer, which will now call the client (on the client
>   thread) instead of owning and wrapping base::Callbacks for the lost
>   context event. It also has no error messages.
> 
> For other implementations of GpuControl there is no chance to lose the
> context or generate error messages, and they don't even bother to store
> the client since it would go unused.
> 
> WebGraphicsContext3D{CommandBuffer}Impl continues to be used for these
> callbacks, but it now goes directly to the GLES2Implementation to set
> callbacks, instead of bypassing it to the CommandBufferProxyImpl.
> 
> The next step will be to have Blink or the ContextProvider directly set
> these callbacks on the GLES2Implementation, bypassing the
> WebGraphicsContext3DImpls entirely. This isn't possible today as the
> two versions of WebGraphicsContext3DImpl (which are
> WebGraphicsContext3DCommandBufferImpl and
> WebGraphicsContext3DInProcessCommandBufferImpl) each have different
> versions of the GpuControl, while both having a GLES2Implementation,
> forcing ContextProviderCommandBuffer to route through this fork
> when setting the callbacks.
> 
> R=piman@chomium.org
> BUG=584497
>
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
> 
> Committed: https://crrev.com/10057772128ab5e67452dc736bf15752dc434ed3
> Cr-Commit-Position: refs/heads/master@{#386761}

TBR=piman@chromium.org,sievers@chromium.org,boliu@chromium.org,fsamuel@chromi...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=584497
==========

to

==========
Revert of Make lost context and error message callbacks on GpuControl go to
client (patchset #18 id:360001 of https://codereview.chromium.org/1864723003/ )

Reason for revert:
This possibly causes page_cycler.intl_hi_ru and page_cycler.intl_es_fr_pt-BR to
fail.
crbug.com/602801

Original issue's description:
> Make lost context and error message callbacks on GpuControl go to client
> 
> In this we introduce a GpuControlClient, that implementations of
> GpuControl will call to notify about the context becoming lost, or of
> error messages.
> 
> Users of GpuControl implement the client to hear about these states.
> The most important users of GpuControl here are:
> - GLES2Implementation, which now has 2 base::Callbacks which it forwards
>   these events to, if they have been set by the user of the
>   GLES2Implementation.
> - PPB_Graphics3D_Impl, which acts on these two events.
> - PepperVideoEncoderHost, which turns lost context events into a call
>   to NotifyPepperError(PP_ERROR_RESOURCE_FAILED).
> - mojo::GLES2Context, which forwards the lost context event to a
>   registered callback.
> 
> Other owners of GpuControl (if they exist) never set callbacks on
> CommandBufferProxyImpl or equivalent before, so they don't care to
> listen to these events and just opt out of setting the client
> altogether.
> 
> The main implementations of GpuControl here are:
> - CommandBufferProxyImpl, which will now call the client instead of
>   owning base::Callbacks for these two events.
> - mojo::CommandBufferClientImpl, which notifies its client of lost
>   contexts, but has no concept of error messages.
> - InProcessCommandBuffer, which will now call the client (on the client
>   thread) instead of owning and wrapping base::Callbacks for the lost
>   context event. It also has no error messages.
> 
> For other implementations of GpuControl there is no chance to lose the
> context or generate error messages, and they don't even bother to store
> the client since it would go unused.
> 
> WebGraphicsContext3D{CommandBuffer}Impl continues to be used for these
> callbacks, but it now goes directly to the GLES2Implementation to set
> callbacks, instead of bypassing it to the CommandBufferProxyImpl.
> 
> The next step will be to have Blink or the ContextProvider directly set
> these callbacks on the GLES2Implementation, bypassing the
> WebGraphicsContext3DImpls entirely. This isn't possible today as the
> two versions of WebGraphicsContext3DImpl (which are
> WebGraphicsContext3DCommandBufferImpl and
> WebGraphicsContext3DInProcessCommandBufferImpl) each have different
> versions of the GpuControl, while both having a GLES2Implementation,
> forcing ContextProviderCommandBuffer to route through this fork
> when setting the callbacks.
> 
> R=piman@chomium.org
> BUG=584497
>
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel
> 
> Committed: https://crrev.com/10057772128ab5e67452dc736bf15752dc434ed3
> Cr-Commit-Position: refs/heads/master@{#386761}

TBR=piman@chromium.org,sievers@chromium.org,boliu@chromium.org,fsamuel@chromi...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=584497
==========

[Wez, 2016-04-14 16:47:51]

Closed in favour of the small fix.