Plugins: Add UKM Metrics to Flash loads

chrome/browser/plugins/plugin_info_message_filter.cc

Index: chrome/browser/plugins/plugin_info_message_filter.cc
diff --git a/chrome/browser/plugins/plugin_info_message_filter.cc b/chrome/browser/plugins/plugin_info_message_filter.cc
index 1459437c232df69502121755874a06df2d5e7600..0b9088d59193cd7a494a14bce7d54ab0b410e1c0 100644
--- a/chrome/browser/plugins/plugin_info_message_filter.cc
+++ b/chrome/browser/plugins/plugin_info_message_filter.cc
@@ -39,9 +39,12 @@
 #include "components/keyed_service/content/browser_context_keyed_service_shutdown_notifier_factory.h"
 #include "components/prefs/pref_service.h"
 #include "components/rappor/rappor_service_impl.h"
+#include "components/ukm/ukm_entry_builder.h"
+#include "components/ukm/ukm_service.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/plugin_service.h"
 #include "content/public/browser/plugin_service_filter.h"
+#include "content/public/browser/render_frame_host.h"
 #include "content/public/common/content_constants.h"
 #include "extensions/features/features.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
@@ -110,36 +113,6 @@ static void SendPluginAvailabilityUMA(const std::string& mime_type,
 
 #endif  // BUILDFLAG(ENABLE_PEPPER_CDMS)
 
-// Report usage metrics for Silverlight and Flash plugin instantiations to the
-// RAPPOR service.
-void ReportMetrics(const std::string& mime_type,
-                   const GURL& url,
-                   const url::Origin& main_frame_origin) {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-
-  if (chrome::IsIncognitoSessionActive())
-    return;
-  rappor::RapporServiceImpl* rappor_service =
-      g_browser_process->rappor_service();
-  if (!rappor_service)
-    return;
-  if (main_frame_origin.unique())
-    return;
-
-  if (mime_type == content::kFlashPluginSwfMimeType ||
-      mime_type == content::kFlashPluginSplMimeType) {
-    rappor_service->RecordSampleString(
-        "Plugins.FlashOriginUrl", rappor::ETLD_PLUS_ONE_RAPPOR_TYPE,
-        net::registry_controlled_domains::GetDomainAndRegistry(
-            main_frame_origin.GetURL(),
-            net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES));
-    rappor_service->RecordSampleString(
-        "Plugins.FlashUrl", rappor::ETLD_PLUS_ONE_RAPPOR_TYPE,
-        net::registry_controlled_domains::GetDomainAndRegistry(
-            url, net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES));
-  }
-}
-
 #if BUILDFLAG(ENABLE_EXTENSIONS)
 // Returns whether a request from a plugin to load |resource| from a renderer
 // with process id |process_id| is a request for an internal resource by an app
@@ -210,6 +183,7 @@ PluginInfoMessageFilter::PluginInfoMessageFilter(int render_process_id,
     : BrowserMessageFilter(ChromeMsgStart),
       context_(render_process_id, profile),
       main_thread_task_runner_(base::ThreadTaskRunnerHandle::Get()),
+      ukm_source_id_(ukm::UkmService::GetNewSourceID()),
       weak_ptr_factory_(this) {
   shutdown_notifier_ =
       ShutdownNotifierFactory::GetInstance()->Get(profile)->Subscribe(
@@ -554,8 +528,61 @@ void PluginInfoMessageFilter::GetPluginInfoReply(
   Send(reply_msg);
   if (output->status != ChromeViewHostMsg_GetPluginInfo_Status::kNotFound) {
     main_thread_task_runner_->PostTask(
-        FROM_HERE, base::Bind(&ReportMetrics, output->actual_mime_type,
-                              params.url, params.main_frame_origin));
+        FROM_HERE,
+        base::Bind(&PluginInfoMessageFilter::ReportMetrics,
+                   context_.render_process_id(), params.render_frame_id,
+                   output->actual_mime_type, params.url,
+                   params.main_frame_origin, ukm_source_id_));
+  }
+}
+
+// static
+void PluginInfoMessageFilter::ReportMetrics(
+    int render_process_id,
+    int render_frame_id,
+    const base::StringPiece& mime_type,
+    const GURL& url,
+    const url::Origin& main_frame_origin,
+    int32_t ukm_source_id) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  if (chrome::IsIncognitoSessionActive())

[Bernhard Bauer, 2017/03/27 11:04:43]

Can we check whether the profile corresponding to the WebContents is incognito
rather than whether any incognito session is active?

[tommycli, 2017/03/28 17:20:49]

Done.

+    return;
+  rappor::RapporServiceImpl* rappor_service =
+      g_browser_process->rappor_service();
+  if (!rappor_service)
+    return;
+  if (main_frame_origin.unique())
+    return;
+
+  if (mime_type == content::kFlashPluginSwfMimeType ||

[Bernhard Bauer, 2017/03/27 11:04:43]

Return early if they mismatch? (Also for the other checks below)

[tommycli, 2017/03/28 17:20:49]

Done.

+      mime_type == content::kFlashPluginSplMimeType) {
+    rappor_service->RecordSampleString(
+        "Plugins.FlashOriginUrl", rappor::ETLD_PLUS_ONE_RAPPOR_TYPE,
+        net::registry_controlled_domains::GetDomainAndRegistry(
+            main_frame_origin.GetURL(),
+            net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES));
+    rappor_service->RecordSampleString(
+        "Plugins.FlashUrl", rappor::ETLD_PLUS_ONE_RAPPOR_TYPE,
+        net::registry_controlled_domains::GetDomainAndRegistry(
+            url, net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES));
+
+    ukm::UkmService* ukm_service = g_browser_process->ukm_service();
+    content::RenderFrameHost* frame =
+        content::RenderFrameHost::FromID(render_process_id, render_frame_id);
+    content::WebContents* web_contents =
+        content::WebContents::FromRenderFrameHost(frame);
+    if (ukm_service && web_contents) {
+      // Verify that the WebContents origin hasn't changed while bouncing to the
+      // UI thread.
+      GURL main_frame_url = web_contents->GetLastCommittedURL();
+      if (url::Origin(main_frame_url) == main_frame_origin) {
+        ukm_service->UpdateSourceURL(ukm_source_id, main_frame_url);
+        std::unique_ptr<ukm::UkmEntryBuilder> builder =
+            ukm_service->GetEntryBuilder(ukm_source_id,

[Bernhard Bauer, 2017/03/27 11:04:43]

Wait, we create a new entry builder and then immediately destroy it?

[Steven Holte, 2017/03/27 19:27:18]

That seems right, since UkmEntryBuilder records the built Entry when it falls
out of scope.  This just creates an Entry with a name and source_id, but no
Metrics.

+                                         "Plugins.FlashInstance");
+      }
+    }
   }
 }