Add PipelineStatus UMA logging to media_internals

content/browser/media/media_internals.h

Index: content/browser/media/media_internals.h
diff --git a/content/browser/media/media_internals.h b/content/browser/media/media_internals.h
index 4c1b2ed2ed6452e3554a12c3fc94d08c66a0ddc9..2368cf8c8df4088edac4fe68596582b752950991 100644
--- a/content/browser/media/media_internals.h
+++ b/content/browser/media/media_internals.h
@@ -15,7 +15,10 @@
 #include "base/synchronization/lock.h"
 #include "base/values.h"
 #include "content/common/content_export.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
 #include "media/audio/audio_logging.h"
+#include "media/base/media_log.h"
 #include "media/video/capture/video_capture_device_info.h"
 
 namespace media {
@@ -27,8 +30,14 @@ namespace content {
 
 // This class stores information about currently active media.
 class CONTENT_EXPORT MediaInternals
-    : NON_EXPORTED_BASE(public media::AudioLogFactory) {
+    : NON_EXPORTED_BASE(public media::AudioLogFactory),
+      public NotificationObserver {

[DaleCurtis, 2014/11/06 23:49:08]

Instead of adding this to the main MediaInternals class, you should just add a
private inner class to the CC file that MediaInternals can own; just like
AudioLogImpl is done.

[prabhur1, 2014/11/07 23:43:23]

Done.

  public:
+  // NotificationObserver implementation.
+  void Observe(int type,
+               const NotificationSource& source,
+               const NotificationDetails& details) override;
+
   // Called with the update string.
   typedef base::Callback<void(const base::string16&)> UpdateCallback;
 
@@ -58,13 +67,47 @@ class CONTENT_EXPORT MediaInternals
   // AudioLogFactory implementation.  Safe to call from any thread.
   scoped_ptr<media::AudioLog> CreateAudioLog(AudioComponent component) override;
 
+  void LogAndClearPlayersInRenderer(int render_process_id);
+
  private:
+  struct PipelineInfo {
+    media::PipelineStatus last_pipeline_status;

[DaleCurtis, 2014/11/06 23:49:08]

If you take my advice above this whole thing should move into the .cc file.  You
should move all the initialization into constructor parameters.  See the syntax
on MediaInternals().

[prabhur1, 2014/11/07 23:43:24]

Done.

+    bool has_audio;
+    bool has_video;
+    std::string audio_codec_name;
+    std::string video_codec_name;
+    std::string video_decoder;
+    PipelineInfo() {
+      has_audio = false;
+      has_video = false;
+      audio_codec_name = "";

[DaleCurtis, 2014/11/06 23:49:08]

Not necessary to initialize the strings.

[prabhur1, 2014/11/07 23:43:23]

Done.

+      video_codec_name = "";
+      video_decoder = "";
+      last_pipeline_status = media::PIPELINE_OK;
+    }
+  };
+
+  // Key is playerid
+  typedef std::map<int, PipelineInfo*> PlayerInfoMap;

[DaleCurtis, 2014/11/06 23:49:08]

Don't use a pointer, no need since you're providing a default constructor...

[prabhur1, 2014/11/07 23:43:23]

Done.

+
+  // Key is renderer id
+  typedef std::map<int, PlayerInfoMap> RendererPlayerMap;
+
+  // Stores player information per renderer
+  RendererPlayerMap renderer_info;
+
+  NotificationRegistrar registrar_;
+
   friend class AudioLogImpl;
   friend class MediaInternalsTest;
   friend struct base::DefaultLazyInstanceTraits<MediaInternals>;
 
   MediaInternals();
 
+  void SavePlayerState(const media::MediaLogEvent& event,
+                       int render_process_id);
+  void LogUMAForPipelineStatus(const PipelineInfo& player_info);

[DaleCurtis, 2014/11/06 23:49:08]

s/Log/Report/

[prabhur1, 2014/11/07 23:43:23]

Done.

+
   // Sends |update| to each registered UpdateCallback.  Safe to call from any
   // thread, but will forward to the IO thread.
   void SendUpdate(const base::string16& update);
@@ -92,6 +135,6 @@ class CONTENT_EXPORT MediaInternals
   DISALLOW_COPY_AND_ASSIGN(MediaInternals);
 };
 
-} // namespace content
+}  // namespace content
 
 #endif  // CONTENT_BROWSER_MEDIA_MEDIA_INTERNALS_H_