Add support for WeakPtr<T>

The implementation is borrowed from Chromium.

Also change use of raw pointers in VideoSendStreamImpl to use WeakPtr<>

BUG= webrtc:6289

Review-Url: https://codereview.webrtc.org/2367853002
Cr-Commit-Position: refs/heads/master@{#14468}

webrtc/BUILD.gn

@@ -439,6 +439,7 @@ if (rtc_include_tests) {
       "base/timeutils_unittest.cc",
       "base/urlencode_unittest.cc",
       "base/versionparsing_unittest.cc",
+      "base/weak_ptr_unittest.cc",
 
       # TODO(ronghuawu): Reenable this test.
       # "windowpicker_unittest.cc",

webrtc/base/BUILD.gn

@@ -210,6 +210,8 @@ rtc_static_library("rtc_task_queue") {
     "sequenced_task_checker.h",
     "sequenced_task_checker_impl.cc",
     "sequenced_task_checker_impl.h",
+    "weak_ptr.cc",
+    "weak_ptr.h",
   ]
 
   if (build_with_chromium) {

webrtc/base/base.gyp

@@ -167,6 +167,8 @@
         'sequenced_task_checker.h',
         'sequenced_task_checker_impl.cc',
         'sequenced_task_checker_impl.h',
+        'weak_ptr.cc',
+        'weak_ptr.h',
       ],
       'conditions': [
         ['build_with_chromium==1', {

webrtc/base/weak_ptr.cc

@@ -0,0 +1,87 @@
+/*
+ *  Copyright 2016 The WebRTC Project Authors. All rights reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include "webrtc/base/weak_ptr.h"
+
+// The implementation is borrowed from chromium except that it does not
+// implement SupportsWeakPtr.
+
+namespace rtc {
+namespace internal {
+
+WeakReference::Flag::Flag() : is_valid_(true) {
+  // Flags only become bound when checked for validity, or invalidated,
+  // so that we can check that later validity/invalidation operations on
+  // the same Flag take place on the same sequence.
+  checker_.Detach();
+}
+
+void WeakReference::Flag::Invalidate() {
+  RTC_DCHECK(checker_.CalledSequentially())
+      << "WeakPtrs must be invalidated on the same sequence.";
+  is_valid_ = false;
+}
+
+bool WeakReference::Flag::IsValid() const {
+  RTC_DCHECK(checker_.CalledSequentially())
+      << "WeakPtrs must be checked on the same sequence.";
+  return is_valid_;
+}
+
+WeakReference::Flag::~Flag() {}
+
+WeakReference::WeakReference() {}
+
+WeakReference::WeakReference(const Flag* flag) : flag_(flag) {}
+
+WeakReference::~WeakReference() {}
+
+WeakReference::WeakReference(WeakReference&& other) = default;
+
+WeakReference::WeakReference(const WeakReference& other) = default;
+
+bool WeakReference::is_valid() const {
+  return flag_.get() && flag_->IsValid();
+}
+
+WeakReferenceOwner::WeakReferenceOwner() {
+  checker_.Detach();
+}
+
+WeakReferenceOwner::~WeakReferenceOwner() {
+  RTC_DCHECK(checker_.CalledSequentially());
+  Invalidate();
+}
+
+WeakReference WeakReferenceOwner::GetRef() const {
+  RTC_DCHECK(checker_.CalledSequentially());
+  // If we hold the last reference to the Flag then create a new one.
+  if (!HasRefs())
+    flag_ = new RefCountedObject<WeakReference::Flag>();
+
+  return WeakReference(flag_.get());
+}
+
+void WeakReferenceOwner::Invalidate() {
+  RTC_DCHECK(checker_.CalledSequentially());
+  if (flag_.get()) {
+    flag_->Invalidate();
+    flag_ = NULL;
+  }
+}
+
+WeakPtrBase::WeakPtrBase() {}
+
+WeakPtrBase::~WeakPtrBase() {}
+
+WeakPtrBase::WeakPtrBase(const WeakReference& ref) : ref_(ref) {}
+
+}  // namespace internal
+}  // namespace rtc

webrtc/base/weak_ptr.h

@@ -0,0 +1,272 @@
+/*
+ *  Copyright 2016 The WebRTC Project Authors. All rights reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef WEBRTC_BASE_WEAK_PTR_H_
+#define WEBRTC_BASE_WEAK_PTR_H_
+
+#include <memory>
+
+#include <utility>
+
+#include "webrtc/base/refcount.h"
+#include "webrtc/base/scoped_ref_ptr.h"
+#include "webrtc/base/sequenced_task_checker.h"
+
+// The implementation is borrowed from chromium except that it does not
+// implement SupportsWeakPtr.
+
+// Weak pointers are pointers to an object that do not affect its lifetime,
+// and which may be invalidated (i.e. reset to nullptr) by the object, or its
+// owner, at any time, most commonly when the object is about to be deleted.
+
+// Weak pointers are useful when an object needs to be accessed safely by one
+// or more objects other than its owner, and those callers can cope with the
+// object vanishing and e.g. tasks posted to it being silently dropped.
+// Reference-counting such an object would complicate the ownership graph and
+// make it harder to reason about the object's lifetime.
+
+// EXAMPLE:
+//
+//  class Controller {
+//   public:
+//    Controller() : weak_factory_(this) {}
+//    void SpawnWorker() { Worker::StartNew(weak_factory_.GetWeakPtr()); }
+//    void WorkComplete(const Result& result) { ... }
+//   private:
+//    // Member variables should appear before the WeakPtrFactory, to ensure
+//    // that any WeakPtrs to Controller are invalidated before its members
+//    // variable's destructors are executed, rendering them invalid.
+//    WeakPtrFactory<Controller> weak_factory_;
+//  };
+//
+//  class Worker {
+//   public:
+//    static void StartNew(const WeakPtr<Controller>& controller) {
+//      Worker* worker = new Worker(controller);
+//      // Kick off asynchronous processing...
+//    }
+//   private:
+//    Worker(const WeakPtr<Controller>& controller)
+//        : controller_(controller) {}
+//    void DidCompleteAsynchronousProcessing(const Result& result) {
+//      if (controller_)
+//        controller_->WorkComplete(result);
+//    }
+//    WeakPtr<Controller> controller_;
+//  };
+//
+// With this implementation a caller may use SpawnWorker() to dispatch multiple
+// Workers and subsequently delete the Controller, without waiting for all
+// Workers to have completed.
+
+// ------------------------- IMPORTANT: Thread-safety -------------------------
+
+// Weak pointers may be passed safely between threads, but must always be
+// dereferenced and invalidated on the same TaskQueue or thread, otherwise
+// checking the pointer would be racey.
+//
+// To ensure correct use, the first time a WeakPtr issued by a WeakPtrFactory
+// is dereferenced, the factory and its WeakPtrs become bound to the calling
+// TaskQueue/thread, and cannot be dereferenced or
+// invalidated on any other TaskQueue/thread. Bound WeakPtrs can still be handed
+// off to other TaskQueues, e.g. to use to post tasks back to object on the
+// bound sequence.
+//
+// Thus, at least one WeakPtr object must exist and have been dereferenced on
+// the correct thread to enforce that other WeakPtr objects will enforce they
+// are used on the desired thread.
+
+namespace rtc {
+
+namespace internal {
+
+class WeakReference {
+ public:
+  // Although Flag is bound to a specific sequence, it may be
+  // deleted from another via base::WeakPtr::~WeakPtr().
+  class Flag : public RefCountInterface {
+   public:
+    Flag();
+
+    void Invalidate();
+    bool IsValid() const;
+
+   private:
+    friend class RefCountedObject<Flag>;
+
+    ~Flag() override;
+
+    SequencedTaskChecker checker_;
+    bool is_valid_;
+  };
+
+  WeakReference();
+  explicit WeakReference(const Flag* flag);
+  ~WeakReference();
+
+  WeakReference(WeakReference&& other);
+  WeakReference(const WeakReference& other);
+  WeakReference& operator=(WeakReference&& other) = default;
+  WeakReference& operator=(const WeakReference& other) = default;
+
+  bool is_valid() const;
+
+ private:
+  scoped_refptr<const Flag> flag_;
+};
+
+class WeakReferenceOwner {
+ public:
+  WeakReferenceOwner();
+  ~WeakReferenceOwner();
+
+  WeakReference GetRef() const;
+
+  bool HasRefs() const { return flag_.get() && !flag_->HasOneRef(); }
+
+  void Invalidate();
+
+ private:
+  SequencedTaskChecker checker_;
+  mutable scoped_refptr<RefCountedObject<WeakReference::Flag>> flag_;
+};
+
+// This class simplifies the implementation of WeakPtr's type conversion
+// constructor by avoiding the need for a public accessor for ref_.  A
+// WeakPtr<T> cannot access the private members of WeakPtr<U>, so this
+// base class gives us a way to access ref_ in a protected fashion.
+class WeakPtrBase {
+ public:
+  WeakPtrBase();
+  ~WeakPtrBase();
+
+  WeakPtrBase(const WeakPtrBase& other) = default;
+  WeakPtrBase(WeakPtrBase&& other) = default;
+  WeakPtrBase& operator=(const WeakPtrBase& other) = default;
+  WeakPtrBase& operator=(WeakPtrBase&& other) = default;
+
+ protected:
+  explicit WeakPtrBase(const WeakReference& ref);
+
+  WeakReference ref_;
+};
+
+}  // namespace internal
+
+template <typename T>
+class WeakPtrFactory;
+
+template <typename T>
+class WeakPtr : public internal::WeakPtrBase {
+ public:
+  WeakPtr() : ptr_(nullptr) {}
+
+  // Allow conversion from U to T provided U "is a" T. Note that this
+  // is separate from the (implicit) copy and move constructors.
+  template <typename U>
+  WeakPtr(const WeakPtr<U>& other)
+      : internal::WeakPtrBase(other), ptr_(other.ptr_) {}
+  template <typename U>
+  WeakPtr(WeakPtr<U>&& other)
+      : internal::WeakPtrBase(std::move(other)), ptr_(other.ptr_) {}
+
+  T* get() const { return ref_.is_valid() ? ptr_ : nullptr; }
+
+  T& operator*() const {
+    RTC_DCHECK(get() != nullptr);
+    return *get();
+  }
+  T* operator->() const {
+    RTC_DCHECK(get() != nullptr);
+    return get();
+  }
+
+  void reset() {
+    ref_ = internal::WeakReference();
+    ptr_ = nullptr;
+  }
+
+  // Allow conditionals to test validity, e.g. if (weak_ptr) {...};
+  explicit operator bool() const { return get() != nullptr; }
+
+ private:
+  template <typename U>
+  friend class WeakPtr;
+  friend class WeakPtrFactory<T>;
+
+  WeakPtr(const internal::WeakReference& ref, T* ptr)
+      : internal::WeakPtrBase(ref), ptr_(ptr) {}
+
+  // This pointer is only valid when ref_.is_valid() is true.  Otherwise, its
+  // value is undefined (as opposed to nullptr).
+  T* ptr_;
+};
+
+// Allow callers to compare WeakPtrs against nullptr to test validity.
+template <class T>
+bool operator!=(const WeakPtr<T>& weak_ptr, std::nullptr_t) {
+  return !(weak_ptr == nullptr);
+}
+template <class T>
+bool operator!=(std::nullptr_t, const WeakPtr<T>& weak_ptr) {
+  return weak_ptr != nullptr;
+}
+template <class T>
+bool operator==(const WeakPtr<T>& weak_ptr, std::nullptr_t) {
+  return weak_ptr.get() == nullptr;
+}
+template <class T>
+bool operator==(std::nullptr_t, const WeakPtr<T>& weak_ptr) {
+  return weak_ptr == nullptr;
+}
+
+// A class may be composed of a WeakPtrFactory and thereby
+// control how it exposes weak pointers to itself.  This is helpful if you only
+// need weak pointers within the implementation of a class.  This class is also
+// useful when working with primitive types.  For example, you could have a
+// WeakPtrFactory<bool> that is used to pass around a weak reference to a bool.
+
+// Note that GetWeakPtr must be called on one and only one TaskQueue or thread
+// and the WeakPtr must only be dereferenced and invalidated on that same
+// TaskQueue/thread. A WeakPtr instance can be copied and posted to other
+// sequences though as long as it is not dereferenced (WeakPtr<T>::get()).
+template <class T>
+class WeakPtrFactory {
+ public:
+  explicit WeakPtrFactory(T* ptr) : ptr_(ptr) {}
+
+  ~WeakPtrFactory() { ptr_ = nullptr; }
+
+  WeakPtr<T> GetWeakPtr() {
+    RTC_DCHECK(ptr_);
+    return WeakPtr<T>(weak_reference_owner_.GetRef(), ptr_);
+  }
+
+  // Call this method to invalidate all existing weak pointers.
+  void InvalidateWeakPtrs() {
+    RTC_DCHECK(ptr_);
+    weak_reference_owner_.Invalidate();
+  }
+
+  // Call this method to determine if any weak pointers exist.
+  bool HasWeakPtrs() const {
+    RTC_DCHECK(ptr_);
+    return weak_reference_owner_.HasRefs();
+  }
+
+ private:
+  internal::WeakReferenceOwner weak_reference_owner_;
+  T* ptr_;
+  RTC_DISALLOW_IMPLICIT_CONSTRUCTORS(WeakPtrFactory);
+};
+
+}  // namespace rtc
+
+#endif  // WEBRTC_BASE_WEAK_PTR_H_

webrtc/base/weak_ptr_unittest.cc

@@ -0,0 +1,233 @@
+/*
+ *  Copyright 2016 The WebRTC Project Authors. All rights reserved.
+ *
+ *  Use of this source code is governed by a BSD-style license
+ *  that can be found in the LICENSE file in the root of the source
+ *  tree. An additional intellectual property rights grant can be found
+ *  in the file PATENTS.  All contributing project authors may
+ *  be found in the AUTHORS file in the root of the source tree.
+ */
+
+#include <string>
+
+#include "webrtc/base/gunit.h"
+#include "webrtc/base/task_queue.h"
+#include "webrtc/base/weak_ptr.h"
+
+namespace rtc {
+
+namespace {
+
+struct Base {
+  std::string member;
+};
+struct Derived : public Base {};
+
+struct Target {};
+
+struct Arrow {
+  WeakPtr<Target> target;
+};
+
+struct TargetWithFactory : public Target {
+  TargetWithFactory() : factory(this) {}
+  WeakPtrFactory<Target> factory;
+};
+
+}  // namespace
+
+TEST(WeakPtrFactoryTest, Basic) {
+  int data;
+  WeakPtrFactory<int> factory(&data);
+  WeakPtr<int> ptr = factory.GetWeakPtr();
+  EXPECT_EQ(&data, ptr.get());
+}
+
+TEST(WeakPtrFactoryTest, Comparison) {
+  int data;
+  WeakPtrFactory<int> factory(&data);
+  WeakPtr<int> ptr = factory.GetWeakPtr();
+  WeakPtr<int> ptr2 = ptr;
+  EXPECT_EQ(ptr.get(), ptr2.get());
+}
+
+TEST(WeakPtrFactoryTest, Move) {
+  int data;
+  WeakPtrFactory<int> factory(&data);
+  WeakPtr<int> ptr = factory.GetWeakPtr();
+  WeakPtr<int> ptr2 = factory.GetWeakPtr();
+  WeakPtr<int> ptr3 = std::move(ptr2);
+  EXPECT_NE(ptr.get(), ptr2.get());
+  EXPECT_EQ(ptr.get(), ptr3.get());
+}
+
+TEST(WeakPtrFactoryTest, OutOfScope) {
+  WeakPtr<int> ptr;
+  EXPECT_EQ(nullptr, ptr.get());
+  {
+    int data;
+    WeakPtrFactory<int> factory(&data);
+    ptr = factory.GetWeakPtr();
+    EXPECT_EQ(&data, ptr.get());
+  }
+  EXPECT_EQ(nullptr, ptr.get());
+}
+
+TEST(WeakPtrFactoryTest, Multiple) {
+  WeakPtr<int> a, b;
+  {
+    int data;
+    WeakPtrFactory<int> factory(&data);
+    a = factory.GetWeakPtr();
+    b = factory.GetWeakPtr();
+    EXPECT_EQ(&data, a.get());
+    EXPECT_EQ(&data, b.get());
+  }
+  EXPECT_EQ(nullptr, a.get());
+  EXPECT_EQ(nullptr, b.get());
+}
+
+TEST(WeakPtrFactoryTest, MultipleStaged) {
+  WeakPtr<int> a;
+  {
+    int data;
+    WeakPtrFactory<int> factory(&data);
+    a = factory.GetWeakPtr();
+    { WeakPtr<int> b = factory.GetWeakPtr(); }
+    EXPECT_NE(nullptr, a.get());
+  }
+  EXPECT_EQ(nullptr, a.get());
+}
+
+TEST(WeakPtrFactoryTest, Dereference) {
+  Base data;
+  data.member = "123456";
+  WeakPtrFactory<Base> factory(&data);
+  WeakPtr<Base> ptr = factory.GetWeakPtr();
+  EXPECT_EQ(&data, ptr.get());
+  EXPECT_EQ(data.member, (*ptr).member);
+  EXPECT_EQ(data.member, ptr->member);
+}
+
+TEST(WeakPtrFactoryTest, UpCast) {
+  Derived data;
+  WeakPtrFactory<Derived> factory(&data);
+  WeakPtr<Base> ptr = factory.GetWeakPtr();
+  ptr = factory.GetWeakPtr();
+  EXPECT_EQ(ptr.get(), &data);
+}
+
+TEST(WeakPtrTest, DefaultConstructor) {
+  WeakPtr<int> ptr;
+  EXPECT_EQ(nullptr, ptr.get());
+}
+
+TEST(WeakPtrFactoryTest, BooleanTesting) {
+  int data;
+  WeakPtrFactory<int> factory(&data);
+
+  WeakPtr<int> ptr_to_an_instance = factory.GetWeakPtr();
+  EXPECT_TRUE(ptr_to_an_instance);
+  EXPECT_FALSE(!ptr_to_an_instance);
+
+  if (ptr_to_an_instance) {
+  } else {
+    ADD_FAILURE() << "Pointer to an instance should result in true.";
+  }
+
+  if (!ptr_to_an_instance) {  // check for operator!().
+    ADD_FAILURE() << "Pointer to an instance should result in !x being false.";
+  }
+
+  WeakPtr<int> null_ptr;
+  EXPECT_FALSE(null_ptr);
+  EXPECT_TRUE(!null_ptr);
+
+  if (null_ptr) {
+    ADD_FAILURE() << "Null pointer should result in false.";
+  }
+
+  if (!null_ptr) {  // check for operator!().
+  } else {
+    ADD_FAILURE() << "Null pointer should result in !x being true.";
+  }
+}
+
+TEST(WeakPtrFactoryTest, ComparisonToNull) {
+  int data;
+  WeakPtrFactory<int> factory(&data);
+
+  WeakPtr<int> ptr_to_an_instance = factory.GetWeakPtr();
+  EXPECT_NE(nullptr, ptr_to_an_instance);
+  EXPECT_NE(ptr_to_an_instance, nullptr);
+
+  WeakPtr<int> null_ptr;
+  EXPECT_EQ(null_ptr, nullptr);
+  EXPECT_EQ(nullptr, null_ptr);
+}
+
+TEST(WeakPtrTest, InvalidateWeakPtrs) {
+  int data;
+  WeakPtrFactory<int> factory(&data);
+  WeakPtr<int> ptr = factory.GetWeakPtr();
+  EXPECT_EQ(&data, ptr.get());
+  EXPECT_TRUE(factory.HasWeakPtrs());
+  factory.InvalidateWeakPtrs();
+  EXPECT_EQ(nullptr, ptr.get());
+  EXPECT_FALSE(factory.HasWeakPtrs());
+
+  // Test that the factory can create new weak pointers after a
+  // InvalidateWeakPtrs call, and they remain valid until the next
+  // InvalidateWeakPtrs call.
+  WeakPtr<int> ptr2 = factory.GetWeakPtr();
+  EXPECT_EQ(&data, ptr2.get());
+  EXPECT_TRUE(factory.HasWeakPtrs());
+  factory.InvalidateWeakPtrs();
+  EXPECT_EQ(nullptr, ptr2.get());
+  EXPECT_FALSE(factory.HasWeakPtrs());
+}
+
+TEST(WeakPtrTest, HasWeakPtrs) {
+  int data;
+  WeakPtrFactory<int> factory(&data);
+  {
+    WeakPtr<int> ptr = factory.GetWeakPtr();
+    EXPECT_TRUE(factory.HasWeakPtrs());
+  }
+  EXPECT_FALSE(factory.HasWeakPtrs());
+}
+
+template <class T>
+std::unique_ptr<T> NewObjectCreatedOnTaskQueue() {
+  std::unique_ptr<T> obj;
+  TaskQueue queue("NewObjectCreatedOnTaskQueue");
+  Event event(false, false);
+  queue.PostTask([&event, &obj] {
+    obj.reset(new T());
+    event.Set();
+  });
+  EXPECT_TRUE(event.Wait(1000));
+  return obj;
+}
+
+TEST(WeakPtrTest, ObjectAndWeakPtrOnDifferentThreads) {
+  // Test that it is OK to create an object with a WeakPtrFactory one thread,
+  // but use it on another.  This tests that we do not trip runtime checks that
+  // ensure that a WeakPtr is not used by multiple threads.
+  std::unique_ptr<TargetWithFactory> target(
+      NewObjectCreatedOnTaskQueue<TargetWithFactory>());
+  WeakPtr<Target> weak_ptr = target->factory.GetWeakPtr();
+  EXPECT_EQ(target.get(), weak_ptr.get());
+}
+
+TEST(WeakPtrTest, WeakPtrInitiateAndUseOnDifferentThreads) {
+  // Test that it is OK to create an object that has a WeakPtr member on one
+  // thread, but use it on another.  This tests that we do not trip runtime
+  // checks that ensure that a WeakPtr is not used by multiple threads.
+  std::unique_ptr<Arrow> arrow(NewObjectCreatedOnTaskQueue<Arrow>());
+  TargetWithFactory target;
+  arrow->target = target.factory.GetWeakPtr();
+  EXPECT_EQ(&target, arrow->target.get());
+}
+
+}  // namespace rtc

webrtc/video/video_send_stream.cc

@@ -20,6 +20,7 @@
 #include "webrtc/base/file.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/base/trace_event.h"
+#include "webrtc/base/weak_ptr.h"
 #include "webrtc/modules/bitrate_controller/include/bitrate_controller.h"
 #include "webrtc/modules/congestion_controller/include/congestion_controller.h"
 #include "webrtc/modules/pacing/packet_router.h"
@@ -339,6 +340,14 @@ class VideoSendStreamImpl : public webrtc::BitrateAllocatorObserver,
   // RtpRtcp modules, declared here as they use other members on construction.
   const std::vector<RtpRtcp*> rtp_rtcp_modules_;
   PayloadRouter payload_router_;
+
+  // |weak_ptr_| to our self. This is used since we can not call
+  // |weak_ptr_factory_.GetWeakPtr| from multiple sequences but it is ok to copy
+  // an existing WeakPtr.
+  rtc::WeakPtr<VideoSendStreamImpl> weak_ptr_;
+  // |weak_ptr_factory_| must be declared last to make sure all WeakPtr's are
+  // invalidated before any other members are destroyed.
+  rtc::WeakPtrFactory<VideoSendStreamImpl> weak_ptr_factory_;
 };
 
 // TODO(tommi): See if there's a more elegant way to create a task that creates
@@ -431,12 +440,13 @@ class VideoSendStream::DestructAndGetRtpStateTask : public rtc::QueuedTask {
 class VideoSendStreamImpl::CheckEncoderActivityTask : public rtc::QueuedTask {
  public:
   static const int kEncoderTimeOutMs = 2000;
-  explicit CheckEncoderActivityTask(VideoSendStreamImpl* send_stream)
-      : activity_(0), send_stream_(send_stream), timed_out_(false) {}
+  explicit CheckEncoderActivityTask(
+      const rtc::WeakPtr<VideoSendStreamImpl>& send_stream)
+      : activity_(0), send_stream_(std::move(send_stream)), timed_out_(false) {}
 
   void Stop() {
     RTC_CHECK(task_checker_.CalledSequentially());
-    send_stream_ = nullptr;
+    send_stream_.reset();
   }
 
   void UpdateEncoderActivity() {
@@ -472,27 +482,28 @@ class VideoSendStreamImpl::CheckEncoderActivityTask : public rtc::QueuedTask {
   volatile int activity_;
 
   rtc::SequencedTaskChecker task_checker_;
-  VideoSendStreamImpl* send_stream_;
+  rtc::WeakPtr<VideoSendStreamImpl> send_stream_;
   bool timed_out_;
 };
 
 class VideoSendStreamImpl::EncoderReconfiguredTask : public rtc::QueuedTask {
  public:
-  EncoderReconfiguredTask(VideoSendStreamImpl* send_stream,
+  EncoderReconfiguredTask(const rtc::WeakPtr<VideoSendStreamImpl>& send_stream,
                           std::vector<VideoStream> streams,
                           int min_transmit_bitrate_bps)
-      : send_stream_(send_stream),
+      : send_stream_(std::move(send_stream)),
         streams_(std::move(streams)),
         min_transmit_bitrate_bps_(min_transmit_bitrate_bps) {}
 
  private:
   bool Run() override {
-    send_stream_->OnEncoderConfigurationChanged(std::move(streams_),
-                                                min_transmit_bitrate_bps_);
+    if (send_stream_)
+      send_stream_->OnEncoderConfigurationChanged(std::move(streams_),
+                                                  min_transmit_bitrate_bps_);
     return true;
   }
 
-  VideoSendStreamImpl* send_stream_;
+  rtc::WeakPtr<VideoSendStreamImpl> send_stream_;
   std::vector<VideoStream> streams_;
   int min_transmit_bitrate_bps_;
 };
@@ -665,9 +676,11 @@ VideoSendStreamImpl::VideoSendStreamImpl(
           congestion_controller_->GetRetransmissionRateLimiter(),
           config_->rtp.ssrcs.size())),
       payload_router_(rtp_rtcp_modules_,
-                      config_->encoder_settings.payload_type) {
+                      config_->encoder_settings.payload_type),
+      weak_ptr_factory_(this) {
   RTC_DCHECK_RUN_ON(worker_queue_);
   LOG(LS_INFO) << "VideoSendStreamInternal: " << config_->ToString();
+  weak_ptr_ = weak_ptr_factory_.GetWeakPtr();
   module_process_thread_checker_.DetachFromThread();
 
   RTC_DCHECK(!config_->rtp.ssrcs.empty());
@@ -778,7 +791,7 @@ void VideoSendStreamImpl::Start() {
   {
     rtc::CritScope lock(&encoder_activity_crit_sect_);
     RTC_DCHECK(!check_encoder_activity_task_);
-    check_encoder_activity_task_ = new CheckEncoderActivityTask(this);
+    check_encoder_activity_task_ = new CheckEncoderActivityTask(weak_ptr_);
     worker_queue_->PostDelayedTask(
         std::unique_ptr<rtc::QueuedTask>(check_encoder_activity_task_),
         CheckEncoderActivityTask::kEncoderTimeOutMs);
@@ -827,14 +840,9 @@ void VideoSendStreamImpl::OnEncoderConfigurationChanged(
     std::vector<VideoStream> streams,
     int min_transmit_bitrate_bps) {
   if (!worker_queue_->IsCurrent()) {
-    // TODO(perkj): Using |this| in post is safe for now since destruction of
-    // VideoSendStreamImpl is synchronized in
-    // VideoSendStream::StopPermanentlyAndGetRtpStates. But we should really
-    // use some kind of weak_ptr to guarantee that VideoSendStreamImpl is still
-    // alive when this task runs.
     worker_queue_->PostTask(
         std::unique_ptr<rtc::QueuedTask>(new EncoderReconfiguredTask(
-            this, std::move(streams), min_transmit_bitrate_bps)));
+            weak_ptr_, std::move(streams), min_transmit_bitrate_bps)));
     return;
   }
   RTC_DCHECK_GE(config_->rtp.ssrcs.size(), streams.size());