Skip to content

Commit

Permalink
Fix DirectX diagnostic collection for about:gpu page.
Browse files Browse the repository at this point in the history
The DirectX diagnostics are collected by a separate GPU process that is not sandboxed. This is because the sandbox prevents them from being collected.
Review URL: https://chromiumcodereview.appspot.com/9212054

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@119534 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
apatrick@chromium.org committed Jan 28, 2012
1 parent 0889c17 commit 6f1485e
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 22 deletions.
6 changes: 4 additions & 2 deletions content/browser/gpu/gpu_data_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,10 @@ void GpuDataManager::RequestCompleteGpuInfoIfNeeded() {
void GpuDataManager::UpdateGpuInfo(const content::GPUInfo& gpu_info) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

complete_gpu_info_available_ = true;
complete_gpu_info_already_requested_ = true;
complete_gpu_info_available_ =
complete_gpu_info_available_ || gpu_info.finalized;
complete_gpu_info_already_requested_ =
complete_gpu_info_already_requested_ || gpu_info.finalized;
{
base::AutoLock auto_lock(gpu_info_lock_);
if (!Merge(&gpu_info_, gpu_info))
Expand Down
17 changes: 14 additions & 3 deletions content/browser/gpu/gpu_process_host.cc
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,9 @@ GpuProcessHost* GpuProcessHost::GetForRenderer(
!it.IsAtEnd(); it.Advance()) {
GpuProcessHost *host = it.GetCurrentValue();

if (host->sandboxed() != (client_id != 0))
continue;

if (HostIsValid(host))
return host;
}
Expand All @@ -214,7 +217,7 @@ GpuProcessHost* GpuProcessHost::GetForRenderer(
cause,
content::CAUSE_FOR_GPU_LAUNCH_MAX_ENUM);

GpuProcessHost* host = new GpuProcessHost(host_id);
GpuProcessHost* host = new GpuProcessHost(host_id, client_id != 0);
if (host->Init())
return host;

Expand Down Expand Up @@ -246,11 +249,12 @@ GpuProcessHost* GpuProcessHost::FromID(int host_id) {
return NULL;
}

GpuProcessHost::GpuProcessHost(int host_id)
GpuProcessHost::GpuProcessHost(int host_id, bool sandboxed)
: host_id_(host_id),
gpu_process_(base::kNullProcessHandle),
in_process_(false),
software_rendering_(false) {
software_rendering_(false),
sandboxed_(sandboxed) {
if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kSingleProcess) ||
CommandLine::ForCurrentProcess()->HasSwitch(switches::kInProcessGPU))
in_process_ = true;
Expand Down Expand Up @@ -519,6 +523,10 @@ bool GpuProcessHost::software_rendering() {
return software_rendering_;
}

bool GpuProcessHost::sandboxed() {
return sandboxed_;
}

void GpuProcessHost::ForceShutdown() {
g_hosts_by_id.Pointer()->Remove(host_id_);
process_->ForceShutdown();
Expand Down Expand Up @@ -551,6 +559,9 @@ bool GpuProcessHost::LaunchGpuProcess(const std::string& channel_id) {
cmd_line->AppendSwitchASCII(switches::kProcessType, switches::kGpuProcess);
cmd_line->AppendSwitchASCII(switches::kProcessChannelID, channel_id);

if (!sandboxed_)
cmd_line->AppendSwitch(switches::kDisableGpuSandbox);

// Propagate relevant command line switches.
static const char* const kSwitchNames[] = {
switches::kDisableBreakpad,
Expand Down
6 changes: 5 additions & 1 deletion content/browser/gpu/gpu_process_host.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,15 @@ class GpuProcessHost : public content::BrowserChildProcessHostDelegate,
// Whether this GPU process is set up to use software rendering.
bool software_rendering();

// Whether this GPU process is sandboxed.
bool sandboxed();

void ForceShutdown();

private:
static bool HostIsValid(GpuProcessHost* host);

GpuProcessHost(int host_id);
GpuProcessHost(int host_id, bool sandboxed);
virtual ~GpuProcessHost();

bool Init();
Expand Down Expand Up @@ -149,6 +152,7 @@ class GpuProcessHost : public content::BrowserChildProcessHostDelegate,
bool in_process_;

bool software_rendering_;
bool sandboxed_;

scoped_ptr<GpuMainThread> in_process_gpu_thread_;

Expand Down
2 changes: 1 addition & 1 deletion content/common/sandbox_policy.cc
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ base::ProcessHandle StartProcessWithAccess(CommandLine* cmd_line,

// If it is the GPU process then it can be disabled by a command line flag.
if ((type == content::PROCESS_TYPE_GPU) &&
(browser_command_line.HasSwitch(switches::kDisableGpuSandbox))) {
(cmd_line->HasSwitch(switches::kDisableGpuSandbox))) {
in_sandbox = false;
DVLOG(1) << "GPU sandbox is disabled";
}
Expand Down
30 changes: 15 additions & 15 deletions content/gpu/gpu_child_thread.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
#include "base/bind.h"
#include "base/command_line.h"
#include "base/threading/worker_pool.h"
#include "base/win/scoped_com_initializer.h"
#include "build/build_config.h"
#include "content/common/child_process.h"
#include "content/common/gpu/gpu_messages.h"
Expand Down Expand Up @@ -173,18 +172,21 @@ void GpuChildThread::OnCollectGraphicsInfo() {
// Prevent concurrent collection of DirectX diagnostics.
collecting_dx_diagnostics_ = true;

// Asynchronously collect the DirectX diagnostics because this can take a
// couple of seconds.
if (!base::WorkerPool::PostTask(
FROM_HERE, base::Bind(&GpuChildThread::CollectDxDiagnostics, this),
true)) {
// Flag GPU info as complete if the DirectX diagnostics cannot be
// collected.
collecting_dx_diagnostics_ = false;
gpu_info_.finalized = true;
} else {
// Do not send response if we are still completing the GPUInfo struct
return;
if (CommandLine::ForCurrentProcess()->HasSwitch(
switches::kDisableGpuSandbox)) {
// Asynchronously collect the DirectX diagnostics because this can take a
// couple of seconds.
if (!base::WorkerPool::PostTask(
FROM_HERE, base::Bind(&GpuChildThread::CollectDxDiagnostics, this),
true)) {
// Flag GPU info as complete if the DirectX diagnostics cannot be
// collected.
collecting_dx_diagnostics_ = false;
gpu_info_.finalized = true;
} else {
// Do not send response if we are still completing the GPUInfo struct
return;
}
}
}
#endif
Expand Down Expand Up @@ -217,8 +219,6 @@ void GpuChildThread::OnHang() {
// Runs on a worker thread. The GPU process never terminates voluntarily so
// it is safe to assume that its message loop is valid.
void GpuChildThread::CollectDxDiagnostics(GpuChildThread* thread) {
base::win::ScopedCOMInitializer com_initializer;

content::DxDiagNode node;
gpu_info_collector::GetDxDiagnostics(&node);

Expand Down
2 changes: 2 additions & 0 deletions content/gpu/gpu_dx_diagnostics_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

#include "base/string_number_conversions.h"
#include "base/utf_string_conversions.h"
#include "base/win/scoped_com_initializer.h"
#include "content/gpu/gpu_info_collector.h"

// Functions in this file depend on functions exported from dxguid.dll.
Expand Down Expand Up @@ -97,6 +98,7 @@ namespace gpu_info_collector {
bool GetDxDiagnostics(content::DxDiagNode* output) {
HRESULT hr;
bool success = false;
base::win::ScopedCOMInitializer com_initializer;

IDxDiagProvider* provider = NULL;
hr = CoCreateInstance(CLSID_DxDiagProvider,
Expand Down

0 comments on commit 6f1485e

Please sign in to comment.