gpu: Have GpuService create and own GpuMemoryBufferFactory.

gpu: Have GpuService create and own GpuMemoryBufferFactory.

Move GpuMemoryBufferFactory ownership into GpuService. Some additional
cleanups:
. Update GpuMemoryBufferFactory::CreateNativeType() to return nullptr
  when native buffers are not supported, instead of the callers having to
  explicitly check first (using GetNativeGpuMemoryBufferType()).
. InProcessGpuThread::gpu_preferences_ is never used. So remove.
. Some now unnecessary #includes.

BUG=none
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2781293003
Cr-Commit-Position: refs/heads/master@{#461337}
Committed: https://chromium.googlesource.com/chromium/src/+/6d41b82585204a17ce52137c71722122221be72b

[sadrul, 2017-03-30 20:55:09]

Description was changed from

==========
gpu: Have GpuService create and own GpuMemoryBufferFactory.

...

BUG=...
==========

to

==========
gpu: Have GpuService create and own GpuMemoryBufferFactory.

...

BUG=...
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[sadrul, 2017-03-30 20:55:12]

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

[commit-bot: I haz the power, 2017-03-30 20:56:38]

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, 2017-03-30 21:19:33]

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

[commit-bot: I haz the power, 2017-03-30 21:19:33]

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

[sadrul, 2017-03-31 00:37:21]

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

[commit-bot: I haz the power, 2017-03-31 00:38:13]

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, 2017-03-31 00:45:35]

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

[commit-bot: I haz the power, 2017-03-31 00:45:36]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
  win_optional_gpu_tests_rel on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_optional_gpu_...)

[sadrul, 2017-03-31 01:26:09]

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

[commit-bot: I haz the power, 2017-03-31 01:26:37]

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, 2017-03-31 02:19:48]

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

[commit-bot: I haz the power, 2017-03-31 02:19:49]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[sadrul, 2017-03-31 02:20:30]

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

[commit-bot: I haz the power, 2017-03-31 02:20:56]

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

[sadrul, 2017-03-31 03:04:37]

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

[commit-bot: I haz the power, 2017-03-31 03:05:03]

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, 2017-03-31 04:06:09]

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

[commit-bot: I haz the power, 2017-03-31 04:06:10]

Dry run: This issue passed the CQ dry run.

[sadrul, 2017-03-31 04:46:07]

Description was changed from

==========
gpu: Have GpuService create and own GpuMemoryBufferFactory.

...

BUG=...
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
gpu: Have GpuService create and own GpuMemoryBufferFactory.

Move GpuMemoryBufferFactory ownership into GpuService. Some additional
cleanups:
. Update GpuMemoryBufferFactory::CreateNativeType() to return nullptr
  when native buffers are not supported, instead of the callers having to
  explicitly check first (using GetNativeGpuMemoryBufferType()).
. InProcessGpuThread::gpu_preferences_ is never used. So remove.
. Some now unnecessary #includes.

BUG=none
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[sadrul, 2017-03-31 04:50:24]

sadrul@chromium.org changed reviewers:
+ jbauman@chromium.org, reveman@chromium.org, tsepez@chromium.org

[sadrul, 2017-03-31 04:50:25]

tsepez@ for #include removals from gpu_host_messages.h

reveman@ for change in gpu_memory_buffer_factory

jbauman@ for the rest.

Thank you!

[reveman, 2017-03-31 06:13:34]

lgtm with nit

https://codereview.chromium.org/2781293003/diff/80001/gpu/ipc/service/gpu_mem...
File gpu/ipc/service/gpu_memory_buffer_factory.cc (right):

https://codereview.chromium.org/2781293003/diff/80001/gpu/ipc/service/gpu_mem...
gpu/ipc/service/gpu_memory_buffer_factory.cc:25: if
(GetNativeGpuMemoryBufferType() == gfx::EMPTY_BUFFER)
nit: can we just remove the NOTREACHED below instead? I think all devices that
will return non-null will have native type != gfx::EMPTY_BUFFER and even if they
didn't creating the factory if possible shouldn't hurt.

[sadrul, 2017-03-31 12:59:33]

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

[commit-bot: I haz the power, 2017-03-31 12:59:43]

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

[sadrul, 2017-03-31 13:09:15]

https://codereview.chromium.org/2781293003/diff/80001/gpu/ipc/service/gpu_mem...
File gpu/ipc/service/gpu_memory_buffer_factory.cc (right):

