services/ui: Remove deprecated gpu code

services/ui: Remove deprecated gpu code

Summary of changes:
. Move RasterThreadHelper into the client lib, since it's used by mus
  clients (views).
. Remove the --use-mojo-gpu-command-buffer-in-mus flag, which means
  chrome gpu command buffer is now used on Windows for mus.
. Delete the rest of the code in services/ui/gles2, and
  OzoneGpuMemoryBuffer from services/ui/surfaces.

BUG=623157, 630893
Committed: https://crrev.com/d4628518ceaafa1f53bc0903dcf5fff3c61635ac
Cr-Commit-Position: refs/heads/master@{#408207}

[sadrul, 2016-07-25 23:00:51]

The CQ bit was checked by sadrul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-25 23:02:20]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-25 23:49:08]

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

[commit-bot: I haz the power, 2016-07-25 23:49:09]

Dry run: This issue passed the CQ dry run.

[sadrul, 2016-07-26 01:33:00]

The CQ bit was checked by sadrul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-26 01:33:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sadrul, 2016-07-26 02:09:47]

The CQ bit was checked by sadrul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-26 02:10:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-26 02:14:14]

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

[commit-bot: I haz the power, 2016-07-26 02:14:19]

Dry run: Try jobs failed on following builders:
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)

[sadrul, 2016-07-26 02:30:30]

Description was changed from

==========
Use the chrome gpu command buffer on Windows for mus

BUG=623157

