gpu: Introduce GpuInit.

gpu: Introduce GpuInit.

Consolidate some of the gpu initialization code into GpuInit, so that the same
initialization code can be used in chrome, and mus. GpuInit is responsible for:

. Creating (and owning) the GpuWatchdogThread (when applicable).
. Deciding when to enter the sandbox.
. Deciding whether GPU initialization failed (i.e. dead on arrival).
. Initializing GL (when applicable), and applying driver bug workarounds.
. Initializing dual-gpu support (when applicable).
. Computing GPUInfo.

Some possibly notable changes:
. In order to decide whether GL still needs to be initialized or not, the old
  code used to look at the --no-sandbox or --in-process-gpu flags. This change
  instead looks at the current GL implementation, and there isn't one, then it
  attempts to initialize GL.
. Changes the order of initialization of some of the components. For example,
  the old code used to initialize the message-loop after intializing dual-gpu
  support. It also used to initialize StatisticsRecorder, set thread priority
  after creating the watchdog etc. The new code initializes these first,
  before initializing the components GpuInit is responsible for.
. --disable-gpu-watchdog and --gpu-sandbox-start-early flags are moved into
  //gpu/ipc/service.
. Some gpu::GPUInfo related flags (e.g. --gpu-device-id, --gpu-driver-vendor,
  --gpu-driver-version, --gpu-driver-date, --gpu-vendor-id) move into
  //gpu/config.

BUG=643746
CQ_INCLUDE_TRYBOTS=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

Committed: https://crrev.com/454af333cd3626311ac08f7128ee48984b5ecfb1
Cr-Commit-Position: refs/heads/master@{#417634}

[sadrul, 2016-09-08 20:51:49]

Description was changed from

==========
[wip] gpu: GpuInit take 2

...

BUG=643746
==========

to

==========
[wip] gpu: GpuInit take 2

...

BUG=643746
CQ_INCLUDE_TRYBOTS=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, 2016-09-08 20:51:51]

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

[commit-bot: I haz the power, 2016-09-08 20:52:22]

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-09-08 21:29:04]

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

[commit-bot: I haz the power, 2016-09-08 21:29:05]

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_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[sadrul, 2016-09-09 01:09:53]

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

[commit-bot: I haz the power, 2016-09-09 01:10:06]

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-09-09 02:37:42]

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

[commit-bot: I haz the power, 2016-09-09 02:37:44]

Dry run: This issue passed the CQ dry run.

[sadrul, 2016-09-09 02:42:57]

Description was changed from

==========
[wip] gpu: GpuInit take 2

...

BUG=643746
CQ_INCLUDE_TRYBOTS=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: Introduce GpuInit.

Consolidate some of the gpu initialization code into GpuInit, so that the same
initialization code can be used in chrome, and mus. GpuInit is responsible for:

. Creating (and owning) the GpuWatchdogThread (when applicable).
. Deciding when to enter the sandbox.
. Whether GPU initialization failed (i.e. dead on arrival).
. Initializing GL (when applicable), and applying driver bug workarounds.
. Initializing dual-gpu support (when applicable).
. Computing GPUInfo.

Some possibly notable changes:
. In order to decide whether GL still needs to be initialized or not, the old
  code used to look at the --no-sandbox or --in-process-gpu flags. This change
  instead looks at the current GL implementation, and there isn't one, then it
  attempts to initialize GL.
. Changes the order of initialization of some of the components. For example,
  the old code used to initialize the message-loop after intializing dual-gpu
  support. It also used to initialize StatisticsRecorder, set thread priority
  after creating the watchdog etc. The new code initializes these first,
  before initializing the components GpuInit is responsible for.

BUG=643746
CQ_INCLUDE_TRYBOTS=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, 2016-09-09 02:47:41]

Description was changed from

==========
gpu: Introduce GpuInit.

Consolidate some of the gpu initialization code into GpuInit, so that the same
initialization code can be used in chrome, and mus. GpuInit is responsible for:

