Report PPAPI plugin load error code to UMA.

content/ppapi_plugin/ppapi_thread.cc

Index: content/ppapi_plugin/ppapi_thread.cc
diff --git a/content/ppapi_plugin/ppapi_thread.cc b/content/ppapi_plugin/ppapi_thread.cc
index ea7431378aa124a1247a55ab2f098f31cbb209c3..689120858e4326c918fa4dd8b946178cae8a8682 100644
--- a/content/ppapi_plugin/ppapi_thread.cc
+++ b/content/ppapi_plugin/ppapi_thread.cc
@@ -11,6 +11,7 @@
 #include "base/debug/crash_logging.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
+#include "base/metrics/sparse_histogram.h"
 #include "base/rand_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
@@ -269,12 +270,15 @@ void PpapiThread::OnLoadPlugin(const base::FilePath& path,
   base::ScopedNativeLibrary library;
   if (plugin_entry_points_.initialize_module == NULL) {
     // Load the plugin from the specified library.
-    std::string error;
-    library.Reset(base::LoadNativeLibrary(path, &error));
+    std::string error_message;
+    uint32 error_code = 0;
+    library.Reset(base::LoadNativeLibrary(path, &error_message, &error_code));
     if (!library.is_valid()) {
-      LOG(ERROR) << "Failed to load Pepper module from "
-        << path.value() << " (error: " << error << ")";
+      LOG(ERROR) << "Failed to load Pepper module from " << path.value()
+                 << " (error: " << error_code << " - " << error_message << ")";
       ReportLoadResult(path, LOAD_FAILED);
+      // Report detailed reason for load failure.
+      ReportLoadErrorCode(path, error_code);
       return;
     }
 
@@ -519,4 +523,15 @@ void PpapiThread::ReportLoadResult(const base::FilePath& path,
   histogram->Add(result);
 }
 
+void PpapiThread::ReportLoadErrorCode(const base::FilePath& path,
+                                      uint32 error_code) {
+  std::ostringstream histogram_name;

[jar (doing other things), 2014/03/21 17:21:56]

nit: I think that cleaner (probably faster) and very readable to just use a
std::string, and then use the concatenation operator "+" to pull the pieces
together.

If you really cared about efficiency, you could give a size to the std::string
constructor.

[xhwang, 2014/03/21 19:00:54]

Done.

+  histogram_name << "Plugin.Ppapi" << (is_broker_ ? "Broker" : "Plugin")
+                 << "LoadErrorCode_" << path.BaseName().MaybeAsASCII();
+
+  // For sparse histograms, we can use the macro, as it does not incorporate a
+  // static.
+  UMA_HISTOGRAM_SPARSE_SLOWLY(histogram_name.str(), error_code);

[jar (doing other things), 2014/03/21 17:21:56]

nit: Don't turn this into a str().  Just pass the std::string.

You are correct that this macro avoids the use of a static, but is best used
when the range is very wide and unpredictable i.e., not for a simple enum in the
range of 1-4).

Assuming this is really a broad error code (I think you get it from Windows as a
GetLastError())... then this is a perfect use of this macro.

[xhwang, 2014/03/21 19:00:54]

Done.

+}
+
 }  // namespace content