patch from issue 2135713002 at patchset 20001
(http://crrev.com/2135713002#ps20001)
==========

to

==========
services/ui: Remove deprecated gpu code

Summary of changes:
. Move OzoneGpuMemoryBuffer into services/ui/surfaces, where it's used.
. Move RasterThreadHelper into the client lib, since it's used by mus
  clients (views).
. Remove the --use-mojo-gpu-command-buffer-in-mus flag, which means
  chrome gpu command buffer is now used on Windows for mus.
. Delete the rest of the code in services/ui/gles2

BUG=623157

patch from issue 2135713002 at patchset 20001
(http://crrev.com/2135713002#ps20001)
==========

[sadrul, 2016-07-26 02:30:41]

Description was changed from

==========
services/ui: Remove deprecated gpu code

Summary of changes:
. Move OzoneGpuMemoryBuffer into services/ui/surfaces, where it's used.
. Move RasterThreadHelper into the client lib, since it's used by mus
  clients (views).
. Remove the --use-mojo-gpu-command-buffer-in-mus flag, which means
  chrome gpu command buffer is now used on Windows for mus.
. Delete the rest of the code in services/ui/gles2

BUG=623157

patch from issue 2135713002 at patchset 20001
(http://crrev.com/2135713002#ps20001)
==========

to

==========
services/ui: Remove deprecated gpu code

Summary of changes:
. Move OzoneGpuMemoryBuffer into services/ui/surfaces, where it's used.
. Move RasterThreadHelper into the client lib, since it's used by mus
  clients (views).
. Remove the --use-mojo-gpu-command-buffer-in-mus flag, which means
  chrome gpu command buffer is now used on Windows for mus.
. Delete the rest of the code in services/ui/gles2

BUG=623157, 630893

patch from issue 2135713002 at patchset 20001
(http://crrev.com/2135713002#ps20001)
==========

[sadrul, 2016-07-26 02:33:55]

The CQ bit was checked by sadrul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-26 02:34:08]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sadrul, 2016-07-26 02:35:11]

Description was changed from

==========
services/ui: Remove deprecated gpu code

Summary of changes:
. Move OzoneGpuMemoryBuffer into services/ui/surfaces, where it's used.
. Move RasterThreadHelper into the client lib, since it's used by mus
  clients (views).
. Remove the --use-mojo-gpu-command-buffer-in-mus flag, which means
  chrome gpu command buffer is now used on Windows for mus.
. Delete the rest of the code in services/ui/gles2

BUG=623157, 630893

patch from issue 2135713002 at patchset 20001
(http://crrev.com/2135713002#ps20001)
==========

to

==========
services/ui: Remove deprecated gpu code

Summary of changes:
. Move OzoneGpuMemoryBuffer into services/ui/surfaces, where it's used.
. Move RasterThreadHelper into the client lib, since it's used by mus
  clients (views).
. Remove the --use-mojo-gpu-command-buffer-in-mus flag, which means
  chrome gpu command buffer is now used on Windows for mus.
. Delete the rest of the code in services/ui/gles2

BUG=623157, 630893
==========

[commit-bot: I haz the power, 2016-07-26 04:29:43]

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

[commit-bot: I haz the power, 2016-07-26 04:29:44]

Dry run: This issue passed the CQ dry run.

[sadrul, 2016-07-26 04:49:58]

sadrul@chromium.org changed reviewers:
+ fsamuel@chromium.org, penghuang@chromium.org

[sadrul, 2016-07-26 04:50:10]

I still need to try this out on a Windows build, but the windows trybots all
look happy.

[Fady Samuel, 2016-07-26 11:50:17]

Yay, thanks for doing this Sadrul! LGTM!

[Peng, 2016-07-26 15:47:04]

https://codereview.chromium.org/2176303002/diff/60001/services/ui/service.cc
File services/ui/service.cc (right):

https://codereview.chromium.org/2176303002/diff/60001/services/ui/service.cc#...
services/ui/service.cc:212: connection->AddInterface<mojom::GpuService>(this);
Reorder interfaces?

https://codereview.chromium.org/2176303002/diff/60001/services/ui/surfaces/di...
File services/ui/surfaces/direct_output_surface_ozone.cc (right):

https://codereview.chromium.org/2176303002/diff/60001/services/ui/surfaces/di...
services/ui/surfaces/direct_output_surface_ozone.cc:38:
ozone_gpu_memory_buffer_manager_.reset(new OzoneGpuMemoryBufferManager());
Seems this is necessary with the mojo command buffer, right?

[sadrul, 2016-07-26 16:13:05]

https://codereview.chromium.org/2176303002/diff/60001/services/ui/service.cc
File services/ui/service.cc (right):

https://codereview.chromium.org/2176303002/diff/60001/services/ui/service.cc#...
services/ui/service.cc:212: connection->AddInterface<mojom::GpuService>(this);
On 2016/07/26 15:47:03, Peng wrote:
> Reorder interfaces?

Done.

https://codereview.chromium.org/2176303002/diff/60001/services/ui/surfaces/di...
File services/ui/surfaces/direct_output_surface_ozone.cc (right):

https://codereview.chromium.org/2176303002/diff/60001/services/ui/surfaces/di...
services/ui/surfaces/direct_output_surface_ozone.cc:38:
ozone_gpu_memory_buffer_manager_.reset(new OzoneGpuMemoryBufferManager());
On 2016/07/26 15:47:03, Peng wrote:
> Seems this is necessary with the mojo command buffer, right?

Oh whoops. You are right. I removed the wrong block of code. Fixed (this allowed
removing the OzoneGpuMemoryBuffer* classes as well). Thanks!

[sadrul, 2016-07-26 16:14:58]

The CQ bit was checked by sadrul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-26 16:15:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[sadrul, 2016-07-26 16:22:57]

Description was changed from

==========
services/ui: Remove deprecated gpu code

Summary of changes:
. Move OzoneGpuMemoryBuffer into services/ui/surfaces, where it's used.
. Move RasterThreadHelper into the client lib, since it's used by mus
  clients (views).
. Remove the --use-mojo-gpu-command-buffer-in-mus flag, which means
  chrome gpu command buffer is now used on Windows for mus.
. Delete the rest of the code in services/ui/gles2

BUG=623157, 630893
==========

to

==========
services/ui: Remove deprecated gpu code

Summary of changes:
. Move RasterThreadHelper into the client lib, since it's used by mus
  clients (views).
. Remove the --use-mojo-gpu-command-buffer-in-mus flag, which means
  chrome gpu command buffer is now used on Windows for mus.
. Delete the rest of the code in services/ui/gles2, and
  OzoneGpuMemoryBuffer from services/ui/surfaces.

BUG=623157, 630893
==========

[commit-bot: I haz the power, 2016-07-26 17:01:36]

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

[commit-bot: I haz the power, 2016-07-26 17:01:37]

Dry run: This issue passed the CQ dry run.

[Peng, 2016-07-26 17:16:59]

On 2016/07/26 16:13:05, sadrul wrote:
> https://codereview.chromium.org/2176303002/diff/60001/services/ui/service.cc
> File services/ui/service.cc (right):
> 
>
https://codereview.chromium.org/2176303002/diff/60001/services/ui/service.cc#...
> services/ui/service.cc:212: connection->AddInterface<mojom::GpuService>(this);
> On 2016/07/26 15:47:03, Peng wrote:
> > Reorder interfaces?
> 
> Done.
> 
>
https://codereview.chromium.org/2176303002/diff/60001/services/ui/surfaces/di...
> File services/ui/surfaces/direct_output_surface_ozone.cc (right):
> 
>
https://codereview.chromium.org/2176303002/diff/60001/services/ui/surfaces/di...
> services/ui/surfaces/direct_output_surface_ozone.cc:38:
> ozone_gpu_memory_buffer_manager_.reset(new OzoneGpuMemoryBufferManager());
> On 2016/07/26 15:47:03, Peng wrote:
> > Seems this is necessary with the mojo command buffer, right?
> 
> Oh whoops. You are right. I removed the wrong block of code. Fixed (this
allowed
> removing the OzoneGpuMemoryBuffer* classes as well). Thanks!

lgtm. Thanks.

[sadrul, 2016-07-26 17:43:41]

sadrul@chromium.org changed reviewers:
+ piman@chromium.org, sky@chromium.org

[sadrul, 2016-07-26 17:43:42]

piman@chromium.org: Please review changes in //content/

sky@chromium.org: Please review changes in //services/ui (other than the gles2)

[piman, 2016-07-26 20:29:04]

LGTM, yay!

[sky, 2016-07-26 21:24:17]

LGTM

[sadrul, 2016-07-27 05:26:30]

The CQ bit was checked by sadrul@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-27 05:26:46]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-27 06:20:15]

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

[commit-bot: I haz the power, 2016-07-27 06:20:17]

Dry run: This issue passed the CQ dry run.

[sadrul, 2016-07-27 18:59:23]

The CQ bit was checked by sadrul@chromium.org

[sadrul, 2016-07-27 18:59:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
fsamuel@chromium.org, penghuang@chromium.org, sky@chromium.org,
piman@chromium.org
Link to the patchset: https://codereview.chromium.org/2176303002/#ps100001
(title: "tot-merge")

[commit-bot: I haz the power, 2016-07-27 18:59:52]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-27 19:05:26]

Description was changed from

==========
services/ui: Remove deprecated gpu code

Summary of changes:
. Move RasterThreadHelper into the client lib, since it's used by mus
  clients (views).
. Remove the --use-mojo-gpu-command-buffer-in-mus flag, which means
  chrome gpu command buffer is now used on Windows for mus.
. Delete the rest of the code in services/ui/gles2, and
  OzoneGpuMemoryBuffer from services/ui/surfaces.

BUG=623157, 630893
==========

to

==========
services/ui: Remove deprecated gpu code

Summary of changes:
. Move RasterThreadHelper into the client lib, since it's used by mus
  clients (views).
. Remove the --use-mojo-gpu-command-buffer-in-mus flag, which means
  chrome gpu command buffer is now used on Windows for mus.
. Delete the rest of the code in services/ui/gles2, and
  OzoneGpuMemoryBuffer from services/ui/surfaces.

BUG=623157, 630893
==========

[commit-bot: I haz the power, 2016-07-27 19:05:27]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-07-27 19:07:18]

