Plugins: Add UKM Metrics to Flash loads

chrome/browser/plugins/plugin_info_message_filter.h

Index: chrome/browser/plugins/plugin_info_message_filter.h
diff --git a/chrome/browser/plugins/plugin_info_message_filter.h b/chrome/browser/plugins/plugin_info_message_filter.h
index 67a851f5db54bd3c7af6d0dba6a2df4b4e7aa34e..42ab27d7400a94b3b3d532530ea79c2ef48adf44 100644
--- a/chrome/browser/plugins/plugin_info_message_filter.h
+++ b/chrome/browser/plugins/plugin_info_message_filter.h
@@ -14,6 +14,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/memory/weak_ptr.h"
 #include "base/sequenced_task_runner_helpers.h"
+#include "base/strings/string_piece.h"
 #include "chrome/browser/plugins/plugin_metadata.h"
 #include "chrome/browser/plugins/plugin_prefs.h"
 #include "components/content_settings/core/common/content_settings.h"
@@ -62,6 +63,8 @@ class PluginInfoMessageFilter : public content::BrowserMessageFilter {
 
     ~Context();
 
+    int render_process_id() { return render_process_id_; }
+
     void DecidePluginStatus(
         const GURL& url,
         const url::Origin& main_frame_origin,
@@ -150,11 +153,23 @@ class PluginInfoMessageFilter : public content::BrowserMessageFilter {
       std::vector<base::string16>* additional_param_values);
 #endif
 
+  // Reports usage metrics to RAPPOR and UKM. This is static because metrics
+  // report on the main thread, and cross-thread weak pointers are problematic.

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

"Problematic" sounds like a Haunted Graveyard
(https://www.usenix.org/sites/default/files/conference/protected-files/srecon1...)
:) In general there are some operations that aren't allowed with cross-thread
weak pointers, but in this particular instance the filter is refcounted _and_
uses weak pointers, and invalidates the weak pointers if the refcount drops to
zero, so either using refcounting or weak pointers would work. The profile might
not exist anymore when the callback runs, but in that case we would get a null
RenderFrameHost and WebContents, which we already check for, and anything else
is coming from the browser process.

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

Done. I couldn't use weak pointers because the weak ptr factory is only for the
IO thread. I ended up using ref counting though. thanks!

[Bernhard Bauer, 2017/03/31 09:47:26]

Hm, I just realized that the WeakPtrFactory could be invalidated on the UI
thread if that is where the last reference is dropped, which would DCHECK (as
invalidation needs to happen on the sequence it's bound to). I'll need to fix
that separately though. Thanks for the rubber-duck debugging! :)

+  static void ReportMetrics(int render_process_id,

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

Can you add a comment to explain why this needs to be in the class?

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

Done.

+                            int render_frame_id,
+                            const base::StringPiece& mime_type,
+                            const GURL& url,
+                            const url::Origin& main_frame_origin,
+                            int32_t ukm_source_id);
+
   Context context_;
   std::unique_ptr<KeyedServiceShutdownNotifier::Subscription>
       shutdown_notifier_;
 
   scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner_;
+
+  const int32_t ukm_source_id_;
+
   base::WeakPtrFactory<PluginInfoMessageFilter> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(PluginInfoMessageFilter);