Revert 72704 - 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

TBR=apatrick@chromium.org
Review URL: http://codereview.chromium.org/6370013

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

17 files changed, 50 insertions, 88 deletions

diff --git a/chrome/browser/dom_ui/gpu_internals_ui.cc b/chrome/browser/dom_ui/gpu_internals_ui.cc
index d5add7f..c9e9dda 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.level() == GPUInfo::kPartial) {
-    info->SetString("level", "partial");
+  if (gpu_info.progress() == GPUInfo::kPartial) {
+    info->SetString("progress", "partial");
   } else {
-    info->SetString("level", "complete");
+    info->SetString("progress", "complete");
   }
 #if defined(OS_WIN)
-  if (gpu_info.level() == GPUInfo::kComplete) {
+  if (gpu_info.progress() == GPUInfo::kComplete) {
     ListValue* dx_info = DxDiagNodeToList(gpu_info.dx_diagnostics());
     info->Set("diagnostics", dx_info);
   }
@@ -323,12 +323,11 @@ Value* GpuMessageHandler::OnRequestGpuInfo(const ListValue* list) {
   GPUInfo gpu_info = GpuProcessHostUIShim::GetInstance()->gpu_info();
 
   std::string html;
-  if (gpu_info.level() != GPUInfo::kComplete) {
-    GpuProcessHostUIShim::GetInstance()->CollectGraphicsInfoAsynchronously(
-        GPUInfo::kComplete);
+  if (gpu_info.progress() != GPUInfo::kComplete) {
+    GpuProcessHostUIShim::GetInstance()->CollectGraphicsInfoAsynchronously();
   }
 
-  if (gpu_info.level() != GPUInfo::kUninitialized) {
+  if (gpu_info.progress() != GPUInfo::kUninitialized) {
     return GpuInfoToDict(gpu_info);
   } else {
     return NULL;
diff --git a/chrome/browser/gpu_blacklist.cc b/chrome/browser/gpu_blacklist.cc
index 1c453a2..0fc87df 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.level() == GPUInfo::kUninitialized)
+  if (gpu_info.progress() == 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 807bf3ad..a4533b4 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.SetLevel(GPUInfo::kComplete);
+  gpu_info.SetProgress(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 9ae2fcd..5419a44 100644
--- a/chrome/browser/gpu_process_host_ui_shim.cc
+++ b/chrome/browser/gpu_process_host_ui_shim.cc
@@ -82,13 +82,12 @@ bool GpuProcessHostUIShim::OnMessageReceived(const IPC::Message& message) {
   return router_.RouteMessage(message);
 }
 
-void GpuProcessHostUIShim::CollectGraphicsInfoAsynchronously(
-    GPUInfo::Level level) {
+void GpuProcessHostUIShim::CollectGraphicsInfoAsynchronously() {
   DCHECK(CalledOnValidThread());
   BrowserThread::PostTask(
       BrowserThread::IO,
       FROM_HERE,
-      new SendOnIOThreadTask(new GpuMsg_CollectGraphicsInfo(level)));
+      new SendOnIOThreadTask(new GpuMsg_CollectGraphicsInfo()));
 }
 
 void GpuProcessHostUIShim::SendAboutGpuCrash() {
diff --git a/chrome/browser/gpu_process_host_ui_shim.h b/chrome/browser/gpu_process_host_ui_shim.h
index 98a813c..f070ed1 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(GPUInfo::Level level);
+  void CollectGraphicsInfoAsynchronously();
 
   // 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 8b47973..5a88e34 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()
-    : level_(kUninitialized),
+    : progress_(kUninitialized),
       vendor_id_(0),
       device_id_(0),
       driver_vendor_(""),
@@ -20,8 +20,8 @@ GPUInfo::GPUInfo()
       can_lose_context_(false) {
 }
 
-GPUInfo::Level GPUInfo::level() const {
-  return level_;
+GPUInfo::Progress GPUInfo::progress() const {
+  return progress_;
 }
 
 base::TimeDelta GPUInfo::initialization_time() const {
@@ -76,8 +76,8 @@ bool GPUInfo::can_lose_context() const {
   return can_lose_context_;
 }
 
-void GPUInfo::SetLevel(Level level) {
-  level_ = level;
+void GPUInfo::SetProgress(Progress progress) {
+  progress_ = progress;
 }
 
 void GPUInfo::SetInitializationTime(
diff --git a/chrome/common/gpu_info.h b/chrome/common/gpu_info.h
index 3989c7a..95c2237 100644
--- a/chrome/common/gpu_info.h
+++ b/chrome/common/gpu_info.h
@@ -22,16 +22,15 @@ class GPUInfo {
   GPUInfo();
   ~GPUInfo() {}
 
-  enum Level {
+  enum Progress {
     kUninitialized,
     kPartial,
-    kCompleting,
     kComplete,
   };
 
   // Returns whether this GPUInfo has been partially or fully initialized with
   // information.
-  Level level() const;
+  Progress progress() const;
 
   // The amount of time taken to get from the process starting to the message
   // loop being pumped.
@@ -88,7 +87,7 @@ class GPUInfo {
   // semantics are available.
   bool can_lose_context() const;
 
-  void SetLevel(Level level);
+  void SetProgress(Progress progress);
 
   void SetInitializationTime(const base::TimeDelta& initialization_time);
 
@@ -120,7 +119,7 @@ class GPUInfo {
 #endif
 
  private:
-  Level level_;
+  Progress progress_;
   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 34630e8..57227e8 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.level(), GPUInfo::kUninitialized);
+  EXPECT_EQ(gpu_info.progress(), 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 65a39d4..7f1f5c0 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.level()));
+  WriteParam(m, static_cast<int32>(p.progress()));
   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 level;
+  int32 progress;
   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, &level);
+  bool ret = ReadParam(m, iter, &progress);
   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->SetLevel(static_cast<GPUInfo::Level>(level));
+  p->SetProgress(static_cast<GPUInfo::Progress>(progress));
   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.level(),
+                               p.progress(),
                                static_cast<int32>(
                                    p.initialization_time().InMilliseconds()),
                                p.vendor_id(),
@@ -218,24 +218,6 @@ 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 6a488c3..22da39a 100644
--- a/chrome/common/gpu_messages_internal.h
+++ b/chrome/common/gpu_messages_internal.h
@@ -6,7 +6,6 @@
 #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"
 
@@ -56,8 +55,7 @@ IPC_MESSAGE_CONTROL0(GpuMsg_Synchronize)
 
 // Tells the GPU process to create a context for collecting graphics card
 // information.
-IPC_MESSAGE_CONTROL1(GpuMsg_CollectGraphicsInfo,
-                     GPUInfo::Level /* level */)
+IPC_MESSAGE_CONTROL0(GpuMsg_CollectGraphicsInfo)
 
 #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 4e2da79..b3f2c34 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.SetLevel(GPUInfo::kPartial);
+  input.SetProgress(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.level(), output.level());
+  EXPECT_EQ(input.progress(), output.progress());
   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 4f13ab9..3ebaca0 100644
--- a/chrome/common/gpu_param_traits.h
+++ b/chrome/common/gpu_param_traits.h
@@ -72,14 +72,6 @@ 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 e401e8f..a904164 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->SetLevel(GPUInfo::kComplete);
+  gpu_info->SetProgress(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 84b62d7..39efdbef 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->SetLevel(GPUInfo::kComplete);
+    gpu_info->SetProgress(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->SetLevel(GPUInfo::kPartial);
+  gpu_info->SetProgress(GPUInfo::kPartial);
   return true;
 }
 
diff --git a/chrome/gpu/gpu_thread.cc b/chrome/gpu/gpu_thread.cc
index 752478e..6e9b3da 100644
--- a/chrome/gpu/gpu_thread.cc
+++ b/chrome/gpu/gpu_thread.cc
@@ -95,6 +95,18 @@ 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_);
@@ -196,26 +208,7 @@ void GpuThread::OnSynchronize() {
   Send(new GpuHostMsg_SynchronizeReply());
 }
 
-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
-
+void GpuThread::OnCollectGraphicsInfo() {
   Send(new GpuHostMsg_GraphicsInfoCollected(gpu_info_));
 }
 
@@ -269,7 +262,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_.SetLevel(GPUInfo::kComplete);
+  thread->gpu_info_.SetProgress(GPUInfo::kComplete);
 }
 
 #endif
diff --git a/chrome/gpu/gpu_thread.h b/chrome/gpu/gpu_thread.h
index 1bbb225..6928645 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(GPUInfo::Level level);
+  void OnCollectGraphicsInfo();
 #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 221448f..8fcd37e 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.level() == GPUInfo::kUninitialized) {
+  if (info.progress() == GPUInfo::kUninitialized) {
     GPUInfoCollectedObserver observer(gpu_host_shim);
-    gpu_host_shim->CollectGraphicsInfoAsynchronously(GPUInfo::kPartial);
+    gpu_host_shim->CollectGraphicsInfoAsynchronously();
     ui_test_utils::RunMessageLoop();
     if (!observer.gpu_info_collected())
       return false;