[Extensions] Record the extension function names which send a bad message.

BUG=462026
R=yoz@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#318830}

5 files changed, 42 insertions, 26 deletions

diff --git a/extensions/browser/extension_function.cc b/extensions/browser/extension_function.cc
index b970fe3..6a136b2c 100644
--- a/extensions/browser/extension_function.cc
+++ b/extensions/browser/extension_function.cc
@@ -404,7 +404,7 @@ void ExtensionFunction::SendResponseImpl(bool success) {
   if (!results_)
     results_.reset(new base::ListValue());
 
-  response_callback_.Run(type, *results_, GetError());
+  response_callback_.Run(type, *results_, GetError(), histogram_value());
 }
 
 void ExtensionFunction::OnRespondingLater(ResponseValue value) {
diff --git a/extensions/browser/extension_function.h b/extensions/browser/extension_function.h
index 9385719..f9a2a79d 100644
--- a/extensions/browser/extension_function.h
+++ b/extensions/browser/extension_function.h
@@ -100,9 +100,11 @@ class ExtensionFunction
     BAD_MESSAGE
   };
 
-  typedef base::Callback<void(ResponseType type,
-                              const base::ListValue& results,
-                              const std::string& error)> ResponseCallback;
+  using ResponseCallback = base::Callback<void(
+      ResponseType type,
+      const base::ListValue& results,
+      const std::string& error,
+      extensions::functions::HistogramValue histogram_value)>;
 
   ExtensionFunction();
 
diff --git a/extensions/browser/extension_function_dispatcher.cc b/extensions/browser/extension_function_dispatcher.cc
index cbc08cc..fcf4ab4 100644
--- a/extensions/browser/extension_function_dispatcher.cc
+++ b/extensions/browser/extension_function_dispatcher.cc
@@ -9,6 +9,7 @@
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/memory/ref_counted.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/process/process.h"
 #include "base/profiler/scoped_profile.h"