. Creating (and owning) the GpuWatchdogThread (when applicable).
. Deciding when to enter the sandbox.
. Whether GPU initialization failed (i.e. dead on arrival).
. Initializing GL (when applicable), and applying driver bug workarounds.
. Initializing dual-gpu support (when applicable).
. Computing GPUInfo.

Some possibly notable changes:
. In order to decide whether GL still needs to be initialized or not, the old
  code used to look at the --no-sandbox or --in-process-gpu flags. This change
  instead looks at the current GL implementation, and there isn't one, then it
  attempts to initialize GL.
. Changes the order of initialization of some of the components. For example,
  the old code used to initialize the message-loop after intializing dual-gpu
  support. It also used to initialize StatisticsRecorder, set thread priority
  after creating the watchdog etc. The new code initializes these first,
  before initializing the components GpuInit is responsible for.

BUG=643746
CQ_INCLUDE_TRYBOTS=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: Introduce GpuInit.

Consolidate some of the gpu initialization code into GpuInit, so that the same
initialization code can be used in chrome, and mus. GpuInit is responsible for:

. Creating (and owning) the GpuWatchdogThread (when applicable).
. Deciding when to enter the sandbox.
. Whether GPU initialization failed (i.e. dead on arrival).
. Initializing GL (when applicable), and applying driver bug workarounds.
. Initializing dual-gpu support (when applicable).
. Computing GPUInfo.

Some possibly notable changes:
. In order to decide whether GL still needs to be initialized or not, the old
  code used to look at the --no-sandbox or --in-process-gpu flags. This change
  instead looks at the current GL implementation, and there isn't one, then it
  attempts to initialize GL.
. Changes the order of initialization of some of the components. For example,
  the old code used to initialize the message-loop after intializing dual-gpu
  support. It also used to initialize StatisticsRecorder, set thread priority
  after creating the watchdog etc. The new code initializes these first,
  before initializing the components GpuInit is responsible for.
. --disable-gpu-watchdog and --gpu-sandbox-start-early flags are moved into
  //gpu/ipc/service.
. Some gpu::GPUInfo related flags (e.g. --gpu-device-id, --gpu-driver-vendor,
  --gpu-driver-version, --gpu-driver-date, --gpu-vendor-id) move into
  //gpu/config.

BUG=643746
CQ_INCLUDE_TRYBOTS=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, 2016-09-09 02:50:35]

Description was changed from

==========
gpu: Introduce GpuInit.

Consolidate some of the gpu initialization code into GpuInit, so that the same
initialization code can be used in chrome, and mus. GpuInit is responsible for:

. Creating (and owning) the GpuWatchdogThread (when applicable).
. Deciding when to enter the sandbox.
. Whether GPU initialization failed (i.e. dead on arrival).
. Initializing GL (when applicable), and applying driver bug workarounds.
. Initializing dual-gpu support (when applicable).
. Computing GPUInfo.

Some possibly notable changes:
. In order to decide whether GL still needs to be initialized or not, the old
  code used to look at the --no-sandbox or --in-process-gpu flags. This change
  instead looks at the current GL implementation, and there isn't one, then it
  attempts to initialize GL.
. Changes the order of initialization of some of the components. For example,
  the old code used to initialize the message-loop after intializing dual-gpu
  support. It also used to initialize StatisticsRecorder, set thread priority
  after creating the watchdog etc. The new code initializes these first,
  before initializing the components GpuInit is responsible for.
. --disable-gpu-watchdog and --gpu-sandbox-start-early flags are moved into
  //gpu/ipc/service.
. Some gpu::GPUInfo related flags (e.g. --gpu-device-id, --gpu-driver-vendor,
  --gpu-driver-version, --gpu-driver-date, --gpu-vendor-id) move into
  //gpu/config.

BUG=643746
CQ_INCLUDE_TRYBOTS=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: Introduce GpuInit.

