Ensure FFmpegDemuxer WeakPtrs are created on the right thread.

media/filters/ffmpeg_demuxer.cc

Index: media/filters/ffmpeg_demuxer.cc
diff --git a/media/filters/ffmpeg_demuxer.cc b/media/filters/ffmpeg_demuxer.cc
index 5946874b763b08b0aca547586d12f1bc701ea8a6..f8d80d53e973b5b6f9c4f19f34be73124fbeeee2 100644
--- a/media/filters/ffmpeg_demuxer.cc
+++ b/media/filters/ffmpeg_demuxer.cc
@@ -860,6 +860,7 @@ void FFmpegDemuxer::Initialize(DemuxerHost* host,
   DCHECK(task_runner_->BelongsToCurrentThread());
   host_ = host;
   text_enabled_ = enable_text_tracks;
+  weak_this_ = weak_factory_.GetWeakPtr();
 
   url_protocol_.reset(new BlockingUrlProtocol(
       data_source_,
@@ -907,6 +908,7 @@ void FFmpegDemuxer::AbortPendingReads() {
   // It's important to invalidate read/seek completion callbacks to avoid any
   // errors that occur because of the data source abort.
   weak_factory_.InvalidateWeakPtrs();
+  weak_this_ = weak_factory_.GetWeakPtr();
   data_source_->Abort();
 
   // Aborting the read may cause EOF to be marked, undo this.
@@ -952,12 +954,15 @@ void FFmpegDemuxer::Stop() {
 void FFmpegDemuxer::StartWaitingForSeek(base::TimeDelta seek_time) {}
 
 void FFmpegDemuxer::CancelPendingSeek(base::TimeDelta seek_time) {
+  if (!blocking_thread_.IsRunning())

[watk, 2016/09/02 00:21:20]

Pretty sure this isn't thread safe?

[DaleCurtis, 2016/09/02 00:32:47]

Why do you think not? IsRunning() is thread safe. Removed in any case since it's
unnecessary.

[watk, 2016/09/02 20:07:09]

Ah, just because the docs in base::Thread say its not threadsafe unless
otherwise noted.

+    return;
+
   if (task_runner_->BelongsToCurrentThread()) {
     AbortPendingReads();
   } else {
-    task_runner_->PostTask(FROM_HERE,
-                           base::Bind(&FFmpegDemuxer::AbortPendingReads,
-                                      weak_factory_.GetWeakPtr()));
+    // Don't use GetWeakPtr() here since we are on the wrong thread.
+    task_runner_->PostTask(
+        FROM_HERE, base::Bind(&FFmpegDemuxer::AbortPendingReads, weak_this_));

[watk, 2016/09/02 00:21:20]

Can you tell me why this pattern is safe? I see it a lot but I don't get it.
Can't we be writing to weak_this_ on another thread (l.911) while calling its
copy constructor here? 

[DaleCurtis, 2016/09/02 00:32:47]

This particular pattern is wrong since we invalidate the WeakPtrs in two places.
I've fixed this by adding a second WeakPtr factory just for cancel pending seek.
It uses the pattern I think you're referring to now. Where we bind the WeakPtr
on the right thread and pass it from the wrong thread.

[watk, 2016/09/02 20:07:10]

Oh ok, I was confused, thanks :)

   }
 }