Remove content/gpu from content/browser/android/DEPS

Remove content/gpu from content/browser/android/DEPS

Synchronous compositor used this include to insert its own
SyncPointManager instance to be used by the in-process GPU thread.

Remove that DEPS and add ContentGpuClient::GetSyncPointManager so that
GPU code can directly get a SyncPointManager from content embedder. This
works for out of process GPU as well.

This also removes the InProcessCommandBuffer dependency from Android.

Committed: https://crrev.com/1384d6d1c0a5ac2407d546f619e1daade00db260
Cr-Commit-Position: refs/heads/master@{#389046}

[boliu, 2016-04-21 23:07:28]

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

[commit-bot: I haz the power, 2016-04-21 23:07:50]

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

[boliu, 2016-04-21 23:16:14]

Description was changed from

==========
Remove content/gpu from content/browser/android/DEPS

Synchronous compositor used this include to insert its own
SyncPointManager instance to be used by the in-process GPU thread.

Remove that DEPS and add ContentGpuClient::GetSyncPointManager so that
GPU code can directly get a SyncPointManager from content embedder.

This also removes the InProcessCommandBuffer dependency from Android.
==========

to

==========
Remove content/gpu from content/browser/android/DEPS

Synchronous compositor used this include to insert its own
SyncPointManager instance to be used by the in-process GPU thread.

Remove that DEPS and add ContentGpuClient::GetSyncPointManager so that
GPU code can directly get a SyncPointManager from content embedder. This
works for out of process GPU as well.

This also removes the InProcessCommandBuffer dependency from Android.
==========

[commit-bot: I haz the power, 2016-04-21 23:20:39]

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

[commit-bot: I haz the power, 2016-04-21 23:20:39]

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

[boliu, 2016-04-22 00:38:50]

boliu@chromium.org changed reviewers:
+ piman@chromium.org

[boliu, 2016-04-22 00:38:51]

ptal

Worthwhile to do this?

[piman, 2016-04-22 00:50:43]

lgtm

https://codereview.chromium.org/1908223002/diff/1/content/gpu/gpu_child_thread.h
File content/gpu/gpu_child_thread.h (right):

https://codereview.chromium.org/1908223002/diff/1/content/gpu/gpu_child_threa...
content/gpu/gpu_child_thread.h:163: // Can be null if overridden by
ContentGpuClient.
nit: it can't be null - if ContentGpuClient doesn't provide one, we create the
default one, right?

[boliu, 2016-04-22 01:08:36]

https://codereview.chromium.org/1908223002/diff/1/content/gpu/gpu_child_thread.h
File content/gpu/gpu_child_thread.h (right):

https://codereview.chromium.org/1908223002/diff/1/content/gpu/gpu_child_threa...
content/gpu/gpu_child_thread.h:163: // Can be null if overridden by
ContentGpuClient.
On 2016/04/22 00:50:43, piman wrote:
> nit: it can't be null - if ContentGpuClient doesn't provide one, we create the
> default one, right?

It's the opposite case. If ContentGpuClient provides a SyncPointManager, that
SyncPointManager can't be owned by GpuChildThread, so can't set as
sync_point_manager_ here.

So the comment is correct, but I'm not sure if anything else you want
clarified..?

[piman, 2016-04-22 04:05:08]

https://codereview.chromium.org/1908223002/diff/1/content/gpu/gpu_child_thread.h
File content/gpu/gpu_child_thread.h (right):

https://codereview.chromium.org/1908223002/diff/1/content/gpu/gpu_child_threa...
content/gpu/gpu_child_thread.h:163: // Can be null if overridden by
ContentGpuClient.
On 2016/04/22 01:08:36, boliu wrote:
> On 2016/04/22 00:50:43, piman wrote:
> > nit: it can't be null - if ContentGpuClient doesn't provide one, we create
the
> > default one, right?
> 
> It's the opposite case. If ContentGpuClient provides a SyncPointManager, that
> SyncPointManager can't be owned by GpuChildThread, so can't set as
> sync_point_manager_ here.
> 
> So the comment is correct, but I'm not sure if anything else you want
> clarified..?

Oh, I see - I misread the implementation because sync_point_manager and
sync_point_manager_ look alike.

Maybe make this owned_sync_point_manager_ to clarify that this is only used to
keep track of ownership?

[boliu, 2016-04-22 04:41:15]

https://codereview.chromium.org/1908223002/diff/1/content/gpu/gpu_child_thread.h
File content/gpu/gpu_child_thread.h (right):

https://codereview.chromium.org/1908223002/diff/1/content/gpu/gpu_child_threa...
content/gpu/gpu_child_thread.h:163: // Can be null if overridden by
ContentGpuClient.
On 2016/04/22 04:05:08, piman wrote:
> On 2016/04/22 01:08:36, boliu wrote:
> > On 2016/04/22 00:50:43, piman wrote:
> > > nit: it can't be null - if ContentGpuClient doesn't provide one, we create
> the
> > > default one, right?
> > 
> > It's the opposite case. If ContentGpuClient provides a SyncPointManager,
that
> > SyncPointManager can't be owned by GpuChildThread, so can't set as
> > sync_point_manager_ here.
> > 
> > So the comment is correct, but I'm not sure if anything else you want
> > clarified..?
> 
> Oh, I see - I misread the implementation because sync_point_manager and
> sync_point_manager_ look alike.
> 
> Maybe make this owned_sync_point_manager_ to clarify that this is only used to
> keep track of ownership?

Done. Also added a comment in .cc that manager from GpuClient can't be owned
here.

[boliu, 2016-04-22 04:41:43]

The CQ bit was checked by boliu@chromium.org

[boliu, 2016-04-22 04:41:44]

The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1908223002/#ps20001
(title: "owned_sync_point_manager_")

[commit-bot: I haz the power, 2016-04-22 04:41:59]

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

[commit-bot: I haz the power, 2016-04-22 06:39:02]

Description was changed from

==========
Remove content/gpu from content/browser/android/DEPS

Synchronous compositor used this include to insert its own
SyncPointManager instance to be used by the in-process GPU thread.

Remove that DEPS and add ContentGpuClient::GetSyncPointManager so that
GPU code can directly get a SyncPointManager from content embedder. This
works for out of process GPU as well.

This also removes the InProcessCommandBuffer dependency from Android.
==========

to

==========
Remove content/gpu from content/browser/android/DEPS

Synchronous compositor used this include to insert its own
SyncPointManager instance to be used by the in-process GPU thread.

Remove that DEPS and add ContentGpuClient::GetSyncPointManager so that
GPU code can directly get a SyncPointManager from content embedder. This
works for out of process GPU as well.

This also removes the InProcessCommandBuffer dependency from Android.
==========

[commit-bot: I haz the power, 2016-04-22 06:39:03]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-04-22 19:45:06]

Description was changed from

==========
Remove content/gpu from content/browser/android/DEPS

Synchronous compositor used this include to insert its own
SyncPointManager instance to be used by the in-process GPU thread.

Remove that DEPS and add ContentGpuClient::GetSyncPointManager so that
GPU code can directly get a SyncPointManager from content embedder. This
works for out of process GPU as well.

This also removes the InProcessCommandBuffer dependency from Android.
==========

to

==========
Remove content/gpu from content/browser/android/DEPS

Synchronous compositor used this include to insert its own
SyncPointManager instance to be used by the in-process GPU thread.

Remove that DEPS and add ContentGpuClient::GetSyncPointManager so that
GPU code can directly get a SyncPointManager from content embedder. This
works for out of process GPU as well.

This also removes the InProcessCommandBuffer dependency from Android.

Committed: https://crrev.com/1384d6d1c0a5ac2407d546f619e1daade00db260
Cr-Commit-Position: refs/heads/master@{#389046}
==========

[commit-bot: I haz the power, 2016-04-22 19:45:07]

Patchset 2 (id:??) landed as
https://crrev.com/1384d6d1c0a5ac2407d546f619e1daade00db260
Cr-Commit-Position: refs/heads/master@{#389046}