WebmMuxer-MediaRecorderHandler: thread hopping and data ownership

media/capture/webm_muxer.h

Index: media/capture/webm_muxer.h
diff --git a/media/capture/webm_muxer.h b/media/capture/webm_muxer.h
index 3fec647fa9cc3e8aecb5a53220392e4541eb5376..b6b6c70ae3c37272f60cc0855c0b3121c5f69170 100644
--- a/media/capture/webm_muxer.h
+++ b/media/capture/webm_muxer.h
@@ -9,7 +9,9 @@
 
 #include "base/callback.h"
 #include "base/gtest_prod_util.h"
+#include "base/message_loop/message_loop.h"
 #include "base/numerics/safe_math.h"
+#include "base/single_thread_task_runner.h"
 #include "base/strings/string_piece.h"
 #include "base/threading/thread_checker.h"
 #include "base/time/time.h"
@@ -30,18 +32,23 @@ class VideoFrame;
 // contructor with the wrapped encoded data.
 // WebmMuxer is created/destroyed on a thread, usually the Main Render thread,
 // and receives OnEncodedVideo()s on another thread, usually Render IO.
+// WebmMuxer is ref counted to avoid races between dtor and eventual encoded
+// frame(s) in flight.
 // [1] http://www.webmproject.org/docs/container/
 // [2] http://www.matroska.org/technical/specs/index.html
 // TODO(mcasas): Add support for Audio muxing.
-class MEDIA_EXPORT WebmMuxer : public NON_EXPORTED_BASE(mkvmuxer::IMkvWriter) {
+class MEDIA_EXPORT WebmMuxer : public NON_EXPORTED_BASE(mkvmuxer::IMkvWriter)
+                             , public base::RefCountedThreadSafe<WebmMuxer> {
  public:
   // Callback to be called when WebmMuxer is ready to write a chunk of data,
   // either any file header or a SingleBlock. The chunk is described as a byte
   // array and a byte length.
   using WriteDataCB = base::Callback<void(const base::StringPiece&)>;
 
-  explicit WebmMuxer(const WriteDataCB& write_data_callback);
-  ~WebmMuxer() override;
+  static void FinalizeSegment(scoped_ptr<mkvmuxer::Segment> segment);

[ajose, 2015/09/17 21:35:27]

Can this be below the ctr? 

https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#...

+
+  WebmMuxer(const scoped_refptr<base::SingleThreadTaskRunner> main_task_runner,
+            const WriteDataCB& write_data_callback);
 
   // Adds a |video_frame| with |encoded_data.data()| to WebM Segment.
   // TODO(mcasas): Revisit how |encoded_data| is passed once the whole recording
@@ -52,7 +59,9 @@ class MEDIA_EXPORT WebmMuxer : public NON_EXPORTED_BASE(mkvmuxer::IMkvWriter) {
                       bool is_key_frame);
 
  private:
+  friend class base::RefCountedThreadSafe<WebmMuxer>;
   friend class WebmMuxerTest;
+  ~WebmMuxer() override;
 
   // Creates and adds a new video track. Called upon receiving the first
   // frame of a given Track, adds |frame_size| and |frame_rate| to the Segment
@@ -68,6 +77,9 @@ class MEDIA_EXPORT WebmMuxer : public NON_EXPORTED_BASE(mkvmuxer::IMkvWriter) {
   void ElementStartNotify(mkvmuxer::uint64 element_id,
                           mkvmuxer::int64 position) override;
 
+  // Used to shutdown properly on the same thread we were created.
+  const scoped_refptr<base::SingleThreadTaskRunner> main_task_runner_;
+
   // Used to DCHECK that we are called on the correct thread (usually IO)
   base::ThreadChecker thread_checker_;
 
@@ -85,7 +97,7 @@ class MEDIA_EXPORT WebmMuxer : public NON_EXPORTED_BASE(mkvmuxer::IMkvWriter) {
   base::CheckedNumeric<mkvmuxer::int64> position_;
 
   // The MkvMuxer active element.
-  mkvmuxer::Segment segment_;
+  scoped_ptr<mkvmuxer::Segment> segment_;
 
   DISALLOW_COPY_AND_ASSIGN(WebmMuxer);
 };