Refactoring GPUInfo: 1) remove the level field; instead, having a finalized field indicating if more info might be collected. 2) change version data from uint32 to string.

Refactoring GpuDataManager: 1) remove GpuFeatureAllowed() because we expose un-filtered flags through GpuFeatureFlags(). 2) disallow launch GPU process to collect gpu info if accelerated_compositing is not allowed.

BUG=none
TEST=unittest

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

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

23 files changed, 134 insertions, 186 deletions

diff --git a/chrome/browser/gpu_data_manager.cc b/chrome/browser/gpu_data_manager.cc
index 2e0a478..576ed4b 100644
--- a/chrome/browser/gpu_data_manager.cc
+++ b/chrome/browser/gpu_data_manager.cc
@@ -69,12 +69,12 @@ void GpuDataManager::RequestCompleteGpuInfoIfNeeded() {
 
   GpuProcessHostUIShim* ui_shim = GpuProcessHostUIShim::GetForRenderer(0);
   if (ui_shim)
-    ui_shim->CollectGpuInfoAsynchronously(GPUInfo::kComplete);
+    ui_shim->CollectGpuInfoAsynchronously();
 }
 
 void GpuDataManager::UpdateGpuInfo(const GPUInfo& gpu_info) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (gpu_info_.level >= gpu_info.level)
+  if (gpu_info_.finalized)
     return;
   gpu_info_ = gpu_info;
   child_process_logging::SetGpuInfo(gpu_info);
@@ -108,13 +108,20 @@ const ListValue& GpuDataManager::log_messages() const {
 GpuFeatureFlags GpuDataManager::GetGpuFeatureFlags() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   UpdateGpuFeatureFlags();
-  // We only need to return the bits that are not in the preliminary
-  // gpu feature flags because the latter work through renderer
-  // commandline switches.
-  uint32 mask = ~(preliminary_gpu_feature_flags_.flags());
-  GpuFeatureFlags masked_flags;
-  masked_flags.set_flags(gpu_feature_flags_.flags() & mask);
-  return masked_flags;
+  return gpu_feature_flags_;
+}
+
+bool GpuDataManager::GpuAccessAllowed() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  UpdateGpuFeatureFlags();
+  // We only need to block GPU process if more features are disallowed other
+  // than those in the preliminary gpu feature flags because the latter work
+  // through renderer commandline switches.
+  // However, if accelerated_compositing is not allowed, then we should always
+  // deny gpu access.
+  uint32 mask = (~(preliminary_gpu_feature_flags_.flags())) |
+                GpuFeatureFlags::kGpuFeatureAcceleratedCompositing;
+  return (gpu_feature_flags_.flags() & mask) == 0;
 }
 
 void GpuDataManager::AddGpuInfoUpdateCallback(Callback0::Type* callback) {
@@ -159,12 +166,6 @@ void GpuDataManager::AppendRendererCommandLine(
   }
 }
 
-bool GpuDataManager::GpuFeatureAllowed(uint32 feature) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  UpdateGpuFeatureFlags();
-  return ((gpu_feature_flags_.flags() & feature) == 0);
-}
-
 void GpuDataManager::RunGpuInfoUpdateCallbacks() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   std::set<Callback0::Type*>::iterator i = gpu_info_update_callbacks_.begin();
