Make MediaInternals handle MediaLogEvents.

chrome/browser/media/media_internals.cc

Index: chrome/browser/media/media_internals.cc
diff --git a/chrome/browser/media/media_internals.cc b/chrome/browser/media/media_internals.cc
index a563db36f399cb951e392e9e7aec3ae7cd0589f9..51579a43f5aff398a3c024cdead3b726dc0cc03d 100644
--- a/chrome/browser/media/media_internals.cc
+++ b/chrome/browser/media/media_internals.cc
@@ -10,11 +10,14 @@
 #include "chrome/browser/media/media_internals_observer.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/webui/web_ui.h"
+#include "media/base/media_log.h"
 #include "media/base/media_log_event.h"
 
 // The names of the javascript functions to call with updates.
-static const char kDeleteItemFunction[] = "media.onItemDeleted";
 static const char kAudioUpdateFunction[] = "media.addAudioStream";
+static const char kDeleteItemFunction[] = "media.onItemDeleted";
+static const char kMediaEventFunction[] = "media.onMediaEvent";
+static const char kMediaUpdateFunction[] = "media.addMediaPlayer";
 static const char kSendEverythingFunction[] = "media.onReceiveEverything";
 
 MediaInternals::~MediaInternals() {}
@@ -50,8 +53,40 @@ void MediaInternals::OnSetAudioStreamVolume(
 void MediaInternals::OnMediaEvent(
     int render_process_id, const media::MediaLogEvent& event) {
   DCHECK(CalledOnValidThread());
-  // TODO(scottfr): Handle |event|. Record status information in data_ and pass
-  //                |event| along to observers.
+  std::string source= base::StringPrintf("media_players.%d.%d",

[scherkus (not reviewing), 2011/08/09 20:41:10]

space before =

[Scott Franklin, 2011/08/09 23:33:39]

Done.

+                                         render_process_id, event.id);
+
+  // Notify observers that |event| has occured.
+  DictionaryValue dict;
+  dict.SetString("source", source);
+  dict.SetString("type", media::MediaLog::EventTypeToString(event.type));
+  dict.SetDouble("time", event.time.ToDoubleT());
+  dict.Set("params", event.params.DeepCopy());
+  SendUpdate(kMediaEventFunction, &dict);
+
+  // Handle parameterless events specially, but otherwise just add parameters
+  // to this.details_ and send it off to any obsevers.

[scherkus (not reviewing), 2011/08/09 20:41:10]

this.details_ ?

[Scott Franklin, 2011/08/09 23:33:39]

Done.

+  switch (event.type) {
+    case media::MediaLogEvent::WEBMEDIAPLAYER_CREATED:
+      SetItemProperty(kMediaUpdateFunction, source, "player_state",
+                      base::Value::CreateStringValue("creating"));

[scherkus (not reviewing), 2011/08/09 20:41:10]

creating vs created?

[Scott Franklin, 2011/08/09 23:33:39]

Done.

+      break;
+    case media::MediaLogEvent::WEBMEDIAPLAYER_DESTROYED:
+      SetItemProperty(kMediaUpdateFunction, source, "player_state",
+                      base::Value::CreateStringValue("destroyed"));
+      break;
+    case media::MediaLogEvent::PLAY:
+      SetItemProperty(kMediaUpdateFunction, source, "player_state",
+                      base::Value::CreateStringValue("playing"));
+      break;
+    case media::MediaLogEvent::PAUSE:
+      SetItemProperty(kMediaUpdateFunction, source, "player_state",
+                      base::Value::CreateStringValue("paused"));
+      break;
+    default:
+      SetItemProperties(kMediaUpdateFunction, source, event.params.DeepCopy());

[scherkus (not reviewing), 2011/08/09 20:41:10]

what's the difference between kMediaEventFunction and kMediaUpdateFunctions?

It looks like we send duplicate information

couldn't this switch code technically get coded in javascript?

[Scott Franklin, 2011/08/09 23:33:39]

Yes, it could (and I meant to leave a note to that effect, but I'm not sure what
happened to it). In fact, just about all of this class could get coded in
javascript instead. this.details_ is totally unnecessary, since the data gets
duplicated in javascript anyways.

The problem is, anything not stored here is unavailable to new instances of
chrome://media-internals, so a new page would be populated by updates that it
has no context with which to handle.

This is an awkward compromise so that if you open a video, then open
chrome://media-internals, you'll still get a list of all its properties. What
you won't get is a detailed list of all the events occurring up to that point
because that would be too large to serialize to JSON and send to a renderer
every time anything changes.

I'm still trying to think of a better way for this class to operate. Recording
everything here for use in filling a newly opened tab but only sending a delta
on each update would be ideal, but that will likely result in a lot of
duplicated code.

[scherkus (not reviewing), 2011/08/10 16:00:37]

Similar to web inspector we can only log+display events only if you open
media-internals first

If it helps clean up a lot of code I'd say it's worth thinking about

[Scott Franklin, 2011/08/10 22:26:51]

Huh? web inspector shows everything.

I think JavaScript should actually be fine.

+      break;
+  }
 }
 
 void MediaInternals::AddObserver(MediaInternalsObserver* observer) {
@@ -75,7 +110,7 @@ void MediaInternals::UpdateAudioStream(
     void* host, int stream_id, const std::string& property, Value* value) {
   std::string stream = base::StringPrintf("audio_streams.%p:%d",
                                           host, stream_id);
-  UpdateItem(kAudioUpdateFunction, stream, property, value);
+  SetItemProperty(kAudioUpdateFunction, stream, property, value);
 }
 
 void MediaInternals::DeleteItem(const std::string& item) {
@@ -84,16 +119,24 @@ void MediaInternals::DeleteItem(const std::string& item) {
   SendUpdate(kDeleteItemFunction, value.get());
 }
 
-void MediaInternals::UpdateItem(
+void MediaInternals::SetItemProperty(
     const std::string& update_fn, const std::string& id,
     const std::string& property, Value* value) {
+  DictionaryValue* properties = new DictionaryValue();
+  properties->Set(property, value);
+  SetItemProperties(update_fn, id, properties);
+}
+
+void MediaInternals::SetItemProperties(
+    const std::string& update_fn, const std::string& id,
+    DictionaryValue* properties) {
   DictionaryValue* item_properties;
   if (!data_.GetDictionary(id, &item_properties)) {
     item_properties = new DictionaryValue();
     data_.Set(id, item_properties);
     item_properties->SetString("id", id);
   }
-  item_properties->Set(property, value);
+  item_properties->MergeDictionary(properties);
   SendUpdate(update_fn, item_properties);
 }