Consolidate some of the gpu initialization code into GpuInit, so that the same
initialization code can be used in chrome, and mus. GpuInit is responsible for:

. Creating (and owning) the GpuWatchdogThread (when applicable).
. Deciding when to enter the sandbox.
. Deciding whether GPU initialization failed (i.e. dead on arrival).
. Initializing GL (when applicable), and applying driver bug workarounds.
. Initializing dual-gpu support (when applicable).
. Computing GPUInfo.

Some possibly notable changes:
. In order to decide whether GL still needs to be initialized or not, the old
  code used to look at the --no-sandbox or --in-process-gpu flags. This change
  instead looks at the current GL implementation, and there isn't one, then it
  attempts to initialize GL.
. Changes the order of initialization of some of the components. For example,
  the old code used to initialize the message-loop after intializing dual-gpu
  support. It also used to initialize StatisticsRecorder, set thread priority
  after creating the watchdog etc. The new code initializes these first,
  before initializing the components GpuInit is responsible for.
. --disable-gpu-watchdog and --gpu-sandbox-start-early flags are moved into
  //gpu/ipc/service.
. Some gpu::GPUInfo related flags (e.g. --gpu-device-id, --gpu-driver-vendor,
  --gpu-driver-version, --gpu-driver-date, --gpu-vendor-id) move into
  //gpu/config.

BUG=643746
CQ_INCLUDE_TRYBOTS=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, 2016-09-09 02:53:07]

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

[sadrul, 2016-09-09 02:53:08]

I have changed a lot of the first version of GpuInit (from
https://codereview.chromium.org/2289553002/) in response to your feedback.
GpuInit does not deal with message-loop, logging, error handlers etc. anymore.
PTAL.

[piman, 2016-09-09 03:32:41]

LGTM overall, I'm ok with this focused approach. Thanks for the work!

I have 2 comments overall, mostly about organization of the code to make it a
bit simpler to follow.

https://codereview.chromium.org/2322123002/diff/20001/content/gpu/gpu_main.cc
File content/gpu/gpu_main.cc (right):

https://codereview.chromium.org/2322123002/diff/20001/content/gpu/gpu_main.cc...
content/gpu/gpu_main.cc:140: WarmUpSandbox();
nit: maybe just inline WarmupSandbox here?

https://codereview.chromium.org/2322123002/diff/20001/gpu/ipc/service/gpu_ini...
File gpu/ipc/service/gpu_init.cc (right):

https://codereview.chromium.org/2322123002/diff/20001/gpu/ipc/service/gpu_ini...
gpu/ipc/service/gpu_init.cc:236: dead_on_arrival_ = IsDeadOnArrival();
How about returning a "success" bool from this function, allowing early-exits?

For example, the "gl::GetGLImplementation() == gl::kGLImplementationNone" check
on l.244 could simply be a "return false" on l.216. The
"gpu_info_.context_info_state == gpu::kCollectInfoFatalFailure" check could move
to l.197, etc.

(BTW, we don't have to turn on the sandbox if we're DOA, since we will exit
before handling any untrusted data)

[sadrul, 2016-09-09 05:51:02]

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

[commit-bot: I haz the power, 2016-09-09 05:51:12]

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-09-09 07:04:34]

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

[commit-bot: I haz the power, 2016-09-09 07:04:35]

Dry run: This issue passed the CQ dry run.

[sadrul, 2016-09-09 15:32:28]

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

[commit-bot: I haz the power, 2016-09-09 15:32:54]

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

[sadrul, 2016-09-09 15:38:04]

Thanks for the quick reviews!

https://codereview.chromium.org/2322123002/diff/20001/content/gpu/gpu_main.cc
File content/gpu/gpu_main.cc (right):

https://codereview.chromium.org/2322123002/diff/20001/content/gpu/gpu_main.cc...
content/gpu/gpu_main.cc:140: WarmUpSandbox();
On 2016/09/09 03:32:41, piman wrote:
> nit: maybe just inline WarmupSandbox here?

