Defered collect DirectX diagnostics until they are needed for about:gpu.

This is because collecting the stats often crashes.

Added a guard to prevent the collection of diagnostics on multiple threads simultaneously.

Renamed GPUInfo::Progress to GPUInfo::Level.

TEST=try, about:gpu does not cause concurrent diagnostics collection
BUG=none

Review URL: http://codereview.chromium.org/6364013

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@72704 0039d316-1c4b-4281-b951-d872f2087c98

17 files changed, 88 insertions, 50 deletions

diff --git a/chrome/browser/dom_ui/gpu_internals_ui.cc b/chrome/browser/dom_ui/gpu_internals_ui.cc
index c9e9dda..d5add7f 100644
--- a/chrome/browser/dom_ui/gpu_internals_ui.cc
+++ b/chrome/browser/dom_ui/gpu_internals_ui.cc
@@ -301,13 +301,13 @@ DictionaryValue* GpuInfoToDict(const GPUInfo& gpu_info) {
   DictionaryValue* info = new DictionaryValue();
   info->Set("basic_info", basic_info);
 
-  if (gpu_info.progress() == GPUInfo::kPartial) {
-    info->SetString("progress", "partial");
+  if (gpu_info.level() == GPUInfo::kPartial) {
+    info->SetString("level", "partial");
   } else {
-    info->SetString("progress", "complete");
+    info->SetString("level", "complete");
   }
 #if defined(OS_WIN)
-  if (gpu_info.progress() == GPUInfo::kComplete) {
+  if (gpu_info.level() == GPUInfo::kComplete) {
     ListValue* dx_info = DxDiagNodeToList(gpu_info.dx_diagnostics());
     info->Set("diagnostics", dx_info);
   }
@@ -323,11 +323,12 @@ Value* GpuMessageHandler::OnRequestGpuInfo(const ListValue* list) {
   GPUInfo gpu_info = GpuProcessHostUIShim::GetInstance()->gpu_info();
 
   std::string html;
-  if (gpu_info.progress() != GPUInfo::kComplete) {
-    GpuProcessHostUIShim::GetInstance()->CollectGraphicsInfoAsynchronously();
+  if (gpu_info.level() != GPUInfo::kComplete) {
+    GpuProcessHostUIShim::GetInstance()->CollectGraphicsInfoAsynchronously(
+        GPUInfo::kComplete);
   }
 
-  if (gpu_info.progress() != GPUInfo::kUninitialized) {
+  if (gpu_info.level() != GPUInfo::kUninitialized) {
     return GpuInfoToDict(gpu_info);
   } else {
     return NULL;
diff --git a/chrome/browser/gpu_blacklist.cc b/chrome/browser/gpu_blacklist.cc
index 0fc87df..1c453a2 100644
--- a/chrome/browser/gpu_blacklist.cc
+++ b/chrome/browser/gpu_blacklist.cc
@@ -489,7 +489,7 @@ GpuFeatureFlags GpuBlacklist::DetermineGpuFeatureFlags(
   active_entries_.clear();
   GpuFeatureFlags flags;
   // No need to go through blacklist entries if GPUInfo isn't available.
-  if (gpu_info.progress() == GPUInfo::kUninitialized)
+  if (gpu_info.level() == GPUInfo::kUninitialized)
     return flags;
   scoped_ptr<Version> driver_version(
       Version::GetVersionFromString(gpu_info.driver_version()));
diff --git a/chrome/browser/gpu_blacklist_unittest.cc b/chrome/browser/gpu_blacklist_unittest.cc
index a4533b4..807bf3ad 100644
--- a/chrome/browser/gpu_blacklist_unittest.cc
+++ b/chrome/browser/gpu_blacklist_unittest.cc
@@ -15,7 +15,7 @@ TEST(GpuBlacklistTest, BlacklistLogic) {
                             0x0640);  // Device ID
   gpu_info.SetDriverInfo("NVIDIA",  // Driver vendor
                          "1.6.18");  // Driver Version
-  gpu_info.SetProgress(GPUInfo::kComplete);
+  gpu_info.SetLevel(GPUInfo::kComplete);
   scoped_ptr<Version> os_version(Version::GetVersionFromString("10.6.4"));
 
   GpuBlacklist blacklist;
diff --git a/chrome/browser/gpu_process_host_ui_shim.cc b/chrome/browser/gpu_process_host_ui_shim.cc
index 5419a44..9ae2fcd 100644
--- a/chrome/browser/gpu_process_host_ui_shim.cc
+++ b/chrome/browser/gpu_process_host_ui_shim.cc
@@ -82,12 +82,13 @@ bool GpuProcessHostUIShim::OnMessageReceived(const IPC::Message& message) {
   return router_.RouteMessage(message);
 }
 
-void GpuProcessHostUIShim::CollectGraphicsInfoAsynchronously() {
+void GpuProcessHostUIShim::CollectGraphicsInfoAsynchronously(
+    GPUInfo::Level level) {
   DCHECK(CalledOnValidThread());
   BrowserThread::PostTask(
       BrowserThread::IO,
       FROM_HERE,
-      new SendOnIOThreadTask(new GpuMsg_CollectGraphicsInfo()));
+      new SendOnIOThreadTask(new GpuMsg_CollectGraphicsInfo(level)));
 }
 
 void GpuProcessHostUIShim::SendAboutGpuCrash() {
diff --git a/chrome/browser/gpu_process_host_ui_shim.h b/chrome/browser/gpu_process_host_ui_shim.h
index f070ed1..98a813c 100644
--- a/chrome/browser/gpu_process_host_ui_shim.h
+++ b/chrome/browser/gpu_process_host_ui_shim.h
@@ -51,7 +51,7 @@ class GpuProcessHostUIShim : public IPC::Channel::Sender,
 
   // Sends a message to the browser process to collect the information from the
   // graphics card.
-  void CollectGraphicsInfoAsynchronously();
+  void CollectGraphicsInfoAsynchronously(GPUInfo::Level level);
 
   // Tells the GPU process to crash. Useful for testing.
   void SendAboutGpuCrash();
diff --git a/chrome/common/gpu_info.cc b/chrome/common/gpu_info.cc
index 5a88e34..8b47973 100644
--- a/chrome/common/gpu_info.cc
+++ b/chrome/common/gpu_info.cc
@@ -5,7 +5,7 @@
 #include "chrome/common/gpu_info.h"
 
 GPUInfo::GPUInfo()
-    : progress_(kUninitialized),
+    : level_(kUninitialized),
       vendor_id_(0),
       device_id_(0),
       driver_vendor_(""),
@@ -20,8 +20,8 @@ GPUInfo::GPUInfo()
       can_lose_context_(false) {
 }
 
-GPUInfo::Progress GPUInfo::progress() const {
-  return progress_;
+GPUInfo::Level GPUInfo::level() const {
+  return level_;
 }
 
 base::TimeDelta GPUInfo::initialization_time() const {
@@ -76,8 +76,8 @@ bool GPUInfo::can_lose_context() const {
   return can_lose_context_;
 }
 
-void GPUInfo::SetProgress(Progress progress) {
-  progress_ = progress;
+void GPUInfo::SetLevel(Level level) {
+  level_ = level;
 }
 
 void GPUInfo::SetInitializationTime(
diff --git a/chrome/common/gpu_info.h b/chrome/common/gpu_info.h
index 95c2237..3989c7a 100644
--- a/chrome/common/gpu_info.h
+++ b/chrome/common/gpu_info.h
@@ -22,15 +22,16 @@ class GPUInfo {
   GPUInfo();
   ~GPUInfo() {}
 
-  enum Progress {
+  enum Level {
     kUninitialized,
     kPartial,
+    kCompleting,
     kComplete,
   };
 
   // Returns whether this GPUInfo has been partially or fully initialized with
   // information.
-  Progress progress() const;
+  Level level() const;
 
   // The amount of time taken to get from the process starting to the message
   // loop being pumped.
@@ -87,7 +88,7 @@ class GPUInfo {
   // semantics are available.
   bool can_lose_context() const;
 
-  void SetProgress(Progress progress);
+  void SetLevel(Level level);
 
   void SetInitializationTime(const base::TimeDelta& initialization_time);
 
@@ -119,7 +120,7 @@ class GPUInfo {
 #endif
 
  private:
-  Progress progress_;
+  Level level_;
   base::TimeDelta initialization_time_;
   uint32 vendor_id_;
   uint32 device_id_;
diff --git a/chrome/common/gpu_info_unittest.cc b/chrome/common/gpu_info_unittest.cc
index 57227e8..34630e8 100644
--- a/chrome/common/gpu_info_unittest.cc
+++ b/chrome/common/gpu_info_unittest.cc
@@ -8,7 +8,7 @@
 // Test that an empty GPUInfo has valid members
 TEST(GPUInfoBasicTest, EmptyGPUInfo) {
   GPUInfo gpu_info;
-  EXPECT_EQ(gpu_info.progress(), GPUInfo::kUninitialized);
+  EXPECT_EQ(gpu_info.level(), GPUInfo::kUninitialized);
   EXPECT_EQ(gpu_info.initialization_time().ToInternalValue(), 0);
   EXPECT_EQ(gpu_info.vendor_id(), 0u);
   EXPECT_EQ(gpu_info.device_id(), 0u);
diff --git a/chrome/common/gpu_messages.cc b/chrome/common/gpu_messages.cc
index 7f1f5c0..65a39d4 100644
--- a/chrome/common/gpu_messages.cc
+++ b/chrome/common/gpu_messages.cc
@@ -128,7 +128,7 @@ void ParamTraits<GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params> ::Log(
 #endif  // if defined(OS_MACOSX)
 
 void ParamTraits<GPUInfo> ::Write(Message* m, const param_type& p) {
-  WriteParam(m, static_cast<int32>(p.progress()));
+  WriteParam(m, static_cast<int32>(p.level()));
   WriteParam(m, p.initialization_time());
   WriteParam(m, p.vendor_id());
   WriteParam(m, p.device_id());
@@ -149,7 +149,7 @@ void ParamTraits<GPUInfo> ::Write(Message* m, const param_type& p) {
 }
 
 bool ParamTraits<GPUInfo> ::Read(const Message* m, void** iter, param_type* p) {
-  int32 progress;
+  int32 level;
   base::TimeDelta initialization_time;
   uint32 vendor_id;
   uint32 device_id;
@@ -163,7 +163,7 @@ bool ParamTraits<GPUInfo> ::Read(const Message* m, void** iter, param_type* p) {
   std::string gl_renderer;
   std::string gl_extensions;
   bool can_lose_context;
-  bool ret = ReadParam(m, iter, &progress);
+  bool ret = ReadParam(m, iter, &level);
   ret = ret && ReadParam(m, iter, &initialization_time);
   ret = ret && ReadParam(m, iter, &vendor_id);
   ret = ret && ReadParam(m, iter, &device_id);
@@ -177,7 +177,7 @@ bool ParamTraits<GPUInfo> ::Read(const Message* m, void** iter, param_type* p) {
   ret = ret && ReadParam(m, iter, &gl_renderer);
   ret = ret && ReadParam(m, iter, &gl_extensions);
   ret = ret && ReadParam(m, iter, &can_lose_context);
-  p->SetProgress(static_cast<GPUInfo::Progress>(progress));
+  p->SetLevel(static_cast<GPUInfo::Level>(level));
   if (!ret)
     return false;
 
@@ -205,7 +205,7 @@ bool ParamTraits<GPUInfo> ::Read(const Message* m, void** iter, param_type* p) {
 
 void ParamTraits<GPUInfo> ::Log(const param_type& p, std::string* l) {
   l->append(base::StringPrintf("<GPUInfo> %d %d %x %x %s %s %x %x %x %d",
-                               p.progress(),
+                               p.level(),
                                static_cast<int32>(
                                    p.initialization_time().InMilliseconds()),
                                p.vendor_id(),
@@ -218,6 +218,24 @@ void ParamTraits<GPUInfo> ::Log(const param_type& p, std::string* l) {
                                p.can_lose_context()));
 }
 
+
+void ParamTraits<GPUInfo::Level> ::Write(Message* m, const param_type& p) {
+  WriteParam(m, static_cast<int32>(p));
+}
+
+bool ParamTraits<GPUInfo::Level> ::Read(const Message* m,
+                                    void** iter,
+                                    param_type* p) {
+  int32 level;
+  bool ret = ReadParam(m, iter, &level);
+  *p = static_cast<GPUInfo::Level>(level);
+  return ret;
+}
+
+void ParamTraits<GPUInfo::Level> ::Log(const param_type& p, std::string* l) {
+  LogParam(static_cast<int32>(p), l);
+}
+
 void ParamTraits<DxDiagNode> ::Write(Message* m, const param_type& p) {
   WriteParam(m, p.values);
   WriteParam(m, p.children);
diff --git a/chrome/common/gpu_messages_internal.h b/chrome/common/gpu_messages_internal.h
index 22da39a..6a488c3 100644
--- a/chrome/common/gpu_messages_internal.h
+++ b/chrome/common/gpu_messages_internal.h
@@ -6,6 +6,7 @@
 #include <string>
 
 #include "base/shared_memory.h"
+#include "chrome/common/gpu_info.h"
 #include "chrome/common/gpu_video_common.h"
 #include "ipc/ipc_message_macros.h"
 
@@ -55,7 +56,8 @@ IPC_MESSAGE_CONTROL0(GpuMsg_Synchronize)
 
 // Tells the GPU process to create a context for collecting graphics card
 // information.
-IPC_MESSAGE_CONTROL0(GpuMsg_CollectGraphicsInfo)
+IPC_MESSAGE_CONTROL1(GpuMsg_CollectGraphicsInfo,
+                     GPUInfo::Level /* level */)
 
 #if defined(OS_MACOSX)
 // Tells the GPU process that the browser process handled the swap
diff --git a/chrome/common/gpu_messages_unittest.cc b/chrome/common/gpu_messages_unittest.cc
index b3f2c34..4e2da79 100644
--- a/chrome/common/gpu_messages_unittest.cc
+++ b/chrome/common/gpu_messages_unittest.cc
@@ -13,7 +13,7 @@
 TEST(GPUIPCMessageTest, GPUInfo) {
   GPUInfo input;
   // Test variables taken from HP Z600 Workstation
-  input.SetProgress(GPUInfo::kPartial);
+  input.SetLevel(GPUInfo::kPartial);
   input.SetInitializationTime(base::TimeDelta::FromMilliseconds(100));
   input.SetVideoCardInfo(0x10de, 0x0658);
   input.SetDriverInfo("NVIDIA", "195.36.24");
@@ -31,7 +31,7 @@ TEST(GPUIPCMessageTest, GPUInfo) {
   GPUInfo output;
   void* iter = NULL;
   EXPECT_TRUE(IPC::ReadParam(&msg, &iter, &output));
-  EXPECT_EQ(input.progress(), output.progress());
+  EXPECT_EQ(input.level(), output.level());
   EXPECT_EQ(input.initialization_time().InMilliseconds(),
             output.initialization_time().InMilliseconds());
   EXPECT_EQ(input.vendor_id(), output.vendor_id());
diff --git a/chrome/common/gpu_param_traits.h b/chrome/common/gpu_param_traits.h
index 3ebaca0..4f13ab9 100644
--- a/chrome/common/gpu_param_traits.h
+++ b/chrome/common/gpu_param_traits.h
@@ -72,6 +72,14 @@ struct ParamTraits<GPUInfo> {
 };
 
 template <>
+struct ParamTraits<GPUInfo::Level> {
+  typedef GPUInfo::Level param_type;
+  static void Write(Message* m, const param_type& p);
+  static bool Read(const Message* m, void** iter, param_type* p);
+  static void Log(const param_type& p, std::string* l);
+};
+
+template <>
 struct ParamTraits<DxDiagNode> {
   typedef DxDiagNode param_type;
   static void Write(Message* m, const param_type& p);
diff --git a/chrome/gpu/gpu_info_collector_mac.mm b/chrome/gpu/gpu_info_collector_mac.mm
index a904164..e401e8f 100644
--- a/chrome/gpu/gpu_info_collector_mac.mm
+++ b/chrome/gpu/gpu_info_collector_mac.mm
@@ -48,7 +48,7 @@ bool CollectGraphicsInfo(GPUInfo* gpu_info) {
 
   gpu_info->SetCanLoseContext(
       gfx::GetGLImplementation() == gfx::kGLImplementationEGLGLES2);
-  gpu_info->SetProgress(GPUInfo::kComplete);
+  gpu_info->SetLevel(GPUInfo::kComplete);
   return CollectGraphicsInfoGL(gpu_info);
 }
 
diff --git a/chrome/gpu/gpu_info_collector_win.cc b/chrome/gpu/gpu_info_collector_win.cc
index 39efdbef..84b62d7 100644
--- a/chrome/gpu/gpu_info_collector_win.cc
+++ b/chrome/gpu/gpu_info_collector_win.cc
@@ -24,7 +24,7 @@ bool CollectGraphicsInfo(GPUInfo* gpu_info) {
   DCHECK(gpu_info);
 
   if (gfx::GetGLImplementation() != gfx::kGLImplementationEGLGLES2) {
-    gpu_info->SetProgress(GPUInfo::kComplete);
+    gpu_info->SetLevel(GPUInfo::kComplete);
     return CollectGraphicsInfoGL(gpu_info);
   }
 
@@ -49,7 +49,7 @@ bool CollectGraphicsInfo(GPUInfo* gpu_info) {
 
   // DirectX diagnostics are collected asynchronously because it takes a
   // couple of seconds. Do not mark as complete until that is done.
-  gpu_info->SetProgress(GPUInfo::kPartial);
+  gpu_info->SetLevel(GPUInfo::kPartial);
   return true;
 }
 
diff --git a/chrome/gpu/gpu_thread.cc b/chrome/gpu/gpu_thread.cc
index 6e9b3da..752478e 100644
--- a/chrome/gpu/gpu_thread.cc
+++ b/chrome/gpu/gpu_thread.cc
@@ -95,18 +95,6 @@ void GpuThread::OnInitialize() {
   gpu_info_collector::CollectGraphicsInfo(&gpu_info_);
   child_process_logging::SetGpuInfo(gpu_info_);
 
-#if defined(OS_WIN)
-  // Asynchronously collect the DirectX diagnostics because this can take a
-  // couple of seconds.
-  if (!base::WorkerPool::PostTask(
-      FROM_HERE,
-      NewRunnableFunction(&GpuThread::CollectDxDiagnostics, this),
-      true)) {
-    // Flag GPU info as complete if the DirectX diagnostics cannot be collected.
-    gpu_info_.SetProgress(GPUInfo::kComplete);
-  }
-#endif
-
   // Record initialization only after collecting the GPU info because that can
   // take a significant amount of time.
   gpu_info_.SetInitializationTime(base::Time::Now() - process_start_time_);
@@ -208,7 +196,26 @@ void GpuThread::OnSynchronize() {
   Send(new GpuHostMsg_SynchronizeReply());
 }
 
-void GpuThread::OnCollectGraphicsInfo() {
+void GpuThread::OnCollectGraphicsInfo(GPUInfo::Level level) {
+#if defined(OS_WIN)
+  if (level == GPUInfo::kComplete && gpu_info_.level() <= GPUInfo::kPartial) {
+    // Prevent concurrent collection of DirectX diagnostics.
+    gpu_info_.SetLevel(GPUInfo::kCompleting);
+
+    // Asynchronously collect the DirectX diagnostics because this can take a
+    // couple of seconds.
+    if (!base::WorkerPool::PostTask(
+        FROM_HERE,
+        NewRunnableFunction(&GpuThread::CollectDxDiagnostics, this),
+        true)) {
+
+      // Flag GPU info as complete if the DirectX diagnostics cannot be
+      // collected.
+      gpu_info_.SetLevel(GPUInfo::kComplete);
+    }
+  }
+#endif
+
   Send(new GpuHostMsg_GraphicsInfoCollected(gpu_info_));
 }
 
@@ -262,7 +269,7 @@ void GpuThread::CollectDxDiagnostics(GpuThread* thread) {
 // Runs on the GPU thread.
 void GpuThread::SetDxDiagnostics(GpuThread* thread, const DxDiagNode& node) {
   thread->gpu_info_.SetDxDiagnostics(node);
-  thread->gpu_info_.SetProgress(GPUInfo::kComplete);
+  thread->gpu_info_.SetLevel(GPUInfo::kComplete);
 }
 
 #endif
diff --git a/chrome/gpu/gpu_thread.h b/chrome/gpu/gpu_thread.h
index 6928645..1bbb225 100644
--- a/chrome/gpu/gpu_thread.h
+++ b/chrome/gpu/gpu_thread.h
@@ -50,7 +50,7 @@ class GpuThread : public ChildThread {
   void OnEstablishChannel(int renderer_id);
   void OnCloseChannel(const IPC::ChannelHandle& channel_handle);
   void OnSynchronize();
-  void OnCollectGraphicsInfo();
+  void OnCollectGraphicsInfo(GPUInfo::Level level);
 #if defined(OS_MACOSX)
   void OnAcceleratedSurfaceBuffersSwappedACK(
       int renderer_id, int32 route_id, uint64 swap_buffers_count);
diff --git a/chrome/test/gpu/gpu_pixel_browsertest.cc b/chrome/test/gpu/gpu_pixel_browsertest.cc
index 8fcd37e..221448f 100644
--- a/chrome/test/gpu/gpu_pixel_browsertest.cc
+++ b/chrome/test/gpu/gpu_pixel_browsertest.cc
@@ -116,9 +116,9 @@ bool CollectGPUInfo(GPUInfo* client_info) {
   if (!gpu_host_shim)
     return false;
   GPUInfo info = gpu_host_shim->gpu_info();
-  if (info.progress() == GPUInfo::kUninitialized) {
+  if (info.level() == GPUInfo::kUninitialized) {
     GPUInfoCollectedObserver observer(gpu_host_shim);
-    gpu_host_shim->CollectGraphicsInfoAsynchronously();
+    gpu_host_shim->CollectGraphicsInfoAsynchronously(GPUInfo::kPartial);
     ui_test_utils::RunMessageLoop();
     if (!observer.gpu_info_collected())
       return false;