Fix DCHECK getting hit on destruction of GpuJpegDecodeAcceleratorHost

media/gpu/ipc/client/gpu_jpeg_decode_accelerator_host.cc

Index: media/gpu/ipc/client/gpu_jpeg_decode_accelerator_host.cc
diff --git a/media/gpu/ipc/client/gpu_jpeg_decode_accelerator_host.cc b/media/gpu/ipc/client/gpu_jpeg_decode_accelerator_host.cc
index fca7a5b7741e500bcc18d6ebda152630076e6747..7b815772b47ac303710189dc57270e969232b84e 100644
--- a/media/gpu/ipc/client/gpu_jpeg_decode_accelerator_host.cc
+++ b/media/gpu/ipc/client/gpu_jpeg_decode_accelerator_host.cc
@@ -31,15 +31,20 @@ class GpuJpegDecodeAcceleratorHost::Receiver : public IPC::Listener,
            const scoped_refptr<base::SingleThreadTaskRunner>& io_task_runner)
       : client_(client),
         io_task_runner_(io_task_runner),
-        weak_factory_for_io_(this) {
+        weak_factory_for_io_(
+            base::MakeUnique<base::WeakPtrFactory<Receiver>>(this)) {

[sandersd (OOO until July 31), 2017/03/30 20:58:50]

I recommend creating a WeakPtr here in the constructor, and handing out copies
of it from AsWeakPtrForIO().

[chfremer, 2017/03/30 21:19:45]

Done.

     DCHECK(CalledOnValidThread());
   }
 
-  ~Receiver() override { DCHECK(CalledOnValidThread()); }
+  ~Receiver() override {
+    DCHECK(CalledOnValidThread());
+    if (weak_factory_for_io_->HasWeakPtrs())

[sandersd (OOO until July 31), 2017/03/30 20:58:49]

I recommend always posting this task, it would be more clear if this factory was
only ever destructed on the IO thread.

Might be worth a comment that this leaks if the IO thread is stopped, but that's
not something that's usually commented on. (Just possibly relevant in this case
since there has been a bug.)

[chfremer, 2017/03/30 21:19:45]

Done.

+      io_task_runner_->DeleteSoon(FROM_HERE, weak_factory_for_io_.release());
+  }
 
-  void InvalidateWeakPtr(base::WaitableEvent* event) {
+  void InvalidateWeakPtrOnIOThread(base::WaitableEvent* event) {
     DCHECK(io_task_runner_->BelongsToCurrentThread());
-    weak_factory_for_io_.InvalidateWeakPtrs();
+    weak_factory_for_io_->InvalidateWeakPtrs();
     event->Signal();
   }
 
@@ -63,7 +68,7 @@ class GpuJpegDecodeAcceleratorHost::Receiver : public IPC::Listener,
   }
 
   base::WeakPtr<IPC::Listener> AsWeakPtrForIO() {
-    return weak_factory_for_io_.GetWeakPtr();
+    return weak_factory_for_io_->GetWeakPtr();
   }
 
  private:
@@ -91,7 +96,7 @@ class GpuJpegDecodeAcceleratorHost::Receiver : public IPC::Listener,
   scoped_refptr<base::SingleThreadTaskRunner> io_task_runner_;
 
   // Weak pointers will be invalidated on IO thread.
-  base::WeakPtrFactory<Receiver> weak_factory_for_io_;
+  std::unique_ptr<base::WeakPtrFactory<Receiver>> weak_factory_for_io_;
 
   DISALLOW_COPY_AND_ASSIGN(Receiver);
 };
@@ -119,7 +124,7 @@ GpuJpegDecodeAcceleratorHost::~GpuJpegDecodeAcceleratorHost() {
     base::WaitableEvent event(base::WaitableEvent::ResetPolicy::AUTOMATIC,
                               base::WaitableEvent::InitialState::NOT_SIGNALED);
     bool task_expected_to_run = io_task_runner_->PostTask(
-        FROM_HERE, base::Bind(&Receiver::InvalidateWeakPtr,
+        FROM_HERE, base::Bind(&Receiver::InvalidateWeakPtrOnIOThread,
                               base::Unretained(receiver_.get()),

[sandersd (OOO until July 31), 2017/03/30 20:58:50]

I'd add a comment here explaining that Unretained is safe because we always wait
until the method finishes executing if there is any chance the task actually
executes.

[chfremer, 2017/03/30 21:19:45]

Done.

                               base::Unretained(&event)));
     // If the current call is happening during the browser shutdown, the