Browse Source

chore: cherry-pick 4 changes from Release-0-M115 (#39269)

Keeley Hammond 1 year ago
parent
commit
7a4948bd80

+ 3 - 0
patches/chromium/.patches

@@ -166,3 +166,6 @@ m114_merge_fix_a_crash_caused_by_calling_trace_event.patch
 base_do_not_use_va_args_twice_in_asprintf.patch
 cherry-pick-85beff6fd302.patch
 m114_webcodecs_fix_crash_when_changing_temporal_layer_count_in_av1.patch
+cherry-pick-933b9fad3a53.patch
+cherry-pick-b03973561862.patch
+cherry-pick-c60a1ab717c7.patch

+ 404 - 0
patches/chromium/cherry-pick-933b9fad3a53.patch

@@ -0,0 +1,404 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Ken Rockot <[email protected]>
+Date: Fri, 9 Jun 2023 07:49:02 +0000
+Subject: Reland "ipcz: Refactor FragmentDescriptor decode"
+
+This is a reland of commit 17dd18d1f2194089b8433e0ca334c81343b591e2
+
+Original change's description:
+> ipcz: Refactor FragmentDescriptor decode
+>
+> Funnels untrusted FragmentDescriptor mapping through a new
+> Fragment::MappedFromDescriptor helper. See the linked bug
+> for more details.
+>
+> Fixed: 1450899
+> Change-Id: I4c7751b9f4299da4a13c0becc1b889160a0c6e66
+> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4599218
+> Reviewed-by: Daniel Cheng <[email protected]>
+> Commit-Queue: Ken Rockot <[email protected]>
+> Cr-Commit-Position: refs/heads/main@{#1155133}
+
+Change-Id: I86ee9118a30dea59d837c377a1f751b20a85a3c3
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4602794
+Reviewed-by: Daniel Cheng <[email protected]>
+Commit-Queue: Ken Rockot <[email protected]>
+Cr-Commit-Position: refs/heads/main@{#1155397}
+
+diff --git a/third_party/ipcz/src/BUILD.gn b/third_party/ipcz/src/BUILD.gn
+index 8b98d75aaa8f6d4df22c872cb9261381647256eb..7dabb9e9d4f3b0c00ce0acbcb2b7ff0e6085fc58 100644
+--- a/third_party/ipcz/src/BUILD.gn
++++ b/third_party/ipcz/src/BUILD.gn
+@@ -210,6 +210,7 @@ ipcz_source_set("impl") {
+     "ipcz/api_object.h",
+     "ipcz/block_allocator.h",
+     "ipcz/box.h",
++    "ipcz/buffer_id.h",
+     "ipcz/buffer_pool.h",
+     "ipcz/driver_memory.h",
+     "ipcz/driver_memory_mapping.h",
+@@ -253,7 +254,6 @@ ipcz_source_set("impl") {
+     "ipcz/block_allocator_pool.cc",
+     "ipcz/block_allocator_pool.h",
+     "ipcz/box.cc",
+-    "ipcz/buffer_id.h",
+     "ipcz/buffer_pool.cc",
+     "ipcz/driver_memory.cc",
+     "ipcz/driver_memory_mapping.cc",
+@@ -376,6 +376,7 @@ ipcz_source_set("ipcz_tests_sources") {
+     "ipcz/driver_memory_test.cc",
+     "ipcz/driver_object_test.cc",
+     "ipcz/driver_transport_test.cc",
++    "ipcz/fragment_test.cc",
+     "ipcz/message_test.cc",
+     "ipcz/node_connector_test.cc",
+     "ipcz/node_link_memory_test.cc",
+diff --git a/third_party/ipcz/src/ipcz/block_allocator_pool.cc b/third_party/ipcz/src/ipcz/block_allocator_pool.cc
+index bd464f897d1fcbde03941ee334d0e1706bf59868..1b9d50b2c77c046d815a94d7760328c8b379ecab 100644
+--- a/third_party/ipcz/src/ipcz/block_allocator_pool.cc
++++ b/third_party/ipcz/src/ipcz/block_allocator_pool.cc
+@@ -86,7 +86,7 @@ Fragment BlockAllocatorPool::Allocate() {
+       FragmentDescriptor descriptor(
+           entry->buffer_id, checked_cast<uint32_t>(offset),
+           checked_cast<uint32_t>(allocator.block_size()));
+-      return Fragment(descriptor, block);
++      return Fragment::FromDescriptorUnsafe(descriptor, block);
+     }
+ 
+     // Allocation from the active allocator failed. Try another if available.
+diff --git a/third_party/ipcz/src/ipcz/buffer_pool.cc b/third_party/ipcz/src/ipcz/buffer_pool.cc
+index 6881346d8f8532f070e5121da16f064ae4a9bdaf..27b23049848967f29f81b10ba4f8fa4ead14d2e2 100644
+--- a/third_party/ipcz/src/ipcz/buffer_pool.cc
++++ b/third_party/ipcz/src/ipcz/buffer_pool.cc
+@@ -26,15 +26,11 @@ Fragment BufferPool::GetFragment(const FragmentDescriptor& descriptor) {
+   absl::MutexLock lock(&mutex_);
+   auto it = mappings_.find(descriptor.buffer_id());
+   if (it == mappings_.end()) {
+-    return Fragment(descriptor, nullptr);
++    return Fragment::PendingFromDescriptor(descriptor);
+   }
+ 
+   auto& [id, mapping] = *it;
+-  if (descriptor.end() > mapping.bytes().size()) {
+-    return {};
+-  }
+-
+-  return Fragment(descriptor, mapping.address_at(descriptor.offset()));
++  return Fragment::MappedFromDescriptor(descriptor, mapping);
+ }
+ 
+ bool BufferPool::AddBlockBuffer(
+diff --git a/third_party/ipcz/src/ipcz/buffer_pool_test.cc b/third_party/ipcz/src/ipcz/buffer_pool_test.cc
+index a009ffe1c20ade013a19b51eceee4faf334eb591..bff66c452a3e2c38b0f208cca1fa1a082f1ee871 100644
+--- a/third_party/ipcz/src/ipcz/buffer_pool_test.cc
++++ b/third_party/ipcz/src/ipcz/buffer_pool_test.cc
+@@ -194,9 +194,11 @@ TEST_F(BufferPoolTest, BasicBlockAllocation) {
+             pool.GetTotalBlockCapacity(kBlockSize));
+ 
+   // We can't free something that isn't a valid allocation.
+-  EXPECT_FALSE(pool.FreeBlock(Fragment{{}, nullptr}));
+-  EXPECT_FALSE(pool.FreeBlock(Fragment{{BufferId{1000}, 0, 1}, nullptr}));
+-  EXPECT_FALSE(pool.FreeBlock(Fragment{{BufferId{0}, 0, 1}, bytes0.data()}));
++  EXPECT_FALSE(pool.FreeBlock(Fragment::FromDescriptorUnsafe({}, nullptr)));
++  EXPECT_FALSE(pool.FreeBlock(
++      Fragment::FromDescriptorUnsafe({BufferId{1000}, 0, 1}, nullptr)));
++  EXPECT_FALSE(pool.FreeBlock(
++      Fragment::FromDescriptorUnsafe({BufferId{0}, 0, 1}, bytes0.data())));
+ 
+   // Allocate all available capacity.
+   std::vector<Fragment> fragments;
+diff --git a/third_party/ipcz/src/ipcz/fragment.cc b/third_party/ipcz/src/ipcz/fragment.cc
+index 651d1c2fca5fe4fb69cdf61c6062bd8804ebf704..2ef4ed8dcfa0a56a73975a0b7dcc3f86bf5a83a0 100644
+--- a/third_party/ipcz/src/ipcz/fragment.cc
++++ b/third_party/ipcz/src/ipcz/fragment.cc
+@@ -6,10 +6,38 @@
+ 
+ #include <cstdint>
+ 
++#include "ipcz/driver_memory_mapping.h"
++#include "ipcz/fragment_descriptor.h"
+ #include "third_party/abseil-cpp/absl/base/macros.h"
++#include "util/safe_math.h"
+ 
+ namespace ipcz {
+ 
++// static
++Fragment Fragment::MappedFromDescriptor(const FragmentDescriptor& descriptor,
++                                        DriverMemoryMapping& mapping) {
++  if (descriptor.is_null()) {
++    return {};
++  }
++
++  const uint32_t end = SaturatedAdd(descriptor.offset(), descriptor.size());
++  if (end > mapping.bytes().size()) {
++    return {};
++  }
++  return Fragment{descriptor, mapping.address_at(descriptor.offset())};
++}
++
++// static
++Fragment Fragment::PendingFromDescriptor(const FragmentDescriptor& descriptor) {
++  return Fragment{descriptor, nullptr};
++}
++
++// static
++Fragment Fragment::FromDescriptorUnsafe(const FragmentDescriptor& descriptor,
++                                        void* base_address) {
++  return Fragment{descriptor, base_address};
++}
++
+ Fragment::Fragment(const FragmentDescriptor& descriptor, void* address)
+     : descriptor_(descriptor), address_(address) {
+   // If `address` is non-null, the descriptor must also be. Note that the
+diff --git a/third_party/ipcz/src/ipcz/fragment.h b/third_party/ipcz/src/ipcz/fragment.h
+index c0151fdcf4b418680172a29d1c0d28b58a5807cd..de65f087b0bc27fd59ab88e23130d5ce0d345a8a 100644
+--- a/third_party/ipcz/src/ipcz/fragment.h
++++ b/third_party/ipcz/src/ipcz/fragment.h
+@@ -14,21 +14,32 @@
+ 
+ namespace ipcz {
+ 
++class DriverMemoryMapping;
++
+ // Represents a span of memory located within the shared memory regions owned by
+ // a NodeLinkMemory, via BufferPool. This is essentially a FragmentDescriptor
+ // plus the actual mapped address of the given buffer and offset.
+ struct Fragment {
+   constexpr Fragment() = default;
+ 
+-  // Constructs a new Fragment over `descriptor`, mapped to `address`. If
+-  // `address` is null, the Fragment is considered "pending" -- it has a
+-  // potentially valid descriptor, but could not be resolved to a mapped address
+-  // yet (e.g. because the relevant BufferPool doesn't have the identified
+-  // buffer mapped yet.)
+-  Fragment(const FragmentDescriptor& descriptor, void* address);
+   Fragment(const Fragment&);
+   Fragment& operator=(const Fragment&);
+ 
++  // Returns a new concrete Fragment corresponding to `descriptor` within the
++  // context of `mapping`. This validates that the fragment's bounds fall within
++  // the bounds of `mapping`. If `descriptor` was null or validation fails, this
++  // returns a null Fragment.
++  static Fragment MappedFromDescriptor(const FragmentDescriptor& descriptor,
++                                       DriverMemoryMapping& mapping);
++
++  // Returns a pending Fragment corresponding to `descriptor`.
++  static Fragment PendingFromDescriptor(const FragmentDescriptor& descriptor);
++
++  // Returns a Fragment corresponding to `descriptor`, with the starting address
++  // already mapped to `address`.
++  static Fragment FromDescriptorUnsafe(const FragmentDescriptor& descriptor,
++                                       void* address);
++
+   // A null fragment is a fragment with a null descriptor, meaning it does not
+   // reference a valid buffer ID.
+   bool is_null() const { return descriptor_.is_null(); }
+@@ -66,6 +77,13 @@ struct Fragment {
+   }
+ 
+  private:
++  // Constructs a new Fragment over `descriptor`, mapped to `address`. If
++  // `address` is null, the Fragment is considered "pending" -- it has a
++  // potentially valid descriptor, but could not be resolved to a mapped address
++  // yet (e.g. because the relevant BufferPool doesn't have the identified
++  // buffer mapped yet.)
++  Fragment(const FragmentDescriptor& descriptor, void* address);
++
+   FragmentDescriptor descriptor_;
+ 
+   // The actual mapped address corresponding to `descriptor_`.
+diff --git a/third_party/ipcz/src/ipcz/fragment_descriptor.h b/third_party/ipcz/src/ipcz/fragment_descriptor.h
+index b247215fd5e5f7c69e521416614465b0321f5d83..aeaa7da9c82761854948d009e7f245c9c9d042c7 100644
+--- a/third_party/ipcz/src/ipcz/fragment_descriptor.h
++++ b/third_party/ipcz/src/ipcz/fragment_descriptor.h
+@@ -39,7 +39,6 @@ struct IPCZ_ALIGN(8) FragmentDescriptor {
+   BufferId buffer_id() const { return buffer_id_; }
+   uint32_t offset() const { return offset_; }
+   uint32_t size() const { return size_; }
+-  uint32_t end() const { return offset_ + size_; }
+ 
+  private:
+   // Identifies the shared memory buffer in which the memory resides. This ID is
+diff --git a/third_party/ipcz/src/ipcz/fragment_test.cc b/third_party/ipcz/src/ipcz/fragment_test.cc
+new file mode 100644
+index 0000000000000000000000000000000000000000..e6b6baa6cb2f1fbdfb89d87d644f63681c797c01
+--- /dev/null
++++ b/third_party/ipcz/src/ipcz/fragment_test.cc
+@@ -0,0 +1,102 @@
++// Copyright 2023 The Chromium Authors
++// Use of this source code is governed by a BSD-style license that can be
++// found in the LICENSE file.
++
++#include "ipcz/fragment.h"
++
++#include <algorithm>
++#include <cstring>
++#include <limits>
++#include <string>
++#include <utility>
++
++#include "ipcz/buffer_id.h"
++#include "ipcz/driver_memory.h"
++#include "ipcz/driver_memory_mapping.h"
++#include "reference_drivers/sync_reference_driver.h"
++#include "testing/gtest/include/gtest/gtest.h"
++
++namespace ipcz {
++namespace {
++
++const IpczDriver& kTestDriver = reference_drivers::kSyncReferenceDriver;
++
++using FragmentTest = testing::Test;
++
++TEST_F(FragmentTest, FromDescriptorUnsafe) {
++  char kBuffer[] = "Hello, world!";
++
++  Fragment f = Fragment::FromDescriptorUnsafe({BufferId{0}, 1, 4}, kBuffer + 1);
++  EXPECT_FALSE(f.is_null());
++  EXPECT_FALSE(f.is_pending());
++  EXPECT_EQ(1u, f.offset());
++  EXPECT_EQ(4u, f.size());
++  EXPECT_EQ("ello", std::string(f.bytes().begin(), f.bytes().end()));
++
++  f = Fragment::FromDescriptorUnsafe({BufferId{0}, 7, 6}, kBuffer + 7);
++  EXPECT_FALSE(f.is_null());
++  EXPECT_FALSE(f.is_pending());
++  EXPECT_EQ(7u, f.offset());
++  EXPECT_EQ(6u, f.size());
++  EXPECT_EQ("world!", std::string(f.bytes().begin(), f.bytes().end()));
++}
++
++TEST_F(FragmentTest, PendingFromDescriptor) {
++  Fragment f = Fragment::PendingFromDescriptor({BufferId{0}, 5, 42});
++  EXPECT_TRUE(f.is_pending());
++  EXPECT_FALSE(f.is_null());
++  EXPECT_EQ(5u, f.offset());
++  EXPECT_EQ(42u, f.size());
++
++  f = Fragment::PendingFromDescriptor({kInvalidBufferId, 0, 0});
++  EXPECT_TRUE(f.is_null());
++  EXPECT_FALSE(f.is_pending());
++}
++
++TEST_F(FragmentTest, NullMappedFromDescriptor) {
++  constexpr size_t kDataSize = 32;
++  DriverMemory memory(kTestDriver, kDataSize);
++  auto mapping = memory.Map();
++
++  Fragment f =
++      Fragment::MappedFromDescriptor({kInvalidBufferId, 0, 0}, mapping);
++  EXPECT_TRUE(f.is_null());
++}
++
++TEST_F(FragmentTest, InvalidMappedFromDescriptor) {
++  constexpr size_t kDataSize = 32;
++  DriverMemory memory(kTestDriver, kDataSize);
++  auto mapping = memory.Map();
++
++  Fragment f;
++
++  // Offset out of bounds
++  f = Fragment::MappedFromDescriptor({BufferId{0}, kDataSize, 1}, mapping);
++  EXPECT_TRUE(f.is_null());
++
++  // Tail out of bounds
++  f = Fragment::MappedFromDescriptor({BufferId{0}, 0, kDataSize + 5}, mapping);
++  EXPECT_TRUE(f.is_null());
++
++  // Tail overflow
++  f = Fragment::MappedFromDescriptor(
++      {BufferId{0}, std::numeric_limits<uint32_t>::max(), 2}, mapping);
++  EXPECT_TRUE(f.is_null());
++}
++
++TEST_F(FragmentTest, ValidMappedFromDescriptor) {
++  const char kData[] = "0123456789abcdef";
++  DriverMemory memory(kTestDriver, std::size(kData));
++  auto mapping = memory.Map();
++  memcpy(mapping.bytes().data(), kData, std::size(kData));
++
++  Fragment f = Fragment::MappedFromDescriptor({BufferId{0}, 2, 11}, mapping);
++  EXPECT_FALSE(f.is_null());
++  EXPECT_FALSE(f.is_pending());
++  EXPECT_EQ(2u, f.offset());
++  EXPECT_EQ(11u, f.size());
++  EXPECT_EQ("23456789abc", std::string(f.bytes().begin(), f.bytes().end()));
++}
++
++}  // namespace
++}  // namespace ipcz
+diff --git a/third_party/ipcz/src/ipcz/node_link_memory.cc b/third_party/ipcz/src/ipcz/node_link_memory.cc
+index 0cfadfa02aa4b7058e04c3b0255412a0d11aed87..9f920faf31748a6c4213045e31331af2a865bb4f 100644
+--- a/third_party/ipcz/src/ipcz/node_link_memory.cc
++++ b/third_party/ipcz/src/ipcz/node_link_memory.cc
+@@ -278,8 +278,9 @@ FragmentRef<RouterLinkState> NodeLinkMemory::GetInitialRouterLinkState(
+   FragmentDescriptor descriptor(kPrimaryBufferId,
+                                 ToOffset(state, primary_buffer_memory_.data()),
+                                 sizeof(RouterLinkState));
+-  return FragmentRef<RouterLinkState>(RefCountedFragment::kUnmanagedRef,
+-                                      Fragment(descriptor, state));
++  return FragmentRef<RouterLinkState>(
++      RefCountedFragment::kUnmanagedRef,
++      Fragment::FromDescriptorUnsafe(descriptor, state));
+ }
+ 
+ Fragment NodeLinkMemory::GetFragment(const FragmentDescriptor& descriptor) {
+diff --git a/third_party/ipcz/src/ipcz/ref_counted_fragment_test.cc b/third_party/ipcz/src/ipcz/ref_counted_fragment_test.cc
+index d5a2243a693597e43f87f116f80599fde383cb59..220c3556a261c5caab7194114af4cf375d9af683 100644
+--- a/third_party/ipcz/src/ipcz/ref_counted_fragment_test.cc
++++ b/third_party/ipcz/src/ipcz/ref_counted_fragment_test.cc
+@@ -64,7 +64,8 @@ TEST_F(RefCountedFragmentTest, SimpleRef) {
+ 
+   FragmentRef<TestObject> ref(
+       RefCountedFragment::kUnmanagedRef,
+-      Fragment(FragmentDescriptor(BufferId(0), 0, sizeof(object)), &object));
++      Fragment::FromDescriptorUnsafe(
++          FragmentDescriptor(BufferId(0), 0, sizeof(object)), &object));
+   EXPECT_EQ(1, object.ref_count_for_testing());
+   ref.reset();
+   EXPECT_EQ(0, object.ref_count_for_testing());
+@@ -75,7 +76,8 @@ TEST_F(RefCountedFragmentTest, Copy) {
+ 
+   FragmentRef<TestObject> ref1(
+       RefCountedFragment::kUnmanagedRef,
+-      Fragment(FragmentDescriptor(BufferId(0), 0, sizeof(object1)), &object1));
++      Fragment::FromDescriptorUnsafe(
++          FragmentDescriptor(BufferId(0), 0, sizeof(object1)), &object1));
+   EXPECT_EQ(1, object1.ref_count_for_testing());
+ 
+   FragmentRef<TestObject> other1 = ref1;
+@@ -88,7 +90,8 @@ TEST_F(RefCountedFragmentTest, Copy) {
+   TestObject object2;
+   auto ref2 = FragmentRef<TestObject>(
+       RefCountedFragment::kUnmanagedRef,
+-      Fragment(FragmentDescriptor(BufferId(0), 0, sizeof(object2)), &object2));
++      Fragment::FromDescriptorUnsafe(
++          FragmentDescriptor(BufferId(0), 0, sizeof(object2)), &object2));
+   EXPECT_EQ(1, object1.ref_count_for_testing());
+   EXPECT_EQ(1, object2.ref_count_for_testing());
+   ref2 = ref1;
+@@ -115,7 +118,8 @@ TEST_F(RefCountedFragmentTest, Move) {
+ 
+   FragmentRef<TestObject> ref1(
+       RefCountedFragment::kUnmanagedRef,
+-      Fragment(FragmentDescriptor(BufferId(0), 0, sizeof(object1)), &object1));
++      Fragment::FromDescriptorUnsafe(
++          FragmentDescriptor(BufferId(0), 0, sizeof(object1)), &object1));
+   EXPECT_EQ(1, ref1.ref_count_for_testing());
+ 
+   FragmentRef<TestObject> other1 = std::move(ref1);
+@@ -133,10 +137,12 @@ TEST_F(RefCountedFragmentTest, Move) {
+   TestObject object3;
+   FragmentRef<TestObject> ref2(
+       RefCountedFragment::kUnmanagedRef,
+-      Fragment(FragmentDescriptor(BufferId(0), 0, sizeof(object2)), &object2));
++      Fragment::FromDescriptorUnsafe(
++          FragmentDescriptor(BufferId(0), 0, sizeof(object2)), &object2));
+   FragmentRef<TestObject> ref3(
+       RefCountedFragment::kUnmanagedRef,
+-      Fragment(FragmentDescriptor(BufferId(0), 0, sizeof(object3)), &object3));
++      Fragment::FromDescriptorUnsafe(
++          FragmentDescriptor(BufferId(0), 0, sizeof(object3)), &object3));
+ 
+   EXPECT_FALSE(ref2.is_null());
+   EXPECT_TRUE(ref2.is_addressable());

+ 119 - 0
patches/chromium/cherry-pick-b03973561862.patch

@@ -0,0 +1,119 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Tommi <[email protected]>
+Date: Wed, 5 Jul 2023 10:55:53 +0000
+Subject: Make RTCDataChannel's channel and observer pointers const.
+
+This allows channel properties to be queried while the RTCDataChannel
+instance exists and avoids potential null deref after entering the
+kClosed state.
+
+(cherry picked from commit 08d5ad011f53a1995bfccef6728bfa62541f7608)
+
+Bug: 1456567, 1457421
+Change-Id: I4747f9c00804b35711667d7320ec6188f20910c4
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4663082
+Commit-Queue: Tomas Gunnarsson <[email protected]>
+Reviewed-by: Elad Alon <[email protected]>
+Cr-Original-Commit-Position: refs/heads/main@{#1165406}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4665530
+Cr-Commit-Position: refs/branch-heads/5845@{#300}
+Cr-Branched-From: 5a5dff63a4a4c63b9b18589819bebb2566c85443-refs/heads/main@{#1160321}
+
+diff --git a/third_party/blink/renderer/modules/peerconnection/rtc_data_channel.cc b/third_party/blink/renderer/modules/peerconnection/rtc_data_channel.cc
+index 78d28d60822f4ce206d869846235352224378076..91c20cbcc5042373964d57545177ff06074db564 100644
+--- a/third_party/blink/renderer/modules/peerconnection/rtc_data_channel.cc
++++ b/third_party/blink/renderer/modules/peerconnection/rtc_data_channel.cc
+@@ -228,11 +228,12 @@ RTCDataChannel::Observer::Observer(
+     scoped_refptr<webrtc::DataChannelInterface> channel)
+     : main_thread_(main_thread),
+       blink_channel_(blink_channel),
+-      webrtc_channel_(channel) {}
++      webrtc_channel_(std::move(channel)) {
++  CHECK(webrtc_channel_.get());
++}
+ 
+ RTCDataChannel::Observer::~Observer() {
+   DCHECK(!blink_channel_) << "Reference to blink channel hasn't been released.";
+-  DCHECK(!webrtc_channel_.get()) << "Unregister hasn't been called.";
+ }
+ 
+ const scoped_refptr<webrtc::DataChannelInterface>&
+@@ -242,13 +243,8 @@ RTCDataChannel::Observer::channel() const {
+ 
+ void RTCDataChannel::Observer::Unregister() {
+   DCHECK(main_thread_->BelongsToCurrentThread());
++  webrtc_channel_->UnregisterObserver();
+   blink_channel_ = nullptr;
+-  if (webrtc_channel_.get()) {
+-    webrtc_channel_->UnregisterObserver();
+-    // Now that we're guaranteed to not get further OnStateChange callbacks,
+-    // it's safe to release our reference to the channel.
+-    webrtc_channel_ = nullptr;
+-  }
+ }
+ 
+ void RTCDataChannel::Observer::OnStateChange() {
+@@ -302,7 +298,7 @@ void RTCDataChannel::Observer::OnMessageImpl(
+ 
+ RTCDataChannel::RTCDataChannel(
+     ExecutionContext* context,
+-    scoped_refptr<webrtc::DataChannelInterface> channel,
++    scoped_refptr<webrtc::DataChannelInterface> data_channel,
+     RTCPeerConnectionHandler* peer_connection_handler)
+     : ExecutionContextLifecycleObserver(context),
+       state_(webrtc::DataChannelInterface::kConnecting),
+@@ -317,7 +313,7 @@ RTCDataChannel::RTCDataChannel(
+       observer_(base::MakeRefCounted<Observer>(
+           context->GetTaskRunner(TaskType::kNetworking),
+           this,
+-          channel)),
++          std::move(data_channel))),
+       signaling_thread_(peer_connection_handler->signaling_thread()) {
+   DCHECK(peer_connection_handler);
+ 
+@@ -340,7 +336,7 @@ RTCDataChannel::RTCDataChannel(
+           observer_, state_),
+       "RegisterObserverAndGetStateUpdate");
+ 
+-  IncrementCounters(*channel.get());
++  IncrementCounters(*(observer_->channel()).get());
+ }
+ 
+ RTCDataChannel::~RTCDataChannel() = default;
+@@ -689,9 +685,8 @@ void RTCDataChannel::Dispose() {
+   if (stopped_)
+     return;
+ 
+-  // Clears the weak persistent reference to this on-heap object.
++  // Clear the weak persistent reference to this on-heap object.
+   observer_->Unregister();
+-  observer_ = nullptr;
+ }
+ 
+ void RTCDataChannel::ScheduleDispatchEvent(Event* event) {
+diff --git a/third_party/blink/renderer/modules/peerconnection/rtc_data_channel.h b/third_party/blink/renderer/modules/peerconnection/rtc_data_channel.h
+index 21bb39382ac0c6acbf984ffbda5f6a4e6c863432..6959b8b1e3a0b586be68cb4a8d0389b7926b98fe 100644
+--- a/third_party/blink/renderer/modules/peerconnection/rtc_data_channel.h
++++ b/third_party/blink/renderer/modules/peerconnection/rtc_data_channel.h
+@@ -152,7 +152,7 @@ class MODULES_EXPORT RTCDataChannel final
+ 
+     const scoped_refptr<base::SingleThreadTaskRunner> main_thread_;
+     WeakPersistent<RTCDataChannel> blink_channel_;
+-    scoped_refptr<webrtc::DataChannelInterface> webrtc_channel_;
++    const scoped_refptr<webrtc::DataChannelInterface> webrtc_channel_;
+   };
+ 
+   void OnStateChange(webrtc::DataChannelInterface::DataState state);
+@@ -195,7 +195,11 @@ class MODULES_EXPORT RTCDataChannel final
+   unsigned buffered_amount_;
+   bool stopped_;
+   bool closed_from_owner_;
+-  scoped_refptr<Observer> observer_;
++  // Keep the `observer_` reference const to make it clear that we don't want
++  // to free the underlying channel (or callback observer) until the
++  // `RTCDataChannel` instance goes away. This allows properties to be queried
++  // after the state reaches `kClosed`.
++  const scoped_refptr<Observer> observer_;
+   scoped_refptr<base::SingleThreadTaskRunner> signaling_thread_;
+   THREAD_CHECKER(thread_checker_);
+ };

+ 187 - 0
patches/chromium/cherry-pick-c60a1ab717c7.patch

@@ -0,0 +1,187 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Taylor Bergquist <[email protected]>
+Date: Tue, 11 Jul 2023 01:32:22 +0000
+Subject: Fix UAF when exiting a nested run loop in
+ TabDragContextImpl::OnGestureEvent.
+
+OnGestureEvent may call ContinueDrag, which may run a nested run loop. After the nested run loop returns, multiple seconds of time may have passed, and the world may be in a very different state; in particular, the window that contains this TabDragContext may have closed.
+
+This CL checks if this has happened, and returns early in that case.
+
+(cherry picked from commit 63d6b8ba8126b16215d33670df8c67dcbc6c9bef)
+
+Bug: 1453465
+Change-Id: I6095c0afeb5aa5f422717f1bbd93b96175e52afa
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4657527
+Reviewed-by: Darryl James <[email protected]>
+Commit-Queue: Taylor Bergquist <[email protected]>
+Code-Coverage: Findit <[email protected]>
+Cr-Original-Commit-Position: refs/heads/main@{#1164449}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4676126
+Reviewed-by: Shibalik Mohapatra <[email protected]>
+Cr-Commit-Position: refs/branch-heads/5845@{#410}
+Cr-Branched-From: 5a5dff63a4a4c63b9b18589819bebb2566c85443-refs/heads/main@{#1160321}
+
+diff --git a/chrome/browser/ui/views/tabs/fake_tab_slot_controller.cc b/chrome/browser/ui/views/tabs/fake_tab_slot_controller.cc
+index 94d27ec3ab22f7afb5e265fdcee662e48f36c00e..81348ceb4a415f789dd384988a54662ec3c3d0e0 100644
+--- a/chrome/browser/ui/views/tabs/fake_tab_slot_controller.cc
++++ b/chrome/browser/ui/views/tabs/fake_tab_slot_controller.cc
+@@ -45,6 +45,12 @@ bool FakeTabSlotController::IsFocusInTabs() const {
+   return false;
+ }
+ 
++TabSlotController::Liveness FakeTabSlotController::ContinueDrag(
++    views::View* view,
++    const ui::LocatedEvent& event) {
++  return Liveness::kAlive;
++}
++
+ bool FakeTabSlotController::EndDrag(EndDragReason reason) {
+   return false;
+ }
+diff --git a/chrome/browser/ui/views/tabs/fake_tab_slot_controller.h b/chrome/browser/ui/views/tabs/fake_tab_slot_controller.h
+index ebbae9b19a8f9534e3a86d2bf28875418a7be80d..4795c222651af3a7146a8bf12e824a1480cdbfae 100644
+--- a/chrome/browser/ui/views/tabs/fake_tab_slot_controller.h
++++ b/chrome/browser/ui/views/tabs/fake_tab_slot_controller.h
+@@ -60,8 +60,8 @@ class FakeTabSlotController : public TabSlotController {
+       TabSlotView* source,
+       const ui::LocatedEvent& event,
+       const ui::ListSelectionModel& original_selection) override {}
+-  void ContinueDrag(views::View* view, const ui::LocatedEvent& event) override {
+-  }
++  Liveness ContinueDrag(views::View* view,
++                        const ui::LocatedEvent& event) override;
+   bool EndDrag(EndDragReason reason) override;
+   Tab* GetTabAt(const gfx::Point& point) override;
+   const Tab* GetAdjacentTab(const Tab* tab, int offset) override;
+diff --git a/chrome/browser/ui/views/tabs/tab_slot_controller.h b/chrome/browser/ui/views/tabs/tab_slot_controller.h
+index 6a43f963a22917b9e9b861d3619d9804851dfb5b..d412ad6d5d9a95aa5d635fae5232eddfa87132ae 100644
+--- a/chrome/browser/ui/views/tabs/tab_slot_controller.h
++++ b/chrome/browser/ui/views/tabs/tab_slot_controller.h
+@@ -49,6 +49,8 @@ class TabSlotController {
+     kEvent
+   };
+ 
++  enum class Liveness { kAlive, kDeleted };
++
+   virtual const ui::ListSelectionModel& GetSelectionModel() const = 0;
+ 
+   // Returns the tab at |index|.
+@@ -126,9 +128,10 @@ class TabSlotController {
+       const ui::LocatedEvent& event,
+       const ui::ListSelectionModel& original_selection) = 0;
+ 
+-  // Continues dragging a Tab.
+-  virtual void ContinueDrag(views::View* view,
+-                            const ui::LocatedEvent& event) = 0;
++  // Continues dragging a Tab. May enter a nested event loop - returns
++  // Liveness::kDeleted if `this` was destroyed during this nested event loop,
++  // and Liveness::kAlive if `this` is still alive.
++  virtual Liveness ContinueDrag(views::View* view, const ui::LocatedEvent& event) = 0;
+ 
+   // Ends dragging a Tab. Returns whether the tab has been destroyed.
+   virtual bool EndDrag(EndDragReason reason) = 0;
+diff --git a/chrome/browser/ui/views/tabs/tab_strip.cc b/chrome/browser/ui/views/tabs/tab_strip.cc
+index f2f18c7f21bc249b95dece091716fcb6872b0c12..22a4b6b761058f44cbea029bbc52dd4071e111e9 100644
+--- a/chrome/browser/ui/views/tabs/tab_strip.cc
++++ b/chrome/browser/ui/views/tabs/tab_strip.cc
+@@ -174,7 +174,7 @@ class TabStrip::TabDragContextImpl : public TabDragContext,
+   }
+ 
+   bool OnMouseDragged(const ui::MouseEvent& event) override {
+-    ContinueDrag(this, event);
++    (void)ContinueDrag(this, event);
+     return true;
+   }
+ 
+@@ -185,6 +185,7 @@ class TabStrip::TabDragContextImpl : public TabDragContext,
+   void OnMouseCaptureLost() override { EndDrag(END_DRAG_CAPTURE_LOST); }
+ 
+   void OnGestureEvent(ui::GestureEvent* event) override {
++    Liveness tabstrip_alive = Liveness::kAlive;
+     switch (event->type()) {
+       case ui::ET_GESTURE_SCROLL_END:
+       case ui::ET_SCROLL_FLING_START:
+@@ -198,7 +199,8 @@ class TabStrip::TabDragContextImpl : public TabDragContext,
+       }
+ 
+       case ui::ET_GESTURE_SCROLL_UPDATE:
+-        ContinueDrag(this, *event);
++        // N.B. !! ContinueDrag may enter a nested run loop !!
++        tabstrip_alive = ContinueDrag(this, *event);
+         break;
+ 
+       case ui::ET_GESTURE_TAP_DOWN:
+@@ -210,6 +212,12 @@ class TabStrip::TabDragContextImpl : public TabDragContext,
+     }
+     event->SetHandled();
+ 
++    // If tabstrip was destroyed (during ContinueDrag above), return early to
++    // avoid UAF below.
++    if (tabstrip_alive == Liveness::kDeleted) {
++      return;
++    }
++
+     // TabDragContext gets event capture as soon as a drag session begins, which
+     // precludes TabStrip from ever getting events like tap or long tap. Forward
+     // this on to TabStrip so it can respond to those events.
+@@ -295,20 +303,20 @@ class TabStrip::TabDragContextImpl : public TabDragContext,
+       std::move(drag_controller_set_callback_).Run(drag_controller_.get());
+   }
+ 
+-  void ContinueDrag(views::View* view, const ui::LocatedEvent& event) {
+-    if (drag_controller_.get() &&
+-        drag_controller_->event_source() == EventSourceFromEvent(event)) {
+-      gfx::Point screen_location(event.location());
+-      views::View::ConvertPointToScreen(view, &screen_location);
++  Liveness ContinueDrag(views::View* view, const ui::LocatedEvent& event) {
++    if (!drag_controller_.get() ||
++        drag_controller_->event_source() != EventSourceFromEvent(event)) {
++      return Liveness::kAlive;
++    }
+ 
+-      // Note: |tab_strip_| can be destroyed during drag, also destroying
+-      // |this|.
+-      base::WeakPtr<TabDragContext> weak_ptr(weak_factory_.GetWeakPtr());
+-      drag_controller_->Drag(screen_location);
++    gfx::Point screen_location(event.location());
++    views::View::ConvertPointToScreen(view, &screen_location);
+ 
+-      if (!weak_ptr)
+-        return;
+-    }
++    // Note: `tab_strip_` can be destroyed during drag, also destroying `this`.
++    base::WeakPtr<TabDragContext> weak_ptr(weak_factory_.GetWeakPtr());
++    drag_controller_->Drag(screen_location);
++
++    return weak_ptr ? Liveness::kAlive : Liveness::kDeleted;
+   }
+ 
+   bool EndDrag(EndDragReason reason) {
+@@ -1573,8 +1581,10 @@ void TabStrip::MaybeStartDrag(
+   drag_context_->MaybeStartDrag(source, event, original_selection);
+ }
+ 
+-void TabStrip::ContinueDrag(views::View* view, const ui::LocatedEvent& event) {
+-  drag_context_->ContinueDrag(view, event);
++TabSlotController::Liveness TabStrip::ContinueDrag(
++    views::View* view,
++    const ui::LocatedEvent& event) {
++  return drag_context_->ContinueDrag(view, event);
+ }
+ 
+ bool TabStrip::EndDrag(EndDragReason reason) {
+diff --git a/chrome/browser/ui/views/tabs/tab_strip.h b/chrome/browser/ui/views/tabs/tab_strip.h
+index fce93e7d993434ce1e99c7cbca71c1391798c1a4..de6f20bb978c32d635b4f8b75841c148996df2c6 100644
+--- a/chrome/browser/ui/views/tabs/tab_strip.h
++++ b/chrome/browser/ui/views/tabs/tab_strip.h
+@@ -278,7 +278,8 @@ class TabStrip : public views::View,
+       TabSlotView* source,
+       const ui::LocatedEvent& event,
+       const ui::ListSelectionModel& original_selection) override;
+-  void ContinueDrag(views::View* view, const ui::LocatedEvent& event) override;
++  Liveness ContinueDrag(views::View* view,
++                        const ui::LocatedEvent& event) override;
+   bool EndDrag(EndDragReason reason) override;
+   Tab* GetTabAt(const gfx::Point& point) override;
+   const Tab* GetAdjacentTab(const Tab* tab, int offset) override;