https://codereview.chromium.org/2781293003/diff/80001/gpu/ipc/service/gpu_mem...
gpu/ipc/service/gpu_memory_buffer_factory.cc:25: if
(GetNativeGpuMemoryBufferType() == gfx::EMPTY_BUFFER)
On 2017/03/31 06:13:34, reveman wrote:
> nit: can we just remove the NOTREACHED below instead? I think all devices that
> will return non-null will have native type != gfx::EMPTY_BUFFER and even if
they
> didn't creating the factory if possible shouldn't hurt.

Done.

[reveman, 2017-03-31 13:38:38]

https://codereview.chromium.org/2781293003/diff/100001/gpu/ipc/service/gpu_me...
File gpu/ipc/service/gpu_memory_buffer_factory.cc (right):

https://codereview.chromium.org/2781293003/diff/100001/gpu/ipc/service/gpu_me...
gpu/ipc/service/gpu_memory_buffer_factory.cc:10: #include
"gpu/ipc/common/gpu_memory_buffer_support.h"
no need for this after removing the lines below:

https://codereview.chromium.org/2781293003/diff/100001/gpu/ipc/service/gpu_me...
gpu/ipc/service/gpu_memory_buffer_factory.cc:26: return nullptr;
remove these two lines now that it's fine to return null below

[commit-bot: I haz the power, 2017-03-31 14:45:09]

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

[commit-bot: I haz the power, 2017-03-31 14:45:11]

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

[Tom Sepez, 2017-03-31 16:07:55]

RS LGTM.  Geting simpler.  Thanks.

[jbauman, 2017-04-01 09:02:00]

lgtm

[sadrul, 2017-04-02 02:07:09]

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

[commit-bot: I haz the power, 2017-04-02 02:07:15]

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

[sadrul, 2017-04-02 02:50:46]

The CQ bit was checked by sadrul@chromium.org

[sadrul, 2017-04-02 02:50:47]

The patchset sent to the CQ was uploaded after l-g-t-m from
reveman@chromium.org, tsepez@chromium.org, jbauman@chromium.org
Link to the patchset: https://codereview.chromium.org/2781293003/#ps120001
(title: "tot merge.")

[sadrul, 2017-04-02 02:50:50]

https://codereview.chromium.org/2781293003/diff/100001/gpu/ipc/service/gpu_me...
File gpu/ipc/service/gpu_memory_buffer_factory.cc (right):

https://codereview.chromium.org/2781293003/diff/100001/gpu/ipc/service/gpu_me...
gpu/ipc/service/gpu_memory_buffer_factory.cc:10: #include
"gpu/ipc/common/gpu_memory_buffer_support.h"
On 2017/03/31 13:38:38, reveman wrote:
> no need for this after removing the lines below:

Done.

https://codereview.chromium.org/2781293003/diff/100001/gpu/ipc/service/gpu_me...
gpu/ipc/service/gpu_memory_buffer_factory.cc:26: return nullptr;
On 2017/03/31 13:38:38, reveman wrote:
> remove these two lines now that it's fine to return null below

Done. Also, updated the doc to say that this can return null when native buffers
are not supported.

[commit-bot: I haz the power, 2017-04-02 02:50:56]

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, 2017-04-02 03:38:54]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1491101446926690,
"parent_rev": "9adf6fe51fa9fe65b9567fef84c5cb91a3146c06", "commit_rev":
"6d41b82585204a17ce52137c71722122221be72b"}

[commit-bot: I haz the power, 2017-04-02 03:40:40]

Description was changed from

==========
gpu: Have GpuService create and own GpuMemoryBufferFactory.

Move GpuMemoryBufferFactory ownership into GpuService. Some additional
cleanups:
. Update GpuMemoryBufferFactory::CreateNativeType() to return nullptr
  when native buffers are not supported, instead of the callers having to
  explicitly check first (using GetNativeGpuMemoryBufferType()).
. InProcessGpuThread::gpu_preferences_ is never used. So remove.
. Some now unnecessary #includes.

BUG=none
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
gpu: Have GpuService create and own GpuMemoryBufferFactory.

Move GpuMemoryBufferFactory ownership into GpuService. Some additional
cleanups:
. Update GpuMemoryBufferFactory::CreateNativeType() to return nullptr
  when native buffers are not supported, instead of the callers having to
  explicitly check first (using GetNativeGpuMemoryBufferType()).
. InProcessGpuThread::gpu_preferences_ is never used. So remove.
. Some now unnecessary #includes.

BUG=none
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2781293003
Cr-Commit-Position: refs/heads/master@{#461337}
Committed:
https://chromium.googlesource.com/chromium/src/+/6d41b82585204a17ce52137c7172...
==========

[commit-bot: I haz the power, 2017-04-02 03:40:41]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/6d41b82585204a17ce52137c7172...