Add UMA reporting on failure to load ppapi plugins.

The UMA histogram name is appended with library names. Since we only have limited number of ppapi plugins this shouldn't be an issue for the server to handle.

BUG=226107
TEST=Tested Widevine CDM loading.

Review URL: https://codereview.chromium.org/13548005

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

2 files changed, 44 insertions, 1 deletions

diff --git a/content/ppapi_plugin/ppapi_thread.cc b/content/ppapi_plugin/ppapi_thread.cc
index 0f718af..006f28c 100644
--- a/content/ppapi_plugin/ppapi_thread.cc
+++ b/content/ppapi_plugin/ppapi_thread.cc
@@ -9,6 +9,7 @@
 #include "base/command_line.h"
 #include "base/debug/crash_logging.h"
 #include "base/logging.h"
+#include "base/metrics/histogram.h"
 #include "base/process_util.h"
 #include "base/rand_util.h"
 #include "base/stringprintf.h"
@@ -259,6 +260,7 @@ void PpapiThread::OnLoadPlugin(const base::FilePath& path,
     if (!library.is_valid()) {
       LOG(ERROR) << "Failed to load Pepper module from "
         << path.value() << " (error: " << error << ")";
+      ReportLoadResult(path, LOAD_FAILED);
       return;
     }
 
@@ -268,6 +270,7 @@ void PpapiThread::OnLoadPlugin(const base::FilePath& path,
             library.GetFunctionPointer("PPP_GetInterface"));
     if (!plugin_entry_points_.get_interface) {
       LOG(WARNING) << "No PPP_GetInterface in plugin library";
+      ReportLoadResult(path, ENTRY_POINT_MISSING);
       return;
     }
 
@@ -286,6 +289,7 @@ void PpapiThread::OnLoadPlugin(const base::FilePath& path,
               library.GetFunctionPointer("PPP_InitializeModule"));
       if (!plugin_entry_points_.initialize_module) {
         LOG(WARNING) << "No PPP_InitializeModule in plugin library";
+        ReportLoadResult(path, ENTRY_POINT_MISSING);
         return;
       }
     }
@@ -323,16 +327,19 @@ void PpapiThread::OnLoadPlugin(const base::FilePath& path,
             library.GetFunctionPointer("PPP_InitializeBroker"));
     if (!init_broker) {
       LOG(WARNING) << "No PPP_InitializeBroker in plugin library";
+      ReportLoadResult(path, ENTRY_POINT_MISSING);
       return;
     }
 
     int32_t init_error = init_broker(&connect_instance_func_);
     if (init_error != PP_OK) {
       LOG(WARNING) << "InitBroker failed with error " << init_error;
+      ReportLoadResult(path, INIT_FAILED);
       return;
     }
     if (!connect_instance_func_) {
       LOG(WARNING) << "InitBroker did not provide PP_ConnectInstance_Func";
+      ReportLoadResult(path, INIT_FAILED);
       return;
     }
   } else {
@@ -348,12 +355,15 @@ void PpapiThread::OnLoadPlugin(const base::FilePath& path,
         &ppapi::proxy::PluginDispatcher::GetBrowserInterface);
     if (init_error != PP_OK) {
       LOG(WARNING) << "InitModule failed with error " << init_error;
+      ReportLoadResult(path, INIT_FAILED);
       return;
     }
   }
 
   // Initialization succeeded, so keep the plugin DLL loaded.
   library_.Reset(library.Release());
+
+  ReportLoadResult(path, LOAD_SUCCESS);
 }
 
 void PpapiThread::OnCreateChannel(base::ProcessId renderer_pid,
@@ -459,7 +469,7 @@ void PpapiThread::SavePluginName(const base::FilePath& path) {
   ppapi::proxy::PluginGlobals::Get()->set_plugin_name(
       path.BaseName().AsUTF8Unsafe());
 
-  // plugin() is NULL when in-process.  Which is fine, because this is
+  // plugin() is NULL when in-process, which is fine, because this is
   // just a hook for setting the process name.
   if (GetContentClient()->plugin()) {
     GetContentClient()->plugin()->PluginProcessStarted(
@@ -467,4 +477,24 @@ void PpapiThread::SavePluginName(const base::FilePath& path) {
   }
 }
 
+void PpapiThread::ReportLoadResult(const base::FilePath& path,
+                                   LoadResult result) {
+  DCHECK_LT(result, LOAD_RESULT_MAX);
+
+  std::ostringstream histogram_name;
+  histogram_name << "Plugin.Ppapi" << (is_broker_ ? "Broker" : "Plugin")
+                 << "LoadResult_" << path.BaseName().MaybeAsASCII();
+
+  // Note: This leaks memory, which is expected behavior.
+  base::HistogramBase* histogram =
+      base::LinearHistogram::FactoryGet(
+          histogram_name.str(),
+          1,
+          LOAD_RESULT_MAX,
+          LOAD_RESULT_MAX + 1,
+          base::HistogramBase::kUmaTargetedHistogramFlag);
+
+  histogram->Add(result);
+}
+
 }  // namespace content
diff --git a/content/ppapi_plugin/ppapi_thread.h b/content/ppapi_plugin/ppapi_thread.h
index 0e21d56..5203ce1 100644
--- a/content/ppapi_plugin/ppapi_thread.h
+++ b/content/ppapi_plugin/ppapi_thread.h
@@ -49,6 +49,17 @@ class PpapiThread : public ChildThread,
   virtual ~PpapiThread();
 
  private:
+  // Make sure the enum list in tools/histogram/histograms.xml is updated with
+  // any change in this list.
+  enum LoadResult {
+    LOAD_SUCCESS,
+    LOAD_FAILED,
+    ENTRY_POINT_MISSING,
+    INIT_FAILED,
+    // NOTE: Add new values only immediately above this line.
+    LOAD_RESULT_MAX  // Boundary value for UMA_HISTOGRAM_ENUMERATION.
+  };
+
   // This class finds the target PluginDispatcher for each message it receives
   // and forwards the message.
   class DispatcherMessageListener : public IPC::Listener {
@@ -113,6 +124,8 @@ class PpapiThread : public ChildThread,
   // Sets up the name of the plugin for logging using the given path.
   void SavePluginName(const base::FilePath& path);
 
+  void ReportLoadResult(const base::FilePath& path, LoadResult result);
+
   // True if running in a broker process rather than a normal plugin process.
   bool is_broker_;