@@ -81,9 +82,14 @@ struct Static {
 base::LazyInstance<Static> g_global_io_data = LAZY_INSTANCE_INITIALIZER;
 
 // Kills the specified process because it sends us a malformed message.
-void KillBadMessageSender(base::ProcessHandle process) {
+// Track the specific function's |histogram_value|, as this may indicate a bug
+// in that API's implementation on the renderer.
+void KillBadMessageSender(base::ProcessHandle process,
+                          functions::HistogramValue histogram_value) {
   NOTREACHED();
   content::RecordAction(base::UserMetricsAction("BadMessageTerminate_EFD"));
+  UMA_HISTOGRAM_ENUMERATION("Extensions.BadMessageFunctionName",
+                            histogram_value, functions::ENUM_BOUNDARY);
   if (process)
     base::KillProcess(process, content::RESULT_CODE_KILLED_BAD_MESSAGE, false);
 }
@@ -94,7 +100,8 @@ void CommonResponseCallback(IPC::Sender* ipc_sender,
                             int request_id,
                             ExtensionFunction::ResponseType type,
                             const base::ListValue& results,
-                            const std::string& error) {
+                            const std::string& error,
+                            functions::HistogramValue histogram_value) {
   DCHECK(ipc_sender);
 
   if (type == ExtensionFunction::BAD_MESSAGE) {
@@ -108,9 +115,8 @@ void CommonResponseCallback(IPC::Sender* ipc_sender,
       // In single process mode it is better if we don't suicide but just crash.
       CHECK(false);
     } else {
-      KillBadMessageSender(peer_process);
+      KillBadMessageSender(peer_process, histogram_value);
     }
-
     return;
   }
 
@@ -125,17 +131,13 @@ void IOThreadResponseCallback(
     int request_id,
     ExtensionFunction::ResponseType type,
     const base::ListValue& results,
-    const std::string& error) {
+    const std::string& error,
+    functions::HistogramValue histogram_value) {
   if (!ipc_sender.get())
     return;
 
-  CommonResponseCallback(ipc_sender.get(),
-                         routing_id,
-                         ipc_sender->PeerHandle(),
-                         request_id,
-                         type,
-                         results,
-                         error);
+  CommonResponseCallback(ipc_sender.get(), routing_id, ipc_sender->PeerHandle(),
+                         request_id, type, results, error, histogram_value);
 }
 
 }  // namespace
@@ -180,11 +182,11 @@ class ExtensionFunctionDispatcher::UIThreadResponseCallbackWrapper
   void OnExtensionFunctionCompleted(int request_id,
                                     ExtensionFunction::ResponseType type,
                                     const base::ListValue& results,
-                                    const std::string& error) {
-    CommonResponseCallback(
-        render_view_host_, render_view_host_->GetRoutingID(),
-        render_view_host_->GetProcess()->GetHandle(), request_id, type,
-        results, error);
+                                    const std::string& error,
+                                    functions::HistogramValue histogram_value) {
+    CommonResponseCallback(render_view_host_, render_view_host_->GetRoutingID(),
+                           render_view_host_->GetProcess()->GetHandle(),
+                           request_id, type, results, error, histogram_value);
   }
 
   base::WeakPtr<ExtensionFunctionDispatcher> dispatcher_;
@@ -438,7 +440,7 @@ bool ExtensionFunctionDispatcher::CheckPermissions(
     const ExtensionFunction::ResponseCallback& callback) {
   if (!function->HasPermission()) {
     LOG(ERROR) << "Permission denied for " << params.name;
-    SendAccessDenied(callback);
+    SendAccessDenied(callback, function->histogram_value());
     return false;
   }
   return true;
@@ -457,7 +459,7 @@ ExtensionFunction* ExtensionFunctionDispatcher::CreateExtensionFunction(
       ExtensionFunctionRegistry::GetInstance()->NewFunction(params.name);
   if (!function) {
     LOG(ERROR) << "Unknown Extension API - " << params.name;
-    SendAccessDenied(callback);
+    SendAccessDenied(callback, function->histogram_value());
     return NULL;
   }
 
@@ -478,10 +480,11 @@ ExtensionFunction* ExtensionFunctionDispatcher::CreateExtensionFunction(
 
 // static
 void ExtensionFunctionDispatcher::SendAccessDenied(
-    const ExtensionFunction::ResponseCallback& callback) {
+    const ExtensionFunction::ResponseCallback& callback,
+    functions::HistogramValue histogram_value) {
   base::ListValue empty_list;
   callback.Run(ExtensionFunction::FAILED, empty_list,
-               "Access to extension API denied.");
+               "Access to extension API denied.", histogram_value);
 }
 
 }  // namespace extensions
diff --git a/extensions/browser/extension_function_dispatcher.h b/extensions/browser/extension_function_dispatcher.h
index 0ae80dc..9224bbd 100644
--- a/extensions/browser/extension_function_dispatcher.h
+++ b/extensions/browser/extension_function_dispatcher.h
@@ -146,7 +146,8 @@ class ExtensionFunctionDispatcher
   // Helper to run the response callback with an access denied error. Can be
   // called on any thread.
   static void SendAccessDenied(
-      const ExtensionFunction::ResponseCallback& callback);
+      const ExtensionFunction::ResponseCallback& callback,
+      functions::HistogramValue histogram_value);
 
   void DispatchWithCallbackInternal(
       const ExtensionHostMsg_Request_Params& params,
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml
index 8453c01..469d2a6 100644
--- a/tools/metrics/histograms/histograms.xml
+++ b/tools/metrics/histograms/histograms.xml
@@ -8442,6 +8442,16 @@ Therefore, the affected-histogram name has to have at least one dot in it.
   </summary>
 </histogram>
 
+<histogram name="Extensions.BadMessageFunctionName" enum="ExtensionFunctions">
+  <owner>kalman@chromium.org</owner>
+  <summary>
+    The number of times each Extension function call sends a bad message,
+    killing the renderer. This may indicate a bug in that API's implementation
+    on the renderer. Note a similar, aggregate metric is BadMessageTerminate_EFD
+    which counts the number of bad messages that are sent overall.
+  </summary>
+</histogram>
+
 <histogram name="Extensions.CheckForExternalUpdatesTime">
   <owner>rkaplow@chromium.org</owner>
   <summary>