Browse Source

chore: cherry-pick 1d491fff578b, f7b87bea19d7 and 549d92d7ef35 from chromium (#38068)

* chore: cherry-pick 1d491fff578b from chromium

* chore: update patches

---------

Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <[email protected]>
Pedro Pontes 2 years ago
parent
commit
29cdcb443a

+ 3 - 0
patches/chromium/.patches

@@ -149,4 +149,7 @@ cherry-pick-1235110fce18.patch
 cherry-pick-b041159d06ad.patch
 cherry-pick-d6946b70b431.patch
 cherry-pick-d9081493c4b2.patch
+merge_m112_remove_the_second_weakptrfactory_from.patch
+merge_m112_check_spdyproxyclientsocket_is_alive_after_write.patch
+check_callback_availability_in.patch
 cherry-pick-63686953dc22.patch

+ 36 - 0
patches/chromium/check_callback_availability_in.patch

@@ -0,0 +1,36 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Kenichi Ishibashi <[email protected]>
+Date: Tue, 18 Apr 2023 05:58:29 +0000
+Subject: Check callback availability in
+ SpdyProxyClientSocket::RunWriteCallback
+
+OnClose() could consume `write_callback_` so it may not be available
+when RunWriteCallback() is invoked.
+
+Bug: 1428820
+Change-Id: I9a5ade62d67f5bf15e12d0915d1ad6098657ffd4
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4437791
+Code-Coverage: Findit <[email protected]>
+Reviewed-by: Adam Rice <[email protected]>
+Commit-Queue: Kenichi Ishibashi <[email protected]>
+Cr-Commit-Position: refs/heads/main@{#1131689}
+
+diff --git a/net/spdy/spdy_proxy_client_socket.cc b/net/spdy/spdy_proxy_client_socket.cc
+index d9b67febc27cc99e5b3383a372451345cec6daaa..bdcf24a1cb65f5df291bd91784d68aa9c05e7b0d 100644
+--- a/net/spdy/spdy_proxy_client_socket.cc
++++ b/net/spdy/spdy_proxy_client_socket.cc
+@@ -278,10 +278,11 @@ int SpdyProxyClientSocket::GetLocalAddress(IPEndPoint* address) const {
+ }
+ 
+ void SpdyProxyClientSocket::RunWriteCallback(int result) {
+-  CHECK(write_callback_);
+-
+   base::WeakPtr<SpdyProxyClientSocket> weak_ptr = weak_factory_.GetWeakPtr();
+-  std::move(write_callback_).Run(result);
++  // `write_callback_` might be consumed by OnClose().
++  if (write_callback_) {
++    std::move(write_callback_).Run(result);
++  }
+   if (!weak_ptr) {
+     // `this` was already destroyed while running `write_callback_`. Must
+     // return immediately without touching any field member.

+ 41 - 0
patches/chromium/merge_m112_check_spdyproxyclientsocket_is_alive_after_write.patch

@@ -0,0 +1,41 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Kenichi Ishibashi <[email protected]>
+Date: Sat, 8 Apr 2023 04:16:50 +0000
+Subject: Check SpdyProxyClientSocket is alive after write callback
+
+To ensure that we don't use any member field.
+
+(cherry picked from commit b71541b22ca19d5c3a7c01fedffe521b26577b72)
+
+Bug: 1428820
+Change-Id: Icf6677c652a47dc2fd2d01675e94cda031a015f2
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4394863
+Reviewed-by: Adam Rice <[email protected]>
+Commit-Queue: Kenichi Ishibashi <[email protected]>
+Cr-Original-Commit-Position: refs/heads/main@{#1125634}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4410322
+Commit-Queue: Rubber Stamper <[email protected]>
+Auto-Submit: Kenichi Ishibashi <[email protected]>
+Bot-Commit: Rubber Stamper <[email protected]>
+Cr-Commit-Position: refs/branch-heads/5615@{#1172}
+Cr-Branched-From: 9c6408ef696e83a9936b82bbead3d41c93c82ee4-refs/heads/main@{#1109224}
+
+diff --git a/net/spdy/spdy_proxy_client_socket.cc b/net/spdy/spdy_proxy_client_socket.cc
+index 173fc5cdbb67958cc9ca43284ff989196b287a8e..d9b67febc27cc99e5b3383a372451345cec6daaa 100644
+--- a/net/spdy/spdy_proxy_client_socket.cc
++++ b/net/spdy/spdy_proxy_client_socket.cc
+@@ -279,7 +279,14 @@ int SpdyProxyClientSocket::GetLocalAddress(IPEndPoint* address) const {
+ 
+ void SpdyProxyClientSocket::RunWriteCallback(int result) {
+   CHECK(write_callback_);
++
++  base::WeakPtr<SpdyProxyClientSocket> weak_ptr = weak_factory_.GetWeakPtr();
+   std::move(write_callback_).Run(result);
++  if (!weak_ptr) {
++    // `this` was already destroyed while running `write_callback_`. Must
++    // return immediately without touching any field member.
++    return;
++  }
+ 
+   if (end_stream_state_ == EndStreamState::kEndStreamReceived) {
+     base::ThreadTaskRunnerHandle::Get()->PostTask(

+ 94 - 0
patches/chromium/merge_m112_remove_the_second_weakptrfactory_from.patch

@@ -0,0 +1,94 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Kenichi Ishibashi <[email protected]>
+Date: Sat, 8 Apr 2023 01:56:25 +0000
+Subject: Remove the second WeakPtrFactory from SpdyProxyClientSocket
+
+It was introduced [1] to work around an old issue that wouldn't happen
+any more since we store a write callback in the class. Instead of having
+the second WeakPtrFactory and moving the callback, we can just keep it
+until RunWriteCallback() is called.
+
+This is a speculative fix for the linked bug.
+
+[1] https://codereview.chromium.org/338583003/
+
+(cherry picked from commit 01b25615896b911e21103dd381fafc1f85886d91)
+
+Bug: 1428820
+Change-Id: I0b5af2675b68188e208c2ecd42293251b2722b28
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4393905
+Reviewed-by: Adam Rice <[email protected]>
+Commit-Queue: Kenichi Ishibashi <[email protected]>
+Cr-Original-Commit-Position: refs/heads/main@{#1125216}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4410320
+Auto-Submit: Kenichi Ishibashi <[email protected]>
+Bot-Commit: Rubber Stamper <[email protected]>
+Commit-Queue: Rubber Stamper <[email protected]>
+Cr-Commit-Position: refs/branch-heads/5615@{#1171}
+Cr-Branched-From: 9c6408ef696e83a9936b82bbead3d41c93c82ee4-refs/heads/main@{#1109224}
+
+diff --git a/net/spdy/spdy_proxy_client_socket.cc b/net/spdy/spdy_proxy_client_socket.cc
+index 5984ecedb3a14274bc33efe20b407dd33bc8a410..173fc5cdbb67958cc9ca43284ff989196b287a8e 100644
+--- a/net/spdy/spdy_proxy_client_socket.cc
++++ b/net/spdy/spdy_proxy_client_socket.cc
+@@ -123,7 +123,6 @@ void SpdyProxyClientSocket::Disconnect() {
+ 
+   write_buffer_len_ = 0;
+   write_callback_.Reset();
+-  write_callback_weak_factory_.InvalidateWeakPtrs();
+ 
+   next_state_ = STATE_DISCONNECTED;
+ 
+@@ -278,9 +277,9 @@ int SpdyProxyClientSocket::GetLocalAddress(IPEndPoint* address) const {
+   return spdy_stream_->GetLocalAddress(address);
+ }
+ 
+-void SpdyProxyClientSocket::RunWriteCallback(CompletionOnceCallback callback,
+-                                             int result) const {
+-  std::move(callback).Run(result);
++void SpdyProxyClientSocket::RunWriteCallback(int result) {
++  CHECK(write_callback_);
++  std::move(write_callback_).Run(result);
+ 
+   if (end_stream_state_ == EndStreamState::kEndStreamReceived) {
+     base::ThreadTaskRunnerHandle::Get()->PostTask(
+@@ -517,8 +516,7 @@ void SpdyProxyClientSocket::OnDataSent() {
+   // stream's write callback chain to unwind (see crbug.com/355511).
+   base::ThreadTaskRunnerHandle::Get()->PostTask(
+       FROM_HERE, base::BindOnce(&SpdyProxyClientSocket::RunWriteCallback,
+-                                write_callback_weak_factory_.GetWeakPtr(),
+-                                std::move(write_callback_), rv));
++                                weak_factory_.GetWeakPtr(), rv));
+ }
+ 
+ void SpdyProxyClientSocket::OnTrailers(const spdy::Http2HeaderBlock& trailers) {
+diff --git a/net/spdy/spdy_proxy_client_socket.h b/net/spdy/spdy_proxy_client_socket.h
+index cbb961a68c93b9485bd0bf1c5e31014bdc95a84e..98b9e74427f48399bbc53bfad5b45a8dc18ab84b 100644
+--- a/net/spdy/spdy_proxy_client_socket.h
++++ b/net/spdy/spdy_proxy_client_socket.h
+@@ -121,9 +121,9 @@ class NET_EXPORT_PRIVATE SpdyProxyClientSocket : public ProxyClientSocket,
+     STATE_CLOSED
+   };
+ 
+-  // Calls |callback.Run(result)|. Used to run a callback posted to the
++  // Calls `write_callback_(result)`. Used to run a callback posted to the
+   // message loop.
+-  void RunWriteCallback(CompletionOnceCallback callback, int result) const;
++  void RunWriteCallback(int result);
+ 
+   void OnIOComplete(int result);
+ 
+@@ -195,13 +195,7 @@ class NET_EXPORT_PRIVATE SpdyProxyClientSocket : public ProxyClientSocket,
+   };
+   EndStreamState end_stream_state_ = EndStreamState::kNone;
+ 
+-  // The default weak pointer factory.
+   base::WeakPtrFactory<SpdyProxyClientSocket> weak_factory_{this};
+-
+-  // Only used for posting write callbacks. Weak pointers created by this
+-  // factory are invalidated in Disconnect().
+-  base::WeakPtrFactory<SpdyProxyClientSocket> write_callback_weak_factory_{
+-      this};
+ };
+ 
+ }  // namespace net