Make sure to run post init tasks even if kSkipGpuDataLoading is set

Make sure to run post init tasks even if kSkipGpuDataLoading is set

Fixes regression introduced by https://codereview.chromium.org/1547793004/
which postpones call to GpuDataManagerImplPrivate::SetGLStrings() and
GpuDataManagerImplPrivate::DisableHardwareAcceleration() until
GpuDataManagerImplPrivate::Initialize() is called.

A solution is to call RunPostInitTasks() for any early return in
GpuDataManagerImplPrivate::Initialize().

R=zmo@chromium.org

BUG=592435
TEST=content_shell --no-sandbox --disable-gpu --skip-gpu-data-loading chrome:gpu

Committed: https://crrev.com/67487fb9f97e4835d80fe1baa084a3cb97df7784
Cr-Commit-Position: refs/heads/master@{#380207}

[Julien Isorce Samsung, 2016-03-08 17:33:17]

1: Patch Set 1 is a solution. (I do not think it is a problem to call
SetGLStrings() if kSkipGpuDataLoading is set)

2: Other solution would be to add a boolean param to
DisableHardwareAcceleration() to avoid to postpone it. So when called from
constructor it would be set to false. And default value would be true /*
postpone if not initialized */.

3: An other solution also could be to split DisableHardwareAcceleration( into 2
functions: InternalDisableHardwareAcceleration() which would be called from the
constrcutor.

4: Last solution could be to just call the content directly from constructor:
if (command_line->HasSwitch(switches::kDisableGpu))
    DisableHardwareAcceleration();// call its content instead.

[Zhenyao Mo, 2016-03-08 19:02:48]

On 2016/03/08 17:33:17, j.isorce wrote:
> 1: Patch Set 1 is a solution. (I do not think it is a problem to call
> SetGLStrings() if kSkipGpuDataLoading is set)
> 
> 2: Other solution would be to add a boolean param to
> DisableHardwareAcceleration() to avoid to postpone it. So when called from
> constructor it would be set to false. And default value would be true /*
> postpone if not initialized */.
> 
> 3: An other solution also could be to split DisableHardwareAcceleration( into
2
> functions: InternalDisableHardwareAcceleration() which would be called from
the
> constrcutor.
> 
> 4: Last solution could be to just call the content directly from constructor:
> if (command_line->HasSwitch(switches::kDisableGpu))
>     DisableHardwareAcceleration();// call its content instead.

The patch set 1 solution looks ok to me, but can you add a unit test to make
sure when we pass both kDisableGPU and kSkipGpuDataLoading commandline switches,
we ended up disabling GPU?

[Julien Isorce Samsung, 2016-03-09 15:58:31]

On 2016/03/08 19:02:48, Zhenyao Mo wrote:
> The patch set 1 solution looks ok to me, but can you add a unit test to make
> sure when we pass both kDisableGPU and kSkipGpuDataLoading commandline
switches,
> we ended up disabling GPU?

I uploaded a browser test. Without the fix in the CL the new test fails with:

[ RUN      ] GpuProcess.skip_gpu_process
[24992:24992:0309/154932:ERROR:system_info_handler.cc(235)] SystemInfoHandler:
request for GPU info timed out! Most recent info sent.

So the new test would have caught the regression.

Also I wanted to check that the driver bug workaround list in chrome:gpu page is
empty (and other fields in the page to make sure they are empty), but I cannot
use self.script_to_evaluate_on_commit for that since it is overwritten later
from "class GpuProcess(gpu_test_base.TestBase): ...
page.script_to_evaluate_on_commit = test_harness_script in gpu_process.py"

Maybe later we can improve chrome.gpuBenchmarking to add API that retrieve gpu
information.

But for me it is ok as it is in Patch Set 2.

[Zhenyao Mo, 2016-03-09 17:18:45]

lgtm

Thanks

[Zhenyao Mo, 2016-03-09 17:18:52]

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

[commit-bot: I haz the power, 2016-03-09 17:18:58]

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

[commit-bot: I haz the power, 2016-03-09 18:48:22]

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

[commit-bot: I haz the power, 2016-03-09 18:48:22]

Dry run: This issue passed the CQ dry run.

[Julien Isorce Samsung, 2016-03-09 20:35:31]

The CQ bit was checked by j.isorce@samsung.com

[commit-bot: I haz the power, 2016-03-09 20:35:53]

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

[commit-bot: I haz the power, 2016-03-09 20:41:32]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-03-09 20:42:30]

Description was changed from

==========
Make sure to run post init tasks even if kSkipGpuDataLoading is set

Fixes regression introduced by https://codereview.chromium.org/1547793004/
which postpones call to GpuDataManagerImplPrivate::SetGLStrings() and
GpuDataManagerImplPrivate::DisableHardwareAcceleration() until
GpuDataManagerImplPrivate::Initialize() is called.

A solution is to call RunPostInitTasks() for any early return in
GpuDataManagerImplPrivate::Initialize().

R=zmo@chromium.org

BUG=592435

TEST=content_shell --no-sandbox --disable-gpu --skip-gpu-data-loading chrome:gpu
==========

to

==========
Make sure to run post init tasks even if kSkipGpuDataLoading is set

Fixes regression introduced by https://codereview.chromium.org/1547793004/
which postpones call to GpuDataManagerImplPrivate::SetGLStrings() and
GpuDataManagerImplPrivate::DisableHardwareAcceleration() until
GpuDataManagerImplPrivate::Initialize() is called.

A solution is to call RunPostInitTasks() for any early return in
GpuDataManagerImplPrivate::Initialize().

R=zmo@chromium.org

BUG=592435

TEST=content_shell --no-sandbox --disable-gpu --skip-gpu-data-loading chrome:gpu

Committed: https://crrev.com/67487fb9f97e4835d80fe1baa084a3cb97df7784
Cr-Commit-Position: refs/heads/master@{#380207}
==========

[commit-bot: I haz the power, 2016-03-09 20:42:31]

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