Done.

https://codereview.chromium.org/2322123002/diff/20001/gpu/ipc/service/gpu_ini...
File gpu/ipc/service/gpu_init.cc (right):

https://codereview.chromium.org/2322123002/diff/20001/gpu/ipc/service/gpu_ini...
gpu/ipc/service/gpu_init.cc:236: dead_on_arrival_ = IsDeadOnArrival();
On 2016/09/09 03:32:41, piman wrote:
> How about returning a "success" bool from this function, allowing early-exits?
> 
> For example, the "gl::GetGLImplementation() == gl::kGLImplementationNone"
check
> on l.244 could simply be a "return false" on l.216. The
> "gpu_info_.context_info_state == gpu::kCollectInfoFatalFailure" check could
move
> to l.197, etc.
> 
> (BTW, we don't have to turn on the sandbox if we're DOA, since we will exit
> before handling any untrusted data)

Thanks! That makes it much simpler. Moved all of checks in IsDeadOnArrival() in
a more appropriate place. So it's no longer needed. Done.

[piman, 2016-09-09 15:55:32]

LGTM, thanks!

[commit-bot: I haz the power, 2016-09-09 16:35:09]

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

[commit-bot: I haz the power, 2016-09-09 16:35:10]

Dry run: This issue passed the CQ dry run.

[sadrul, 2016-09-09 18:09:38]

The CQ bit was checked by sadrul@chromium.org

[commit-bot: I haz the power, 2016-09-09 18:10:23]

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-09-09 18:14:51]

Description was changed from

==========
gpu: Introduce GpuInit.

Consolidate some of the gpu initialization code into GpuInit, so that the same
initialization code can be used in chrome, and mus. GpuInit is responsible for:

. Creating (and owning) the GpuWatchdogThread (when applicable).
. Deciding when to enter the sandbox.
. Deciding whether GPU initialization failed (i.e. dead on arrival).
. Initializing GL (when applicable), and applying driver bug workarounds.
. Initializing dual-gpu support (when applicable).
. Computing GPUInfo.

Some possibly notable changes:
. In order to decide whether GL still needs to be initialized or not, the old
  code used to look at the --no-sandbox or --in-process-gpu flags. This change
  instead looks at the current GL implementation, and there isn't one, then it
  attempts to initialize GL.
. Changes the order of initialization of some of the components. For example,
  the old code used to initialize the message-loop after intializing dual-gpu
  support. It also used to initialize StatisticsRecorder, set thread priority
  after creating the watchdog etc. The new code initializes these first,
  before initializing the components GpuInit is responsible for.
. --disable-gpu-watchdog and --gpu-sandbox-start-early flags are moved into
  //gpu/ipc/service.
. Some gpu::GPUInfo related flags (e.g. --gpu-device-id, --gpu-driver-vendor,
  --gpu-driver-version, --gpu-driver-date, --gpu-vendor-id) move into
  //gpu/config.

BUG=643746
CQ_INCLUDE_TRYBOTS=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: Introduce GpuInit.

Consolidate some of the gpu initialization code into GpuInit, so that the same
initialization code can be used in chrome, and mus. GpuInit is responsible for:

. Creating (and owning) the GpuWatchdogThread (when applicable).
. Deciding when to enter the sandbox.
. Deciding whether GPU initialization failed (i.e. dead on arrival).
. Initializing GL (when applicable), and applying driver bug workarounds.
. Initializing dual-gpu support (when applicable).
. Computing GPUInfo.

Some possibly notable changes:
. In order to decide whether GL still needs to be initialized or not, the old
  code used to look at the --no-sandbox or --in-process-gpu flags. This change
  instead looks at the current GL implementation, and there isn't one, then it
  attempts to initialize GL.
. Changes the order of initialization of some of the components. For example,
  the old code used to initialize the message-loop after intializing dual-gpu
  support. It also used to initialize StatisticsRecorder, set thread priority
  after creating the watchdog etc. The new code initializes these first,
  before initializing the components GpuInit is responsible for.
