New issue
Advanced search Search tips

Issue 899361 link

Starred by 2 users
Status: Fixed
Owner:
magchen@chromium.org
Closed: Nov 10
Cc:
zmo@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Mac
Pri: 2
Type: Bug



Sign in to add a comment

AppendExtraCommandLineSwitches called twice during GPU process launch

Project Member Reported by marshall@chromium.org, Oct 26 
Chrome Version: 71.0.3578.0
OS: Windows 10

What steps will reproduce the problem?
(1) Implement ContentBrowserClient::AppendExtraCommandLineSwitches in an application using the Content API and add a command-line switch.
(2) Run the application.
(3) View the GPU process command-line in Process Explorer.

What is the expected result?
The command-line switch added in AppendExtraCommandLineSwitches should only appear once.

What happens instead?
The command-line switch appears twice.

Please use labels and text to provide additional information.
The problem is that GpuProcessHost::LaunchGpuProcess calls ContentBrowserClient::AppendExtraCommandLineSwitches multiple times. First directly [1] and then indirectly via BrowserChildProcessHostImpl::Launch [2].

[1] https://cs.chromium.org/chromium/src/content/browser/gpu/gpu_process_host.cc?type=cs&q=GpuProcessHost::LaunchGpuProcess+AppendExtraCommandLineSwitches&sq=package:chromium&g=0&l=1066

[2] https://cs.chromium.org/chromium/src/content/browser/browser_child_process_host_impl.cc?type=cs&q=BrowserChildProcessHostImpl::Launch+AppendExtraCommandLineSwitches&sq=package:chromium&g=0&l=239

Comment 1 by enne@chromium.org, Nov 2

Owner: zmo@chromium.org
Status: Assigned (was: Untriaged)
zmo: could you take a look at this (or find somebody else to)?

Comment 2 by zmo@chromium.org, Nov 2

Cc: zmo@chromium.org
Labels: -Pri-3 Pri-2
Owner: magchen@chromium.org
Maggie: can you take a look at this bug?

Comment 3 by magchen@chromium.org, Nov 2 

I can reproduce the issue.
Project Member

Comment 4 by bugdroid1@chromium.org, Nov 10 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/47b8dc1d746cd4ebc2d725ab24891553602bfeaf

commit 47b8dc1d746cd4ebc2d725ab24891553602bfeaf
Author: Maggie Chen <magchen@chromium.org>
Date: Sat Nov 10 00:12:34 2018

Fix duplicate command line switches in GPU process

AppendExtraCommandLineSwitches was called twice during GPU process launch and
it caused some duplicate command line switches in GPU process.
In GpuProcessHost::LaunchGpuProcess, the second AppendExtraCommandLineSwitches
call was from BrowserChildProcessHostImpl::Launch().

The solution is to divide BrowserChildProcessHostImpl::Launch() into two parts
- AppendExtraCommandLineSwitches and LaunchWithoutExtraComandLineSwitches. The
GPU process will now call LaunchWithoutExtraComandLineSwitches to avoid running
AppendExtraCommandLineSwitches twice.

Bug:899361

Change-Id: I4ddfeaaf9624aac68e9ab68aabac6ed4e83f7a03
Reviewed-on: https://chromium-review.googlesource.com/c/1316660
Commit-Queue: Maggie Chen <magchen@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607043}
[modify] https://crrev.com/47b8dc1d746cd4ebc2d725ab24891553602bfeaf/content/browser/browser_child_process_host_impl.cc
[modify] https://crrev.com/47b8dc1d746cd4ebc2d725ab24891553602bfeaf/content/browser/browser_child_process_host_impl.h
[modify] https://crrev.com/47b8dc1d746cd4ebc2d725ab24891553602bfeaf/content/browser/gpu/gpu_process_host.cc

Comment 5 by magchen@chromium.org, Nov 10

Status: Fixed (was: Assigned)

Comment 6 by marshall@chromium.org, Nov 12 

Thanks for the quick fix :)
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 12 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dc307fcb2fc163aad76d80e033768008f8390045

commit dc307fcb2fc163aad76d80e033768008f8390045
Author: Maggie Chen <magchen@chromium.org>
Date: Mon Nov 12 23:56:24 2018

Add UMA GPU.GLImplementation to record the software rendering mode

The histogram GPU.GPUProcessSoftwareRendering recorded in
GpuProcessHost::LaunchGpuProcess() does not have a correct software rendering
status. GpuInit::InitializeAndStartSandbox() is where we check if we are doing
software rendering.

A new histogram GPU.GLImplementation is added to record all GL modes. If the
GPU.GLImplementation mode is kGLImplementationSwiftShaderGL, the GPU process is
running with software rendering.

Bug:899361

Change-Id: Ie2ef9b84c4055a2407128c3f4f27fbd802445485
Reviewed-on: https://chromium-review.googlesource.com/c/1317923
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: kylechar <kylechar@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Reviewed-by: Alexis Hétu <sugoi@chromium.org>
Commit-Queue: Maggie Chen <magchen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607377}
[modify] https://crrev.com/dc307fcb2fc163aad76d80e033768008f8390045/content/browser/gpu/gpu_process_host.cc
[modify] https://crrev.com/dc307fcb2fc163aad76d80e033768008f8390045/gpu/ipc/service/gpu_init.cc
[modify] https://crrev.com/dc307fcb2fc163aad76d80e033768008f8390045/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/dc307fcb2fc163aad76d80e033768008f8390045/tools/metrics/histograms/histograms.xml
[modify] https://crrev.com/dc307fcb2fc163aad76d80e033768008f8390045/ui/gl/gl_implementation.h
►

Sign in to add a comment