@@ -228,8 +229,6 @@ bool GpuDataManager::UpdateGpuBlacklist() {
 
 void GpuDataManager::UpdateGpuFeatureFlags() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  if (gpu_info_.level == GPUInfo::kUninitialized)
-    return;
 
   // Need to call this before checking gpu_feature_flags_set_ because it might
   // be reset if a newer version of GPU blacklist is downloaed.
@@ -249,7 +248,7 @@ void GpuDataManager::UpdateGpuFeatureFlags() {
     uint32 max_entry_id = gpu_blacklist->max_entry_id();
     if (gpu_feature_flags_.flags() != 0) {
       // If gpu is blacklisted, no further GPUInfo will be collected.
-      gpu_info_.level = GPUInfo::kComplete;
+      gpu_info_.finalized = true;
 
       // Notify clients that GpuInfo state has changed
       RunGpuInfoUpdateCallbacks();
diff --git a/chrome/browser/gpu_data_manager.h b/chrome/browser/gpu_data_manager.h
index 112405e..c2566fe 100644
--- a/chrome/browser/gpu_data_manager.h
+++ b/chrome/browser/gpu_data_manager.h
@@ -28,7 +28,7 @@ class GpuDataManager {
   // Requests complete GPUinfo if it has not already been requested
   void RequestCompleteGpuInfoIfNeeded();
 
-  // Only update if the level is higher than the cached GPUInfo level.
+  // Only update if the current GPUInfo is not finalized.
   void UpdateGpuInfo(const GPUInfo& gpu_info);
 
   const GPUInfo& gpu_info() const;
@@ -44,6 +44,13 @@ class GpuDataManager {
   // If necessary, compute the flags before returning them.
   GpuFeatureFlags GetGpuFeatureFlags();
 
+  // This indicator might change because we could collect more GPU info or
+  // because the GPU blacklist could be updated.
+  // If this returns false, any further GPU access, including launching GPU
+  // process, establish GPU channel, and GPU info collection, should be
+  // blocked.
+  bool GpuAccessAllowed();
+
   // Add a callback.
   void AddGpuInfoUpdateCallback(Callback0::Type* callback);
 
@@ -55,9 +62,6 @@ class GpuDataManager {
   // in correspondance to preliminary gpu feature flags.
   void AppendRendererCommandLine(CommandLine* command_line);
 
-  // If necessary, compute the flags before checking if a feature is allowed
-  bool GpuFeatureAllowed(uint32 feature);
-
  private:
   friend struct DefaultSingletonTraits<GpuDataManager>;
 
diff --git a/chrome/browser/gpu_process_host_ui_shim.cc b/chrome/browser/gpu_process_host_ui_shim.cc
index b45eca6..380a37c 100644
--- a/chrome/browser/gpu_process_host_ui_shim.cc
+++ b/chrome/browser/gpu_process_host_ui_shim.cc
@@ -112,6 +112,11 @@ GpuProcessHostUIShim::GpuProcessHostUIShim()
 GpuProcessHostUIShim* GpuProcessHostUIShim::GetForRenderer(int renderer_id) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
+  // Don't grant further access to GPU if it is not allowed.
+  GpuDataManager* gpu_data_manager = GpuDataManager::GetInstance();
+  if (gpu_data_manager != NULL && !gpu_data_manager->GpuAccessAllowed())
+    return NULL;
+
   // The current policy is to ignore the renderer ID and use a single GPU
   // process for all renderers. Later this will be extended to allow the
   // use of multiple GPU processes.
@@ -267,7 +272,7 @@ void GpuProcessHostUIShim::EstablishGpuChannel(
   linked_ptr<EstablishChannelCallback> wrapped_callback(callback);
 
   // If GPU features are already blacklisted, no need to establish the channel.
-  if (gpu_data_manager_->GetGpuFeatureFlags().flags() != 0) {
+  if (!gpu_data_manager_->GpuAccessAllowed()) {
     EstablishChannelError(
         wrapped_callback.release(), IPC::ChannelHandle(), NULL, GPUInfo());
     return;
@@ -332,14 +337,9 @@ void GpuProcessHostUIShim::DidDestroyAcceleratedSurface(int renderer_id,
 
 #endif
 
-void GpuProcessHostUIShim::CollectGpuInfoAsynchronously(
-    GPUInfo::Level level) {
+void GpuProcessHostUIShim::CollectGpuInfoAsynchronously() {
   DCHECK(CalledOnValidThread());
-
-  // If GPU is already blacklisted, no more info will be collected.
-  if (gpu_data_manager_->GetGpuFeatureFlags().flags() != 0)
-    return;
-  Send(new GpuMsg_CollectGraphicsInfo(level));
+  Send(new GpuMsg_CollectGraphicsInfo());
 }
 
 void GpuProcessHostUIShim::SendAboutGpuCrash() {
@@ -416,7 +416,7 @@ void GpuProcessHostUIShim::OnChannelEstablished(
   // Currently if any of the GPU features are blacklisted, we don't establish a
   // GPU channel.
   if (!channel_handle.name.empty() &&
-      gpu_data_manager_->GetGpuFeatureFlags().flags() != 0) {
+      !gpu_data_manager_->GpuAccessAllowed()) {
     Send(new GpuMsg_CloseChannel(channel_handle));
     EstablishChannelError(callback.release(),
                           IPC::ChannelHandle(),
diff --git a/chrome/browser/gpu_process_host_ui_shim.h b/chrome/browser/gpu_process_host_ui_shim.h
index a91f111..f57488b 100644
--- a/chrome/browser/gpu_process_host_ui_shim.h
+++ b/chrome/browser/gpu_process_host_ui_shim.h
@@ -49,6 +49,7 @@ class GpuProcessHostUIShim
   // Returns null on failure. It is not safe to store the pointer once control
   // has returned to the message loop as it can be destroyed. Instead store the
   // associated GPU host ID. A renderer ID of zero means the browser process.
+  // This could return NULL if GPU access is not allowed (blacklisted).
   static GpuProcessHostUIShim* GetForRenderer(int renderer_id);
 
   // Destroy the GpuProcessHostUIShim with the given host ID. This can only
@@ -113,7 +114,7 @@ class GpuProcessHostUIShim
 
   // Sends a message to the browser process to collect the information from the
   // graphics card.
-  void CollectGpuInfoAsynchronously(GPUInfo::Level level);
+  void CollectGpuInfoAsynchronously();
 
   // Tells the GPU process to crash. Useful for testing.
   void SendAboutGpuCrash();
diff --git a/chrome/browser/tab_contents/render_view_host_delegate_helper.cc b/chrome/browser/tab_contents/render_view_host_delegate_helper.cc
index a0a3278..f5d686b 100644
--- a/chrome/browser/tab_contents/render_view_host_delegate_helper.cc
+++ b/chrome/browser/tab_contents/render_view_host_delegate_helper.cc
@@ -326,14 +326,12 @@ WebPreferences RenderViewHostDelegateHelper::GetWebkitPrefs(
   {  // Certain GPU features might have been blacklisted.
     GpuDataManager* gpu_data_manager = GpuDataManager::GetInstance();
     DCHECK(gpu_data_manager);
-    if (!gpu_data_manager->GpuFeatureAllowed(
-            GpuFeatureFlags::kGpuFeatureAcceleratedCompositing))
+    uint32 blacklist_flags = gpu_data_manager->GetGpuFeatureFlags().flags();
+    if (blacklist_flags & GpuFeatureFlags::kGpuFeatureAcceleratedCompositing)
       web_prefs.accelerated_compositing_enabled = false;
-    if (!gpu_data_manager->GpuFeatureAllowed(
-            GpuFeatureFlags::kGpuFeatureWebgl))
+    if (blacklist_flags & GpuFeatureFlags::kGpuFeatureWebgl)
       web_prefs.experimental_webgl_enabled = false;
-    if (!gpu_data_manager->GpuFeatureAllowed(
-            GpuFeatureFlags::kGpuFeatureMultisampling))
+    if (blacklist_flags & GpuFeatureFlags::kGpuFeatureMultisampling)
       web_prefs.gl_multisampling_enabled = false;
   }
 
diff --git a/chrome/browser/ui/webui/gpu_internals_ui.cc b/chrome/browser/ui/webui/gpu_internals_ui.cc
index e8d6d3b..8e8bf2c 100644
--- a/chrome/browser/ui/webui/gpu_internals_ui.cc
+++ b/chrome/browser/ui/webui/gpu_internals_ui.cc
@@ -304,12 +304,6 @@ ListValue* DxDiagNodeToList(const DxDiagNode& node) {
 
 #endif  // OS_WIN
 
-std::string VersionNumberToString(uint32 value) {
-  int hi = (value >> 8) & 0xff;
-  int low = value & 0xff;
-  return base::IntToString(hi) + "." + base::IntToString(low);
-}
-
 DictionaryValue* GpuInfoToDict(const GPUInfo& gpu_info) {
   ListValue* basic_info = new ListValue();
   basic_info->Append(NewDescriptionValuePair("Initialization time",
@@ -325,11 +319,11 @@ DictionaryValue* GpuInfoToDict(const GPUInfo& gpu_info) {
   basic_info->Append(NewDescriptionValuePair("Driver date",
       gpu_info.driver_date));
   basic_info->Append(NewDescriptionValuePair("Pixel shader version",
-      VersionNumberToString(gpu_info.pixel_shader_version)));
+      gpu_info.pixel_shader_version));
   basic_info->Append(NewDescriptionValuePair("Vertex shader version",
-      VersionNumberToString(gpu_info.vertex_shader_version)));
+      gpu_info.vertex_shader_version));
   basic_info->Append(NewDescriptionValuePair("GL version",
-      VersionNumberToString(gpu_info.gl_version)));
+      gpu_info.gl_version));
   basic_info->Append(NewDescriptionValuePair("GL_VENDOR",
       gpu_info.gl_vendor));
   basic_info->Append(NewDescriptionValuePair("GL_RENDERER",
@@ -343,10 +337,8 @@ DictionaryValue* GpuInfoToDict(const GPUInfo& gpu_info) {
   info->Set("basic_info", basic_info);
 
 #if defined(OS_WIN)
-  if (gpu_info.level == GPUInfo::kComplete) {
-    ListValue* dx_info = DxDiagNodeToList(gpu_info.dx_diagnostics);
-    info->Set("diagnostics", dx_info);
-  }
+  ListValue* dx_info = DxDiagNodeToList(gpu_info.dx_diagnostics);
+  info->Set("diagnostics", dx_info);
 #endif
 
   return info;
@@ -360,8 +352,6 @@ Value* GpuMessageHandler::OnRequestLogMessages(const ListValue*) {
 
 void GpuMessageHandler::OnGpuInfoUpdate() {
   const GPUInfo& gpu_info = gpu_data_manager_->gpu_info();
-  if (gpu_info.level == GPUInfo::kUninitialized)
-    return;
 
   // Get GPU Info.
   DictionaryValue* gpu_info_val = GpuInfoToDict(gpu_info);
@@ -371,7 +361,6 @@ void GpuMessageHandler::OnGpuInfoUpdate() {
   if (blacklisting_reasons)
     gpu_info_val->Set("blacklistingReasons", blacklisting_reasons);
 
-
   // Send GPU Info to javascript.
   web_ui_->CallJavascriptFunction("browserBridge.onGpuInfoUpdate",
       *gpu_info_val);
diff --git a/chrome/common/child_process_logging_linux.cc b/chrome/common/child_process_logging_linux.cc
index fbf7b2d..f15536e 100644
--- a/chrome/common/child_process_logging_linux.cc
+++ b/chrome/common/child_process_logging_linux.cc
@@ -65,11 +65,11 @@ void SetGpuInfo(const GPUInfo& gpu_info) {
           kGpuStringSize - 1);
   g_gpu_driver_ver[kGpuStringSize - 1] = '\0';
   strncpy(g_gpu_ps_ver,
-          base::UintToString(gpu_info.pixel_shader_version).c_str(),
+          gpu_info.pixel_shader_version.c_str(),
           kGpuStringSize - 1);
   g_gpu_ps_ver[kGpuStringSize - 1] = '\0';
   strncpy(g_gpu_vs_ver,
-          base::UintToString(gpu_info.vertex_shader_version).c_str(),
+          gpu_info.vertex_shader_version.c_str(),
           kGpuStringSize - 1);
   g_gpu_vs_ver[kGpuStringSize - 1] = '\0';
 }
diff --git a/chrome/common/child_process_logging_mac.mm b/chrome/common/child_process_logging_mac.mm
index fd1ed16..0a13af0 100644
--- a/chrome/common/child_process_logging_mac.mm
+++ b/chrome/common/child_process_logging_mac.mm
@@ -155,13 +155,13 @@ void SetGpuInfoImpl(const GPUInfo& gpu_info,
                  gpu_info.driver_version,
                  set_key_func);
   SetGpuKeyValue(kGPUPixelShaderVersionParamName,
-                 base::UintToString(gpu_info.pixel_shader_version),
+                 gpu_info.pixel_shader_version,
                  set_key_func);
   SetGpuKeyValue(kGPUVertexShaderVersionParamName,
-                 base::UintToString(gpu_info.vertex_shader_version),
+                 gpu_info.vertex_shader_version,
                  set_key_func);
   SetGpuKeyValue(kGPUGLVersionParamName,
-                 base::UintToString(gpu_info.gl_version),
+                 gpu_info.gl_version,
                  set_key_func);
 }
 
diff --git a/chrome/common/child_process_logging_win.cc b/chrome/common/child_process_logging_win.cc
index a5b7d88..6f9650c 100644
--- a/chrome/common/child_process_logging_win.cc
+++ b/chrome/common/child_process_logging_win.cc
@@ -141,8 +141,8 @@ void SetGpuInfo(const GPUInfo& gpu_info) {
       base::StringPrintf(L"0x%04x", gpu_info.vendor_id).c_str(),
       base::StringPrintf(L"0x%04x", gpu_info.device_id).c_str(),
       UTF8ToUTF16(gpu_info.driver_version).c_str(),
-      base::UintToString16(gpu_info.pixel_shader_version).c_str(),
-      base::UintToString16(gpu_info.vertex_shader_version).c_str());
+      UTF8ToUTF16(gpu_info.pixel_shader_version).c_str(),
+      UTF8ToUTF16(gpu_info.vertex_shader_version).c_str());
 }
 
 void SetNumberOfViews(int number_of_views) {
diff --git a/content/browser/gpu_blacklist.cc b/content/browser/gpu_blacklist.cc
index 1724c5f..03c90b2 100644
--- a/content/browser/gpu_blacklist.cc
+++ b/content/browser/gpu_blacklist.cc
@@ -631,9 +631,6 @@ GpuFeatureFlags GpuBlacklist::DetermineGpuFeatureFlags(
     const GPUInfo& gpu_info) {
   active_entries_.clear();
   GpuFeatureFlags flags;
-  // No need to go through blacklist entries if GPUInfo isn't available.
-  if (gpu_info.level == GPUInfo::kUninitialized)
-    return flags;
 
   if (os == kOsAny)
     os = GetOsType();
diff --git a/content/browser/gpu_blacklist_unittest.cc b/content/browser/gpu_blacklist_unittest.cc
index 0394a06..4a4c1ea 100644
--- a/content/browser/gpu_blacklist_unittest.cc
+++ b/content/browser/gpu_blacklist_unittest.cc
@@ -31,7 +31,6 @@ class GpuBlacklistTest : public testing::Test {
     gpu_info_.driver_vendor = "NVIDIA";
     gpu_info_.driver_version = "1.6.18";
     gpu_info_.driver_date = "7-14-2009";
-    gpu_info_.level = GPUInfo::kComplete;
   }
 
   void TearDown() {
diff --git a/content/common/gpu_info.cc b/content/common/gpu_info.cc
index 55ecdbc..691917d 100644
--- a/content/common/gpu_info.cc
+++ b/content/common/gpu_info.cc
@@ -5,22 +5,8 @@
 #include "content/common/gpu_info.h"
 
 GPUInfo::GPUInfo()
-    : level(kUninitialized),
+    : finalized(false),
       vendor_id(0),
       device_id(0),
-      driver_vendor(""),
-      driver_version(""),
-      driver_date(""),
-      pixel_shader_version(0),
-      vertex_shader_version(0),
-      gl_version(0),
-      gl_version_string(""),
-      gl_vendor(""),
-      gl_renderer(""),
-      gl_extensions(""),
-      can_lose_context(false),
-      collection_error(false) {
-}
-
-GPUInfo::~GPUInfo() {
+      can_lose_context(false) {
 }
diff --git a/content/common/gpu_info.h b/content/common/gpu_info.h
index b571682..28edd31 100644
--- a/content/common/gpu_info.h
+++ b/content/common/gpu_info.h
@@ -18,19 +18,9 @@
 
 struct GPUInfo {
   GPUInfo();
-  ~GPUInfo();
 
-  enum Level {
-    kUninitialized,
-    kPreliminary,
-    kPartial,
-    kCompleting,
-    kComplete,
-  };
-
-  // Whether this GPUInfo has been partially or fully initialized with
-  // information.
-  Level level;
+  // Whether more GPUInfo fields might be collected in the future.
+  bool finalized;
 
   // The amount of time taken to get from the process starting to the message
   // loop being pumped.
@@ -52,23 +42,15 @@ struct GPUInfo {
   // The date of the graphics driver currently installed.
   std::string driver_date;
 
-  // The version of the pixel/fragment shader used by the gpu.  Major version in
-  // the second lowest 8 bits, minor in the lowest 8 bits, eg version 2.5 would
-  // be 0x00000205.
-  uint32 pixel_shader_version;
+  // The version of the pixel/fragment shader used by the gpu.
+  std::string pixel_shader_version;
 
-  // The version of the vertex shader used by the gpu.  Major version in the
-  // second lowest 8 bits, minor in the lowest 8 bits, eg version 2.5 would be
-  // 0x00000205.
-  uint32 vertex_shader_version;
+  // The version of the vertex shader used by the gpu.
+  std::string vertex_shader_version;
 
   // The version of OpenGL we are using.
-  // Major version in the second lowest 8 bits, minor in the lowest 8 bits,
-  // eg version 2.5 would be 0x00000205.
-  // Returns 0 if we're not using OpenGL, say because we're going through
-  // D3D instead.
   // TODO(zmo): should be able to tell if it's GL or GLES.
-  uint32 gl_version;
+  std::string gl_version;
 
   // The GL_VERSION string.  "" if we are not using OpenGL.
   std::string gl_version_string;
@@ -86,11 +68,6 @@ struct GPUInfo {
   // semantics are available.
   bool can_lose_context;
 
-  // True if there was an error at any stage of collecting GPUInfo data.
-  // If there was an error, then the GPUInfo fields may be incomplete or set
-  // to default values such as 0 or empty string.
-  bool collection_error;
-
 #if defined(OS_WIN)
   // The information returned by the DirectX Diagnostics Tool.
   DxDiagNode dx_diagnostics;
diff --git a/content/common/gpu_info_unittest.cc b/content/common/gpu_info_unittest.cc
index 8e99e37..34f0ee6 100644
--- a/content/common/gpu_info_unittest.cc
+++ b/content/common/gpu_info_unittest.cc
@@ -8,16 +8,16 @@
 // 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.finalized, false);
   EXPECT_EQ(gpu_info.initialization_time.ToInternalValue(), 0);
   EXPECT_EQ(gpu_info.vendor_id, 0u);
   EXPECT_EQ(gpu_info.device_id, 0u);
   EXPECT_EQ(gpu_info.driver_vendor, "");
   EXPECT_EQ(gpu_info.driver_version, "");
   EXPECT_EQ(gpu_info.driver_date, "");
-  EXPECT_EQ(gpu_info.pixel_shader_version, 0u);
-  EXPECT_EQ(gpu_info.vertex_shader_version, 0u);
-  EXPECT_EQ(gpu_info.gl_version, 0u);
+  EXPECT_EQ(gpu_info.pixel_shader_version, "");
+  EXPECT_EQ(gpu_info.vertex_shader_version, "");
+  EXPECT_EQ(gpu_info.gl_version, "");
   EXPECT_EQ(gpu_info.gl_version_string, "");
   EXPECT_EQ(gpu_info.gl_vendor, "");
   EXPECT_EQ(gpu_info.gl_renderer, "");
diff --git a/content/common/gpu_messages.h b/content/common/gpu_messages.h
index 558f6b3..a5731c8 100644
--- a/content/common/gpu_messages.h
+++ b/content/common/gpu_messages.h
@@ -88,10 +88,8 @@ IPC_STRUCT_TRAITS_BEGIN(DxDiagNode)
   IPC_STRUCT_TRAITS_MEMBER(children)
 IPC_STRUCT_TRAITS_END()
 
-IPC_ENUM_TRAITS(GPUInfo::Level)
-
 IPC_STRUCT_TRAITS_BEGIN(GPUInfo)
-  IPC_STRUCT_TRAITS_MEMBER(level)
+  IPC_STRUCT_TRAITS_MEMBER(finalized)
   IPC_STRUCT_TRAITS_MEMBER(initialization_time)
   IPC_STRUCT_TRAITS_MEMBER(vendor_id)
   IPC_STRUCT_TRAITS_MEMBER(device_id)
@@ -106,7 +104,6 @@ IPC_STRUCT_TRAITS_BEGIN(GPUInfo)
   IPC_STRUCT_TRAITS_MEMBER(gl_renderer)
   IPC_STRUCT_TRAITS_MEMBER(gl_extensions)
   IPC_STRUCT_TRAITS_MEMBER(can_lose_context)
-  IPC_STRUCT_TRAITS_MEMBER(collection_error)
 #if defined(OS_WIN)
   IPC_STRUCT_TRAITS_MEMBER(dx_diagnostics)
 #endif
@@ -153,8 +150,7 @@ IPC_MESSAGE_CONTROL4(GpuMsg_CreateViewCommandBuffer,
 
 // 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/content/gpu/gpu_info_collector.cc b/content/gpu/gpu_info_collector.cc
index 5211e9d..13b293e 100644
--- a/content/gpu/gpu_info_collector.cc
+++ b/content/gpu/gpu_info_collector.cc
@@ -55,8 +55,8 @@ std::string GetGLString(unsigned int pname) {
   return "";
 }
 
-uint32 GetVersionNumberFromString(const std::string& version_string) {
-  int major = 0, minor = 0;
+// Return a version string in the format of "major.minor".
+std::string GetVersionFromString(const std::string& version_string) {
   size_t begin = version_string.find_first_of("0123456789");
   if (begin != std::string::npos) {
     size_t end = version_string.find_first_not_of("01234567890.", begin);
@@ -67,12 +67,10 @@ uint32 GetVersionNumberFromString(const std::string& version_string) {
       sub_string = version_string.substr(begin);
     std::vector<std::string> pieces;
     base::SplitString(sub_string, '.', &pieces);
-    if (pieces.size() >= 2) {
-      base::StringToInt(pieces[0], &major);
-      base::StringToInt(pieces[1], &minor);
-    }
+    if (pieces.size() >= 2)
+      return pieces[0] + "." + pieces[1];
   }
-  return ((major << 8) + minor);
+  return "";
 }
 
 }  // namespace anonymous
@@ -107,10 +105,9 @@ bool CollectGLVersionInfo(GPUInfo* gpu_info) {
   std::string glsl_version_string =
       GetGLString(GL_SHADING_LANGUAGE_VERSION);
 
-  uint32 gl_version = GetVersionNumberFromString(gl_version_string);
-  gpu_info->gl_version = gl_version;
+  gpu_info->gl_version = GetVersionFromString(gl_version_string);
 
-  uint32 glsl_version = GetVersionNumberFromString(glsl_version_string);
+  std::string glsl_version = GetVersionFromString(glsl_version_string);
   gpu_info->pixel_shader_version = glsl_version;
   gpu_info->vertex_shader_version = glsl_version;
 
diff --git a/content/gpu/gpu_info_collector_linux.cc b/content/gpu/gpu_info_collector_linux.cc
index 08b1f9c..6ee7c97 100644
--- a/content/gpu/gpu_info_collector_linux.cc
+++ b/content/gpu/gpu_info_collector_linux.cc
@@ -143,15 +143,13 @@ bool CollectGraphicsInfo(GPUInfo* gpu_info) {
   // desktop GL and GL_ARB_robustness extension is available.
   gpu_info->can_lose_context =
       (gfx::GetGLImplementation() == gfx::kGLImplementationEGLGLES2);
-  gpu_info->level = GPUInfo::kComplete;
+  gpu_info->finalized = true;
   return CollectGraphicsInfoGL(gpu_info);
 }
 
 bool CollectPreliminaryGraphicsInfo(GPUInfo* gpu_info) {
   DCHECK(gpu_info);
 
-  gpu_info->level = GPUInfo::kPartial;
-
   bool rt = true;
   if (!CollectVideoCardInfo(gpu_info))
     rt = false;
diff --git a/content/gpu/gpu_info_collector_mac.mm b/content/gpu/gpu_info_collector_mac.mm
index e1b625d..c524fa7 100644
--- a/content/gpu/gpu_info_collector_mac.mm
+++ b/content/gpu/gpu_info_collector_mac.mm
@@ -49,15 +49,13 @@ bool CollectGraphicsInfo(GPUInfo* gpu_info) {
 
   gpu_info->can_lose_context =
       (gfx::GetGLImplementation() == gfx::kGLImplementationEGLGLES2);
-  gpu_info-> level = GPUInfo::kComplete;
+  gpu_info->finalized = true;
   return CollectGraphicsInfoGL(gpu_info);
 }
 
 bool CollectPreliminaryGraphicsInfo(GPUInfo* gpu_info) {
   DCHECK(gpu_info);
 
-  gpu_info->level = GPUInfo::kPartial;
-
   bool rt = true;
   if (!CollectVideoCardInfo(gpu_info))
     rt = false;
diff --git a/content/gpu/gpu_info_collector_unittest.cc b/content/gpu/gpu_info_collector_unittest.cc
index a441484..1cc4b64 100644
--- a/content/gpu/gpu_info_collector_unittest.cc
+++ b/content/gpu/gpu_info_collector_unittest.cc
@@ -27,8 +27,8 @@ class GPUInfoCollectorTest : public testing::Test {
     const uint32 device_id = 0x0658;
     const char* driver_vendor = "";  // not implemented
     const char* driver_version = "";
-    const uint32 shader_version = 0x00000128;
-    const uint32 gl_version = 0x00000301;
+    const char* shader_version = "1.40";
+    const char* gl_version = "3.1";
     const char* gl_renderer = "Quadro FX 380/PCI/SSE2";
     const char* gl_vendor = "NVIDIA Corporation";
     const char* gl_version_string = "3.1.0";
@@ -41,8 +41,8 @@ class GPUInfoCollectorTest : public testing::Test {
     const uint32 device_id = 0x0640;
     const char* driver_vendor = "";  // not implemented
     const char* driver_version = "1.6.18";
-    const uint32 shader_version = 0x00000114;
-    const uint32 gl_version = 0x00000201;
+    const char* shader_version = "1.20";
+    const char* gl_version = "2.1";
     const char* gl_renderer = "NVIDIA GeForce GT 120 OpenGL Engine";
     const char* gl_vendor = "NVIDIA Corporation";
     const char* gl_version_string = "2.1 NVIDIA-1.6.18";
@@ -55,8 +55,8 @@ class GPUInfoCollectorTest : public testing::Test {
     const uint32 device_id = 0x0658;
     const char* driver_vendor = "NVIDIA";
     const char* driver_version = "195.36.24";
-    const uint32 shader_version = 0x00000132;
-    const uint32 gl_version = 0x00000302;
+    const char* shader_version = "1.50";
+    const char* gl_version = "3.2";
     const char* gl_renderer = "Quadro FX 380/PCI/SSE2";
     const char* gl_vendor = "NVIDIA Corporation";
     const char* gl_version_string = "3.2.0 NVIDIA 195.36.24";
@@ -69,7 +69,6 @@ class GPUInfoCollectorTest : public testing::Test {
     test_values_.device_id = device_id;
     test_values_.driver_vendor = driver_vendor;
     test_values_.driver_version =driver_version;
-    test_values_.driver_date = "";
     test_values_.pixel_shader_version = shader_version;
     test_values_.vertex_shader_version = shader_version;
     test_values_.gl_version = gl_version;
@@ -113,8 +112,8 @@ class GPUInfoCollectorTest : public testing::Test {
 TEST_F(GPUInfoCollectorTest, DriverVendorGL) {
   GPUInfo gpu_info;
   gpu_info_collector::CollectGraphicsInfoGL(&gpu_info);
-  std::string driver_vendor = gpu_info.driver_vendor;
-  EXPECT_EQ(test_values_.driver_vendor, driver_vendor);
+  EXPECT_EQ(test_values_.driver_vendor,
+            gpu_info.driver_vendor);
 }
 
 // Skip Windows because the driver version is obtained from bot registry.
@@ -122,56 +121,56 @@ TEST_F(GPUInfoCollectorTest, DriverVendorGL) {
 TEST_F(GPUInfoCollectorTest, DriverVersionGL) {
   GPUInfo gpu_info;
   gpu_info_collector::CollectGraphicsInfoGL(&gpu_info);
-  std::string driver_version = gpu_info.driver_version;
-  EXPECT_EQ(test_values_.driver_version, driver_version);
+  EXPECT_EQ(test_values_.driver_version,
+            gpu_info.driver_version);
 }
 #endif
 
 TEST_F(GPUInfoCollectorTest, PixelShaderVersionGL) {
   GPUInfo gpu_info;
   gpu_info_collector::CollectGraphicsInfoGL(&gpu_info);
-  uint32 ps_version = gpu_info.pixel_shader_version;
-  EXPECT_EQ(test_values_.pixel_shader_version, ps_version);
+  EXPECT_EQ(test_values_.pixel_shader_version,
+            gpu_info.pixel_shader_version);
 }
 
 TEST_F(GPUInfoCollectorTest, VertexShaderVersionGL) {
   GPUInfo gpu_info;
   gpu_info_collector::CollectGraphicsInfoGL(&gpu_info);
-  uint32 vs_version = gpu_info.vertex_shader_version;
-  EXPECT_EQ(test_values_.vertex_shader_version, vs_version);
+  EXPECT_EQ(test_values_.vertex_shader_version,
+            gpu_info.vertex_shader_version);
 }
 
 TEST_F(GPUInfoCollectorTest, GLVersionGL) {
   GPUInfo gpu_info;
   gpu_info_collector::CollectGraphicsInfoGL(&gpu_info);
-  uint32 gl_version = gpu_info.gl_version;
-  EXPECT_EQ(test_values_.gl_version, gl_version);
+  EXPECT_EQ(test_values_.gl_version,
+            gpu_info.gl_version);
 }
 
 TEST_F(GPUInfoCollectorTest, GLVersionStringGL) {
   GPUInfo gpu_info;
   gpu_info_collector::CollectGraphicsInfoGL(&gpu_info);
-  std::string gl_version_string = gpu_info.gl_version_string;
-  EXPECT_EQ(test_values_.gl_version_string, gl_version_string);
+  EXPECT_EQ(test_values_.gl_version_string,
+            gpu_info.gl_version_string);
 }
 
 TEST_F(GPUInfoCollectorTest, GLRendererGL) {
   GPUInfo gpu_info;
   gpu_info_collector::CollectGraphicsInfoGL(&gpu_info);
-  std::string gl_renderer = gpu_info.gl_renderer;
-  EXPECT_EQ(test_values_.gl_renderer, gl_renderer);
+  EXPECT_EQ(test_values_.gl_renderer,
+            gpu_info.gl_renderer);
 }
 
 TEST_F(GPUInfoCollectorTest, GLVendorGL) {
   GPUInfo gpu_info;
   gpu_info_collector::CollectGraphicsInfoGL(&gpu_info);
-  std::string gl_vendor = gpu_info.gl_vendor;
-  EXPECT_EQ(test_values_.gl_vendor, gl_vendor);
+  EXPECT_EQ(test_values_.gl_vendor,
+            gpu_info.gl_vendor);
 }
 
 TEST_F(GPUInfoCollectorTest, GLExtensionsGL) {
   GPUInfo gpu_info;
   gpu_info_collector::CollectGraphicsInfoGL(&gpu_info);
-  std::string gl_extensions = gpu_info.gl_extensions;
-  EXPECT_EQ(test_values_.gl_extensions, gl_extensions);
+  EXPECT_EQ(test_values_.gl_extensions,
+            gpu_info.gl_extensions);
 }
diff --git a/content/gpu/gpu_info_collector_win.cc b/content/gpu/gpu_info_collector_win.cc
index 35ab7c0..75302d0 100644
--- a/content/gpu/gpu_info_collector_win.cc
+++ b/content/gpu/gpu_info_collector_win.cc
@@ -20,6 +20,19 @@
 #include "libEGL/main.h"
 #include "libEGL/Display.h"
 
+namespace {
+
+// The version number stores the major and minor version in the least 16 bits;
+// for example, 2.5 is 0x00000205.
+// Returned string is in the format of "major.minor".
+std::string VersionNumberToString(uint32 version_number) {
+  int hi = (version_number >> 8) & 0xff;
+  int low = version_number & 0xff;
+  return base::IntToString(hi) + "." + base::IntToString(low);
+}
+
+}  // namespace anonymous
+
 // Setup API functions
 typedef HDEVINFO (WINAPI*SetupDiGetClassDevsWFunc)(
     CONST GUID *ClassGuid,
@@ -51,13 +64,10 @@ bool CollectGraphicsInfo(GPUInfo* gpu_info) {
   DCHECK(gpu_info);
 
   if (gfx::GetGLImplementation() != gfx::kGLImplementationEGLGLES2) {
-    gpu_info->level = GPUInfo::kComplete;
+    gpu_info->finalized = true;
     return CollectGraphicsInfoGL(gpu_info);
   }
 
-  // Set to partial now in case this function returns false below.
-  gpu_info->level = GPUInfo::kPartial;
-
   // TODO(zmo): the following code only works if running on top of ANGLE.
   // Need to handle the case when running on top of real EGL/GLES2 drivers.
 
@@ -91,8 +101,6 @@ bool CollectGraphicsInfo(GPUInfo* gpu_info) {
 bool CollectPreliminaryGraphicsInfo(GPUInfo* gpu_info) {
   DCHECK(gpu_info);
 
-  gpu_info->level = GPUInfo::kPreliminary;
-
   bool rt = true;
   if (!CollectVideoCardInfo(gpu_info))
     rt = false;
@@ -111,8 +119,10 @@ bool CollectGraphicsInfoD3D(IDirect3D9* d3d, GPUInfo* gpu_info) {
   if (d3d->GetDeviceCaps(D3DADAPTER_DEFAULT,
                          D3DDEVTYPE_HAL,
                          &d3d_caps) == D3D_OK) {
-    gpu_info->pixel_shader_version = d3d_caps.PixelShaderVersion;
-    gpu_info->vertex_shader_version = d3d_caps.VertexShaderVersion;
+    gpu_info->pixel_shader_version =
+        VersionNumberToString(d3d_caps.PixelShaderVersion);
+    gpu_info->vertex_shader_version =
+        VersionNumberToString(d3d_caps.VertexShaderVersion);
   } else {
     LOG(ERROR) << "d3d->GetDeviceCaps() failed";
     succeed = false;
diff --git a/content/gpu/gpu_info_unittest_win.cc b/content/gpu/gpu_info_unittest_win.cc
index d468a48..182f2d3 100644
--- a/content/gpu/gpu_info_unittest_win.cc
+++ b/content/gpu/gpu_info_unittest_win.cc
@@ -47,13 +47,11 @@ class GPUInfoTest : public testing::Test {
 TEST_F(GPUInfoTest, PixelShaderVersionD3D) {
   GPUInfo gpu_info;
   ASSERT_TRUE(gpu_info_collector::CollectGraphicsInfoD3D(&d3d_, &gpu_info));
-  uint32 ps_version = gpu_info.pixel_shader_version;
-  EXPECT_EQ(ps_version, D3DPS_VERSION(3, 0));
+  EXPECT_EQ(gpu_info.pixel_shader_version, "3.0");
 }
 
 TEST_F(GPUInfoTest, VertexShaderVersionD3D) {
   GPUInfo gpu_info;
   ASSERT_TRUE(gpu_info_collector::CollectGraphicsInfoD3D(&d3d_, &gpu_info));
-  uint32 vs_version = gpu_info.vertex_shader_version;
-  EXPECT_EQ(vs_version, D3DVS_VERSION(3, 0));
+  EXPECT_EQ(gpu_info.vertex_shader_version, "3.0");
 }
diff --git a/content/gpu/gpu_thread.cc b/content/gpu/gpu_thread.cc
index 7eaf7fd..3dc960e 100644
--- a/content/gpu/gpu_thread.cc
+++ b/content/gpu/gpu_thread.cc
@@ -48,7 +48,8 @@ bool InitializeGpuSandbox() {
 
 #if defined(OS_WIN)
 GpuThread::GpuThread(sandbox::TargetServices* target_services)
-    : target_services_(target_services) {
+    : target_services_(target_services),
+      collecting_dx_diagnostics_(false) {
 }
 #else
 GpuThread::GpuThread() {
@@ -129,11 +130,7 @@ void GpuThread::OnInitialize() {
     MessageLoop::current()->Quit();
     return;
   }
-  bool gpu_info_result = gpu_info_collector::CollectGraphicsInfo(&gpu_info_);
-  if (!gpu_info_result) {
-    gpu_info_.collection_error = true;
-    LOG(ERROR) << "gpu_info_collector::CollectGraphicsInfo() failed";
-  }
+  gpu_info_collector::CollectGraphicsInfo(&gpu_info_);
 
   content::GetContentClient()->SetGpuInfo(gpu_info_);
   LOG(INFO) << "gpu_info_collector::CollectGraphicsInfo complete";
@@ -247,11 +244,11 @@ void GpuThread::OnSynchronize() {
   Send(new GpuHostMsg_SynchronizeReply());
 }
 
-void GpuThread::OnCollectGraphicsInfo(GPUInfo::Level level) {
+void GpuThread::OnCollectGraphicsInfo() {
 #if defined(OS_WIN)
-  if (level == GPUInfo::kComplete && gpu_info_.level <= GPUInfo::kPartial) {
+  if (!gpu_info_.finalized && !collecting_dx_diagnostics_) {
     // Prevent concurrent collection of DirectX diagnostics.
-    gpu_info_.level = GPUInfo::kCompleting;
+    collecting_dx_diagnostics_ = true;
 
     // Asynchronously collect the DirectX diagnostics because this can take a
     // couple of seconds.
@@ -262,7 +259,8 @@ void GpuThread::OnCollectGraphicsInfo(GPUInfo::Level level) {
 
       // Flag GPU info as complete if the DirectX diagnostics cannot be
       // collected.
-      gpu_info_.level = GPUInfo::kComplete;
+      collecting_dx_diagnostics_ = false;
+      gpu_info_.finalized = true;
     } else {
       // Do not send response if we are still completing the GPUInfo struct
       return;
@@ -340,7 +338,8 @@ void GpuThread::CollectDxDiagnostics(GpuThread* thread) {
 // Runs on the GPU thread.
 void GpuThread::SetDxDiagnostics(GpuThread* thread, const DxDiagNode& node) {
   thread->gpu_info_.dx_diagnostics = node;
-  thread->gpu_info_.level = GPUInfo::kComplete;
+  thread->gpu_info_.finalized = true;
+  thread->collecting_dx_diagnostics_ = false;
   thread->Send(new GpuHostMsg_GraphicsInfoCollected(thread->gpu_info_));
 }
 
diff --git a/content/gpu/gpu_thread.h b/content/gpu/gpu_thread.h
index 21ad923..4a0c348 100644
--- a/content/gpu/gpu_thread.h
+++ b/content/gpu/gpu_thread.h
@@ -58,7 +58,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();
   void OnCreateViewCommandBuffer(
       gfx::PluginWindowHandle window,
       int32 render_view_id,
@@ -89,6 +89,9 @@ class GpuThread : public ChildThread {
 #if defined(OS_WIN)
   // Windows specific client sandbox interface.
   sandbox::TargetServices* target_services_;
+
+  // Indicates whether DirectX Diagnostics collection is ongoing.
+  bool collecting_dx_diagnostics_;
 #endif
 
   DISALLOW_COPY_AND_ASSIGN(GpuThread);