Description was changed from

==========
services/ui: Remove deprecated gpu code

Summary of changes:
. Move RasterThreadHelper into the client lib, since it's used by mus
  clients (views).
. Remove the --use-mojo-gpu-command-buffer-in-mus flag, which means
  chrome gpu command buffer is now used on Windows for mus.
. Delete the rest of the code in services/ui/gles2, and
  OzoneGpuMemoryBuffer from services/ui/surfaces.

BUG=623157, 630893
==========

to

==========
services/ui: Remove deprecated gpu code

Summary of changes:
. Move RasterThreadHelper into the client lib, since it's used by mus
  clients (views).
. Remove the --use-mojo-gpu-command-buffer-in-mus flag, which means
  chrome gpu command buffer is now used on Windows for mus.
. Delete the rest of the code in services/ui/gles2, and
  OzoneGpuMemoryBuffer from services/ui/surfaces.

BUG=623157, 630893

Committed: https://crrev.com/d4628518ceaafa1f53bc0903dcf5fff3c61635ac
Cr-Commit-Position: refs/heads/master@{#408207}
==========

[commit-bot: I haz the power, 2016-07-27 19:07:20]

Patchset 6 (id:??) landed as
https://crrev.com/d4628518ceaafa1f53bc0903dcf5fff3c61635ac
Cr-Commit-Position: refs/heads/master@{#408207}

[xing.xu, 2017-01-20 06:49:52]

Sorry, I didn't get why here remove CommandBufferImpl here. Now that many
componenets are moving into mojo, why here reverts gpu commandbuffer's mojo
implementation?
Any hints will be welcome!

[Fady Samuel, 2017-01-22 22:23:45]

On 2017/01/20 06:49:52, xing.xu wrote:
> Sorry, I didn't get why here remove CommandBufferImpl here. Now that many
> componenets are moving into mojo, why here reverts gpu commandbuffer's mojo
> implementation?
> Any hints will be welcome!

We decided that the more prudent course of action is to refactor existing code
as opposed to rewirte the gpu command buffer using mojo. Refactoring existing
production code gives us some measure of confidence that the code is correct and
continues to perform well whereas a rewrite leaves us in the dark until we turn
on the new code path.

[xing.xu, 2017-01-26 02:36:38]

On 2017/01/22 22:23:45, Fady Samuel wrote:
> On 2017/01/20 06:49:52, xing.xu wrote:
> > Sorry, I didn't get why here remove CommandBufferImpl here. Now that many
> > componenets are moving into mojo, why here reverts gpu commandbuffer's mojo
> > implementation?
> > Any hints will be welcome!
> 
> We decided that the more prudent course of action is to refactor existing code
> as opposed to rewirte the gpu command buffer using mojo. Refactoring existing
> production code gives us some measure of confidence that the code is correct
and
> continues to perform well whereas a rewrite leaves us in the dark until we
turn
> on the new code path.


Thanks Samuel.  So, currently when with flag: kUseMusInRenderer,
Render will use mojo to seup a an non-mojo IPC between GPU and render like this:
GpuChannel<=non-mojo IPC=>GpuChannelHost.

Is there any plan to mojotify the IPC between GpuChannel and GpuChannelHost?

[sadrul, 2017-01-26 02:40:43]

On 2017/01/26 02:36:38, xing.xu wrote:
> On 2017/01/22 22:23:45, Fady Samuel wrote:
> > On 2017/01/20 06:49:52, xing.xu wrote:
> > > Sorry, I didn't get why here remove CommandBufferImpl here. Now that many
> > > componenets are moving into mojo, why here reverts gpu commandbuffer's
mojo
> > > implementation?
> > > Any hints will be welcome!
> > 
> > We decided that the more prudent course of action is to refactor existing
code
> > as opposed to rewirte the gpu command buffer using mojo. Refactoring
existing
> > production code gives us some measure of confidence that the code is correct
> and
> > continues to perform well whereas a rewrite leaves us in the dark until we
> turn
> > on the new code path.
> 
> 
> Thanks Samuel.  So, currently when with flag: kUseMusInRenderer,
> Render will use mojo to seup a an non-mojo IPC between GPU and render like
this:
> GpuChannel<=non-mojo IPC=>GpuChannelHost.
> 
> Is there any plan to mojotify the IPC between GpuChannel and GpuChannelHost?

We do want to move to using mojo IPC instead of chrome-IPC for this, yes. But we
are not actively working on this part yet.

[sadrul, 2017-01-26 03:04:27]

On 2017/01/26 02:40:43, sadrul wrote:
> On 2017/01/26 02:36:38, xing.xu wrote:
> > On 2017/01/22 22:23:45, Fady Samuel wrote:
> > > On 2017/01/20 06:49:52, xing.xu wrote:
> > > > Sorry, I didn't get why here remove CommandBufferImpl here. Now that
many
> > > > componenets are moving into mojo, why here reverts gpu commandbuffer's
> mojo
> > > > implementation?
> > > > Any hints will be welcome!
> > > 
> > > We decided that the more prudent course of action is to refactor existing
> code
> > > as opposed to rewirte the gpu command buffer using mojo. Refactoring
> existing
> > > production code gives us some measure of confidence that the code is
correct
> > and
> > > continues to perform well whereas a rewrite leaves us in the dark until we
> > turn
> > > on the new code path.
> > 
> > 
> > Thanks Samuel.  So, currently when with flag: kUseMusInRenderer,
> > Render will use mojo to seup a an non-mojo IPC between GPU and render like
> this:
> > GpuChannel<=non-mojo IPC=>GpuChannelHost.

Note that this happens in regular chrome on platforms too (i.e. not just with
--use-mus-in-renderer)

> > 
> > Is there any plan to mojotify the IPC between GpuChannel and GpuChannelHost?
> 
> We do want to move to using mojo IPC instead of chrome-IPC for this, yes. But
we
> are not actively working on this part yet.

[xing.xu, 2017-01-26 07:00:36]

Clear! I am looking forward to it.

Thanks all your guys!

[sadrul, 2017-01-27 17:37:21]

On 2017/01/26 07:00:36, xing.xu wrote:
> Clear! I am looking forward to it.

Note that we would gladly accept patches for this too :-)

> 
> Thanks all your guys!