MediaRecorder: make MediaRecorderHandler a MediaStreamObserver

content/renderer/media/media_stream.cc

Index: content/renderer/media/media_stream.cc
diff --git a/content/renderer/media/media_stream.cc b/content/renderer/media/media_stream.cc
index 27e61b914ed3f999ec6659c1f29a2e2d7acaebdd..30138dba3cdca8d16c398937e5db36db48344ff9 100644
--- a/content/renderer/media/media_stream.cc
+++ b/content/renderer/media/media_stream.cc
@@ -7,13 +7,12 @@
 #include <algorithm>
 
 #include "base/logging.h"
-#include "third_party/WebKit/public/platform/WebString.h"
 
 namespace content {
 
 // static
-MediaStream* MediaStream::GetMediaStream(
-    const blink::WebMediaStream& stream) {
+MediaStream* MediaStream::GetMediaStream(const blink::WebMediaStream& stream) {
+  DCHECK(stream.extraData());
   return static_cast<MediaStream*>(stream.extraData());
 }
 
@@ -21,7 +20,7 @@ MediaStream::MediaStream() {
 }
 
 MediaStream::~MediaStream() {
-  DCHECK(observers_.empty());
+  observers_.clear();

[perkj_chrome, 2016/01/07 09:18:00]

Why this change? Is there a case when the observers can outlive the MediaStream?
It that case, how do they know that they can not call RemoveObserver ? 

[mcasas, 2016/01/08 00:14:12]

Yes, MediaRecorder is destroyed independently from the MediaStream().
Because this is the destructor of the class, that is accessed
via l.14
MediaStream* MediaStream::GetMediaStream(...)
once the dtor has been called, noone should try to
de-register as observer, right?

Correct me if I'm wrong, but this DCHECK was there to
debug some strange race condition appeared on garbage
collection between the only current observer, WebRtc,
and this MediaStream. But honestly, it does not make
sense that MediaStream waits for the Observers to
deregister before going down, specially taken into 
account the complete lack of guarantee from Blink on
the destruction order.

Note that I brought this up in the CL description.

[perkj_chrome, 2016/01/08 07:54:50]

Right - and hence the DCHECK that no observer is still valid. Because that could
lead to a heap use after free if the observer call RemoveObserver after the
dtor.

A content::MediaStream will never be destroyed as long as the blink MediaStream
exist. And the blink MediaStream exists as long has there is a reference to it.

So the DCHECK(observers_.empty()) is correct I would say.

 }
 
 void MediaStream::AddObserver(MediaStreamObserver* observer) {