MediaStream should fire ended event when all tracks are ended

Source/WebCore/Modules/mediastream/MediaStream.cpp

 /*
  * Copyright (C) 2011 Google Inc. All rights reserved.
  * Copyright (C) 2011, 2012 Ericsson AB. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 97 matching lines @@
     : ContextDestructionObserver(context)
     , m_stopped(false)
     , m_descriptor(streamDescriptor)
     , m_scheduledEventTimer(this, &MediaStream::scheduledEventTimerFired)
 {
     m_descriptor->setClient(this);
 
     size_t numberOfAudioTracks = m_descriptor->numberOfAudioComponents();
     m_audioTracks.reserveCapacity(numberOfAudioTracks);
     for (size_t i = 0; i < numberOfAudioTracks; i++)
-        m_audioTracks.append(MediaStreamTrack::create(context, m_descriptor->audioComponent(i)));
+        m_audioTracks.append(MediaStreamTrack::create(context, m_descriptor->audioComponent(i), this));
 
     size_t numberOfVideoTracks = m_descriptor->numberOfVideoComponents();
     m_videoTracks.reserveCapacity(numberOfVideoTracks);
     for (size_t i = 0; i < numberOfVideoTracks; i++)
-        m_videoTracks.append(MediaStreamTrack::create(context, m_descriptor->videoComponent(i)));
+        m_videoTracks.append(MediaStreamTrack::create(context, m_descriptor->videoComponent(i), this));
 }
 
 MediaStream::~MediaStream()
 {
     m_descriptor->setClient(0);
+
+    for (size_t i = 0; i < m_audioTracks.size(); ++i)
+        m_audioTracks[i]->setStream(0);
+
+    for (size_t i = 0; i < m_videoTracks.size(); ++i)
+        m_videoTracks[i]->setStream(0);
 }
 
 bool MediaStream::ended() const
 {
     return m_stopped || m_descriptor->ended();
 }
 
 void MediaStream::addTrack(PassRefPtr<MediaStreamTrack> prpTrack, ExceptionCode& ec)
 {
     if (ended()) {
         ec = INVALID_STATE_ERR;
         return;
     }
 
     if (!prpTrack) {
         ec = TYPE_MISMATCH_ERR;
         return;
     }
 
     RefPtr<MediaStreamTrack> track = prpTrack;
 
     if (getTrackById(track->id()))
         return;
 
     RefPtr<MediaStreamComponent> component = MediaStreamComponent::create(m_descriptor.get(), track->component()->source());
-    RefPtr<MediaStreamTrack> newTrack = MediaStreamTrack::create(scriptExecutionContext(), component.get());
+    RefPtr<MediaStreamTrack> newTrack = MediaStreamTrack::create(scriptExecutionContext(), component.get(), this);
 
     switch (component->source()->type()) {
     case MediaStreamSource::TypeAudio:
         m_descriptor->addAudioComponent(component.release());
         m_audioTracks.append(newTrack);
         break;
     case MediaStreamSource::TypeVideo:
         m_descriptor->addVideoComponent(component.release());
         m_videoTracks.append(newTrack);
         break;
@@ skipping 13 matching lines @@
         ec = TYPE_MISMATCH_ERR;
         return;
     }
 
     RefPtr<MediaStreamTrack> track = prpTrack;
 
     switch (track->component()->source()->type()) {
     case MediaStreamSource::TypeAudio: {
         size_t pos = m_audioTracks.find(track);
         if (pos != notFound) {
             m_audioTracks.remove(pos);

[abarth-chromium, 2013/04/08 16:58:41]

Will |track| have a dangling reference to this MediaStream object?

The approach you've taken in this patch to clearing m_stream is very fragile. 
I'd recommend creating an object that wraps Vector that knows how to clear the
m_stream when something is removed.

[Li Yin, 2013/04/09 03:11:54]

Thanks for your reminder.
If we implement a wrap object for Vector, it seems we still need a public
function to return Vector, because MediaStreamTrackVector is exposed to JS.

             m_descriptor->removeAudioComponent(track->component());
         }
         break;
     }
     case MediaStreamSource::TypeVideo: {
         size_t pos = m_videoTracks.find(track);
         if (pos != notFound) {
             m_videoTracks.remove(pos);
             m_descriptor->removeVideoComponent(track->component());
         }
@@ skipping 58 matching lines @@
 }
 
 void MediaStream::addRemoteTrack(MediaStreamComponent* component)
 {
     ASSERT(component && !component->stream());
     if (ended())
         return;
 
     component->setStream(descriptor());
 
-    RefPtr<MediaStreamTrack> track = MediaStreamTrack::create(scriptExecutionContext(), component);
+    RefPtr<MediaStreamTrack> track = MediaStreamTrack::create(scriptExecutionContext(), component, this);
     switch (component->source()->type()) {
     case MediaStreamSource::TypeAudio:
         m_audioTracks.append(track);
         break;
     case MediaStreamSource::TypeVideo:
         m_videoTracks.append(track);
         break;
     }
 
     scheduleDispatchEvent(MediaStreamTrackEvent::create(eventNames().addtrackEvent, false, false, track));
@@ skipping 18 matching lines @@
     for (size_t i = 0; i < tracks->size(); ++i) {
         if ((*tracks)[i]->component() == component) {
             index = i;
             break;
         }
     }
     if (index == notFound)
         return;
 
     RefPtr<MediaStreamTrack> track = (*tracks)[index];
     tracks->remove(index);

[abarth-chromium, 2013/04/08 16:58:41]

Here's another remove that might leave a dangling reference.

     scheduleDispatchEvent(MediaStreamTrackEvent::create(eventNames().removetrackEvent, false, false, track));
 }
 
 void MediaStream::scheduleDispatchEvent(PassRefPtr<Event> event)
 {
     m_scheduledEvents.append(event);
 
     if (!m_scheduledEventTimer.isActive())
         m_scheduledEventTimer.startOneShot(0);
 }
 
 void MediaStream::scheduledEventTimerFired(Timer<MediaStream>*)
 {
     if (m_stopped)
         return;
 
     Vector<RefPtr<Event> > events;
     events.swap(m_scheduledEvents);
 
     Vector<RefPtr<Event> >::iterator it = events.begin();
     for (; it != events.end(); ++it)
         dispatchEvent((*it).release());
 
     events.clear();
 }
 
+void MediaStream::trackEndedNotification()
+{
+    for (size_t i = 0; i < m_audioTracks.size(); ++i)
+        if (!m_audioTracks[i]->ended())
+            return;
+
+    for (size_t i = 0; i < m_videoTracks.size(); ++i)
+        if (!m_videoTracks[i]->ended())
+            return;
+
+    streamEnded();
+}
 } // namespace WebCore
 
 #endif // ENABLE(MEDIA_STREAM)