Media Source dispatches inband text tracks

webkit/media/webmediaplayer_impl.cc

Index: webkit/media/webmediaplayer_impl.cc
diff --git a/webkit/media/webmediaplayer_impl.cc b/webkit/media/webmediaplayer_impl.cc
index 6a95ea9ec21088633eb91ce0e3bdb8b9534cde3a..7493d09a55e2d3d55a9698b962d8971bc8e82d93 100644
--- a/webkit/media/webmediaplayer_impl.cc
+++ b/webkit/media/webmediaplayer_impl.cc
@@ -46,10 +46,12 @@
 #include "webkit/compositor_bindings/web_layer_impl.h"
 #include "webkit/media/buffered_data_source.h"
 #include "webkit/media/webaudiosourceprovider_impl.h"
+#include "webkit/media/webinbandtexttrack_impl.h"
 #include "webkit/media/webmediaplayer_delegate.h"
 #include "webkit/media/webmediaplayer_params.h"
 #include "webkit/media/webmediaplayer_util.h"
 #include "webkit/media/webmediasourceclient_impl.h"
+#include "webkit/media/webtexttrack_impl.h"
 #include "webkit/plugins/ppapi/ppapi_webplugin_impl.h"
 
 using WebKit::WebCanvas;
@@ -146,7 +148,8 @@ WebMediaPlayerImpl::WebMediaPlayerImpl(
       chunk_demuxer_(NULL),
       pending_repaint_(false),
       pending_size_change_(false),
-      video_frame_provider_client_(NULL) {
+      video_frame_provider_client_(NULL),
+      text_track_index_(0) {
   media_log_->AddEvent(
       media_log_->CreateEvent(media::MediaLogEvent::WEBMEDIAPLAYER_CREATED));
 
@@ -987,6 +990,37 @@ void WebMediaPlayerImpl::OnNeedKey(const std::string& key_system,
                          init_data_size);
 }
 
+media::TextTrack*
+WebMediaPlayerImpl::OnTextTrack(media::TextKind kind,
+                                const std::string& label,
+                                const std::string& language) {
+  typedef WebInbandTextTrackImpl::Kind webkind_t;
+  const webkind_t webkind = static_cast<webkind_t>(kind);
+  const WebKit::WebString weblabel = WebKit::WebString::fromUTF8(label);
+  const WebKit::WebString weblanguage = WebKit::WebString::fromUTF8(language);
+
+  // TODO(matthewjheaney): resolve lifetime issues for text_track_.
+  // Note that the text track object is adopted by its client, and so
+  // it does get destroyed during teardown of the tracks, and
+  // therefore there's no memory leak. However, there is the issue of
+  // lifetime, since we share a pointer to this text track object with
+  // its (owning) client.
+
+  WebInbandTextTrackImpl* const text_track =
+    new WebInbandTextTrackImpl(webkind, weblabel, weblanguage,
+                               text_track_index_++);
+  GetClient()->addTextTrack(text_track);
+
+  // TODO(matthewjheaney): better name for WebTextTrackImpl?
+  WebTextTrackImpl* const text_cb = new WebTextTrackImpl(text_track->client());

[acolwell GONE FROM CHROMIUM, 2013/05/10 02:22:08]

I think you need to be careful about ownership here. Does addTextTrack()
transfers ownership of text_track to Blink? If so then you need to be careful
about situations where Blink blows away text_track, but text_cb is still alive.

If ownership is not transfered, then text_cb should probably own text_track so
that when the TextTrack is destroyed text_track is also destroyed.

[Matthew Heaney (Chromium), 2013/05/10 05:21:08]

OK, that makes sense.  I'll make the necessary changes to webkit and create a
separate CL for that.

+
+  // TODO(matthewjheaney): need to keep record of created text tracks?
+  //text_tracks_.insert(text_cb);
+
+  // TODO(matthewjheaney): for now, rely on caller to destroy text_cb.
+  return text_cb;
+}
+
 #define COMPILE_ASSERT_MATCHING_ENUM(name) \
   COMPILE_ASSERT(static_cast<int>(WebKit::WebMediaPlayerClient::name) == \
                  static_cast<int>(media::Decryptor::k ## name), \
@@ -1082,6 +1116,8 @@ void WebMediaPlayerImpl::StartPipeline(WebKit::WebMediaSource* media_source) {
         BIND_TO_RENDER_LOOP_1(&WebMediaPlayerImpl::OnDemuxerOpened,
                               base::Passed(&ms)),
         BIND_TO_RENDER_LOOP_2(&WebMediaPlayerImpl::OnNeedKey, "", ""),
+        // TODO(matthewjheaney): need confirmation that Unretained is OK here
+        base::Bind(&WebMediaPlayerImpl::OnTextTrack, base::Unretained(this)),

[acolwell GONE FROM CHROMIUM, 2013/05/10 02:22:08]

Yes. this is fine.

[Matthew Heaney (Chromium), 2013/05/10 05:21:08]

Done.

         base::Bind(&LogMediaSourceError, media_log_));
     demuxer_.reset(chunk_demuxer_);