. --disable-gpu-watchdog and --gpu-sandbox-start-early flags are moved into
  //gpu/ipc/service.
. Some gpu::GPUInfo related flags (e.g. --gpu-device-id, --gpu-driver-vendor,
  --gpu-driver-version, --gpu-driver-date, --gpu-vendor-id) move into
  //gpu/config.

BUG=643746
CQ_INCLUDE_TRYBOTS=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
==========

[commit-bot: I haz the power, 2016-09-09 18:14:53]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-09 18:16:33]

Description was changed from

==========
gpu: Introduce GpuInit.

Consolidate some of the gpu initialization code into GpuInit, so that the same
initialization code can be used in chrome, and mus. GpuInit is responsible for:

. Creating (and owning) the GpuWatchdogThread (when applicable).
. Deciding when to enter the sandbox.
. Deciding whether GPU initialization failed (i.e. dead on arrival).
. Initializing GL (when applicable), and applying driver bug workarounds.
. Initializing dual-gpu support (when applicable).
. Computing GPUInfo.

Some possibly notable changes:
. In order to decide whether GL still needs to be initialized or not, the old
  code used to look at the --no-sandbox or --in-process-gpu flags. This change
  instead looks at the current GL implementation, and there isn't one, then it
  attempts to initialize GL.
. Changes the order of initialization of some of the components. For example,
  the old code used to initialize the message-loop after intializing dual-gpu
  support. It also used to initialize StatisticsRecorder, set thread priority
  after creating the watchdog etc. The new code initializes these first,
  before initializing the components GpuInit is responsible for.
. --disable-gpu-watchdog and --gpu-sandbox-start-early flags are moved into
  //gpu/ipc/service.
. Some gpu::GPUInfo related flags (e.g. --gpu-device-id, --gpu-driver-vendor,
  --gpu-driver-version, --gpu-driver-date, --gpu-vendor-id) move into
  //gpu/config.

BUG=643746
CQ_INCLUDE_TRYBOTS=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: Introduce GpuInit.

Consolidate some of the gpu initialization code into GpuInit, so that the same
initialization code can be used in chrome, and mus. GpuInit is responsible for:

. Creating (and owning) the GpuWatchdogThread (when applicable).
. Deciding when to enter the sandbox.
. Deciding whether GPU initialization failed (i.e. dead on arrival).
. Initializing GL (when applicable), and applying driver bug workarounds.
. Initializing dual-gpu support (when applicable).
. Computing GPUInfo.

Some possibly notable changes:
. In order to decide whether GL still needs to be initialized or not, the old
  code used to look at the --no-sandbox or --in-process-gpu flags. This change
  instead looks at the current GL implementation, and there isn't one, then it
  attempts to initialize GL.
. Changes the order of initialization of some of the components. For example,
  the old code used to initialize the message-loop after intializing dual-gpu
  support. It also used to initialize StatisticsRecorder, set thread priority
  after creating the watchdog etc. The new code initializes these first,
  before initializing the components GpuInit is responsible for.
. --disable-gpu-watchdog and --gpu-sandbox-start-early flags are moved into
  //gpu/ipc/service.
. Some gpu::GPUInfo related flags (e.g. --gpu-device-id, --gpu-driver-vendor,
  --gpu-driver-version, --gpu-driver-date, --gpu-vendor-id) move into
  //gpu/config.

BUG=643746
CQ_INCLUDE_TRYBOTS=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

Committed: https://crrev.com/454af333cd3626311ac08f7128ee48984b5ecfb1
Cr-Commit-Position: refs/heads/master@{#417634}
==========

[commit-bot: I haz the power, 2016-09-09 18:16:35]

Patchset 4 (id:??) landed as
https://crrev.com/454af333cd3626311ac08f7128ee48984b5ecfb1
Cr-Commit-Position: refs/heads/master@{#417634}