Browse Source

chore: cherry-pick 5 changes from Release-0-M116 (#39558)

* chore: [24-x-y] cherry-pick 3 changes from Release-0-M116

* 8ff63d378f2c from v8
* 5486190be556 from angle
* d671b099a57d from v8

* chore: update patches

* chore: cherry-pick missing changes

---------

Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
Pedro Pontes 1 year ago
parent
commit
f22a5b80eb

+ 1 - 0
patches/angle/.patches

@@ -1 +1,2 @@
 fix_rename_webswapcgllayer_to_webswapcgllayerchromium.patch
+cherry-pick-5486190be556.patch

+ 41 - 0
patches/angle/cherry-pick-5486190be556.patch

@@ -0,0 +1,41 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Geoff Lang <[email protected]>
+Date: Fri, 21 Jul 2023 13:45:52 -0400
+Subject: Fix read size validation for RGBX formats.
+
+GL_RGBX8_ANGLE is the only format where the upload format is 3-channel
+RGB, whilethe download format is 4-channel RGBX.  As such, the internal
+format corresponding to format+type expects 3-byte input/output.  The
+format is fixed here for readPixels to output 4 bytes per pixel.
+
+Bug: chromium:1458046
+Change-Id: Iec737ed64bade003cfab50dc5f595eb4875e81e4
+Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4706957
+Commit-Queue: Shahbaz Youssefi <[email protected]>
+(cherry picked from commit 430a4f559cbc2bcd5d026e8b36ee46ddd80e9651)
+Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4765136
+Commit-Queue: Daniel Yip <[email protected]>
+Auto-Submit: Daniel Yip <[email protected]>
+(cherry picked from commit 4a372ad49ceddea6c13f79adb212a777ec770a66)
+
+diff --git a/src/libANGLE/formatutils.cpp b/src/libANGLE/formatutils.cpp
+index 0b8d4eeb537690da830a0d7162e86ea1379c42b4..9e3007fa12189aa8a1bebb33e869cf5f597e04a9 100644
+--- a/src/libANGLE/formatutils.cpp
++++ b/src/libANGLE/formatutils.cpp
+@@ -1715,7 +1715,15 @@ const InternalFormat &GetInternalFormatInfo(GLenum internalFormat, GLenum type)
+ GLuint InternalFormat::computePixelBytes(GLenum formatType) const
+ {
+     const auto &typeInfo = GetTypeInfo(formatType);
+-    GLuint components    = typeInfo.specialInterpretation ? 1u : componentCount;
++    GLuint components    = componentCount;
++    if (sizedInternalFormat == GL_RGBX8_ANGLE)
++    {
++        components = 4;
++    }
++    else if (typeInfo.specialInterpretation)
++    {
++        components = 1;
++    }
+     return components * typeInfo.bytes;
+ }
+ 

+ 1 - 0
patches/chromium/.patches

@@ -139,3 +139,4 @@ cherry-pick-933b9fad3a53.patch
 cherry-pick-b03973561862.patch
 cherry-pick-c60a1ab717c7.patch
 cherry-pick-aa23556ff213.patch
+networkcontext_don_t_access_url_loader_factories_during_destruction.patch

+ 93 - 0
patches/chromium/networkcontext_don_t_access_url_loader_factories_during_destruction.patch

@@ -0,0 +1,93 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Adam Rice <[email protected]>
+Date: Tue, 8 Aug 2023 08:48:51 +0000
+Subject: NetworkContext: Don't access url_loader_factories_ during destruction
+
+Move the contents of `url_loader_factories_` to a temporary variable in
+the destructor of network::NetworkContext so that re-entrant calls to
+DestroyURLLoaderFactory() don't happen after it has started being
+destroyed.
+
+BUG=1465833
+
+(cherry picked from commit e579b20308290df03f045c5d0ccb852d96b24ce3)
+
+Change-Id: I476f0865256bdcba4ec934688597e69991968f84
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4733351
+Reviewed-by: Kenichi Ishibashi <[email protected]>
+Commit-Queue: Adam Rice <[email protected]>
+Cr-Original-Commit-Position: refs/heads/main@{#1177648}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4756334
+Bot-Commit: Rubber Stamper <[email protected]>
+Auto-Submit: Adam Rice <[email protected]>
+Commit-Queue: Rubber Stamper <[email protected]>
+Cr-Commit-Position: refs/branch-heads/5845@{#1252}
+Cr-Branched-From: 5a5dff63a4a4c63b9b18589819bebb2566c85443-refs/heads/main@{#1160321}
+
+diff --git a/services/network/network_context.cc b/services/network/network_context.cc
+index 97fa9e6e96fde894feb7da28afca6f65a310199d..d3cda420386ce946568fdc2656308d9c3cf3b610 100644
+--- a/services/network/network_context.cc
++++ b/services/network/network_context.cc
+@@ -692,6 +692,8 @@ NetworkContext::NetworkContext(
+ }
+ 
+ NetworkContext::~NetworkContext() {
++  is_destructing_ = true;
++
+   // May be nullptr in tests.
+   if (network_service_)
+     network_service_->DeregisterNetworkContext(this);
+@@ -749,6 +751,12 @@ NetworkContext::~NetworkContext() {
+     }
+   }
+ #endif  // BUILDFLAG(IS_DIRECTORY_TRANSFER_REQUIRED)
++
++  // Clear `url_loader_factories_` before deleting the contents, as it can
++  // result in re-entrant calls to DestroyURLLoaderFactory().
++  std::set<std::unique_ptr<cors::CorsURLLoaderFactory>,
++           base::UniquePtrComparator>
++      url_loader_factories = std::move(url_loader_factories_);
+ }
+ 
+ // static
+@@ -974,6 +982,9 @@ void NetworkContext::DisableQuic() {
+ 
+ void NetworkContext::DestroyURLLoaderFactory(
+     cors::CorsURLLoaderFactory* url_loader_factory) {
++  if (is_destructing_) {
++    return;
++  }
+   auto it = url_loader_factories_.find(url_loader_factory);
+   DCHECK(it != url_loader_factories_.end());
+   url_loader_factories_.erase(it);
+diff --git a/services/network/network_context.h b/services/network/network_context.h
+index 06cd75b4aa3ee7a582fe9a2b2cd288cb61b3c19e..fd3655943b1b1caf8a6db35219f437113163276a 100644
+--- a/services/network/network_context.h
++++ b/services/network/network_context.h
+@@ -915,6 +915,10 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext
+   // according to the spec.
+   bool acam_preflight_spec_conformant_ = true;
+ 
++  // True once the destructor has been called. Used to guard against re-entrant
++  // calls to DestroyURLLoaderFactory().
++  bool is_destructing_ = false;
++
+   // Indicating whether
+   // https://fetch.spec.whatwg.org/#cors-non-wildcard-request-header-name is
+   // supported.
+@@ -923,13 +927,8 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkContext
+ 
+   // CorsURLLoaderFactory assumes that fields owned by the NetworkContext always
+   // live longer than the factory.  Therefore we want the factories to be
+-  // destroyed before other fields above.  In particular:
+-  // - This must be below |url_request_context_| so that the URLRequestContext
+-  //   outlives all the URLLoaderFactories and URLLoaders that depend on it;
+-  //   for the same reason, it must also be below |network_context_|.
+-  // - This must be below |loader_count_per_process_| that is touched by
+-  //   CorsURLLoaderFactory::DestroyURLLoader (see also
+-  //   https://crbug.com/1174943).
++  // destroyed before other fields above.  This is accomplished by explicitly
++  // clearing `url_loader_factories_` in the destructor.
+   std::set<std::unique_ptr<cors::CorsURLLoaderFactory>,
+            base::UniquePtrComparator>
+       url_loader_factories_;

+ 3 - 1
patches/config.json

@@ -25,5 +25,7 @@
 
   "src/electron/patches/webrtc": "src/third_party/webrtc",
 
-  "src/electron/patches/pdfium": "src/third_party/pdfium"
+  "src/electron/patches/pdfium": "src/third_party/pdfium",
+
+  "src/electron/patches/skia": "src/third_party/skia"
 }

+ 1 - 0
patches/skia/.patches

@@ -0,0 +1 @@
+enforce_an_upper_limit_of_715_million_path_verbs_in_skpath.patch

+ 118 - 0
patches/skia/enforce_an_upper_limit_of_715_million_path_verbs_in_skpath.patch

@@ -0,0 +1,118 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: John Stiles <[email protected]>
+Date: Thu, 3 Aug 2023 13:33:52 -0400
+Subject: Enforce an upper limit of 715 million path verbs in SkPath.
+
+Bug: chromium:1464215
+Change-Id: Iedb7d73fc80de5ffb881b664dd77314cc2c6b108
+Reviewed-on: https://skia-review.googlesource.com/c/skia/+/735316
+Reviewed-by: Brian Osman <[email protected]>
+Commit-Queue: John Stiles <[email protected]>
+
+diff --git a/relnotes/path_715M.md b/relnotes/path_715M.md
+new file mode 100644
+index 0000000000000000000000000000000000000000..7be9a40f1fc5b4f6432c490725b05d536d497fb1
+--- /dev/null
++++ b/relnotes/path_715M.md
+@@ -0,0 +1 @@
++SkPath now enforces an upper limit of 715 million path verbs.
+diff --git a/src/core/SkPath.cpp b/src/core/SkPath.cpp
+index 6cc1e2ef1b49b18aa6692dba655ef5ca83610b0d..976edee87e763a1f44396902d4f641ed3da8e452 100644
+--- a/src/core/SkPath.cpp
++++ b/src/core/SkPath.cpp
+@@ -34,6 +34,7 @@
+ #include <cmath>
+ #include <cstring>
+ #include <iterator>
++#include <limits.h>
+ #include <utility>
+ 
+ struct SkPath_Storage_Equivalent {
+@@ -3421,43 +3422,52 @@ bool SkPath::IsCubicDegenerate(const SkPoint& p1, const SkPoint& p2,
+ 
+ SkPathVerbAnalysis sk_path_analyze_verbs(const uint8_t vbs[], int verbCount) {
+     SkPathVerbAnalysis info = {false, 0, 0, 0};
+-
+     bool needMove = true;
+     bool invalid = false;
+-    for (int i = 0; i < verbCount; ++i) {
+-        switch ((SkPathVerb)vbs[i]) {
+-            case SkPathVerb::kMove:
+-                needMove = false;
+-                info.points += 1;
+-                break;
+-            case SkPathVerb::kLine:
+-                invalid |= needMove;
+-                info.segmentMask |= kLine_SkPathSegmentMask;
+-                info.points += 1;
+-                break;
+-            case SkPathVerb::kQuad:
+-                invalid |= needMove;
+-                info.segmentMask |= kQuad_SkPathSegmentMask;
+-                info.points += 2;
+-                break;
+-            case SkPathVerb::kConic:
+-                invalid |= needMove;
+-                info.segmentMask |= kConic_SkPathSegmentMask;
+-                info.points += 2;
+-                info.weights += 1;
+-                break;
+-            case SkPathVerb::kCubic:
+-                invalid |= needMove;
+-                info.segmentMask |= kCubic_SkPathSegmentMask;
+-                info.points += 3;
+-                break;
+-            case SkPathVerb::kClose:
+-                invalid |= needMove;
+-                needMove = true;
+-                break;
+-            default:
+-                invalid = true;
+-                break;
++
++    if (verbCount >= (INT_MAX / 3)) {
++        // A path with an extremely high number of quad, conic or cubic verbs could cause
++        // `info.points` to overflow. To prevent against this, we reject extremely large paths. This
++        // check is conservative and assumes the worst case (in particular, it assumes that every
++        // verb consumes 3 points, which would only happen for a path composed entirely of cubics).
++        // This limits us to 700 million verbs, which is large enough for any reasonable use case.
++        invalid = true;
++    } else {
++        for (int i = 0; i < verbCount; ++i) {
++            switch ((SkPathVerb)vbs[i]) {
++                case SkPathVerb::kMove:
++                    needMove = false;
++                    info.points += 1;
++                    break;
++                case SkPathVerb::kLine:
++                    invalid |= needMove;
++                    info.segmentMask |= kLine_SkPathSegmentMask;
++                    info.points += 1;
++                    break;
++                case SkPathVerb::kQuad:
++                    invalid |= needMove;
++                    info.segmentMask |= kQuad_SkPathSegmentMask;
++                    info.points += 2;
++                    break;
++                case SkPathVerb::kConic:
++                    invalid |= needMove;
++                    info.segmentMask |= kConic_SkPathSegmentMask;
++                    info.points += 2;
++                    info.weights += 1;
++                    break;
++                case SkPathVerb::kCubic:
++                    invalid |= needMove;
++                    info.segmentMask |= kCubic_SkPathSegmentMask;
++                    info.points += 3;
++                    break;
++                case SkPathVerb::kClose:
++                    invalid |= needMove;
++                    needMove = true;
++                    break;
++                default:
++                    invalid = true;
++                    break;
++            }
+         }
+     }
+     info.valid = !invalid;

+ 2 - 0
patches/v8/.patches

@@ -16,3 +16,5 @@ cherry-pick-0035a4a8dac2.patch
 cherry-pick-2e76270cf65e.patch
 merged_runtime_set_instance_prototypes_directly_on_maps.patch
 merged_compiler_stackcheck_can_have_side_effects.patch
+cherry-pick-8ff63d378f2c.patch
+cherry-pick-d671b099a57d.patch

+ 126 - 0
patches/v8/cherry-pick-8ff63d378f2c.patch

@@ -0,0 +1,126 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Shu-yu Guo <[email protected]>
+Date: Wed, 2 Aug 2023 17:41:03 -0700
+Subject: Merged: [builtins] Clear FixedArray slot in Promise builtins
+
+Fixed: chromium:1468943
+(cherry picked from commit a84849ed718932b94dc877bb44a2d38eb8a0aef9)
+
+Change-Id: Ia2b181c373c15bd1840e2a1572c0e930cddcd788
+Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4753495
+Commit-Queue: Adam Klein <[email protected]>
+Reviewed-by: Adam Klein <[email protected]>
+Auto-Submit: Shu-yu Guo <[email protected]>
+Cr-Commit-Position: refs/branch-heads/11.6@{#28}
+Cr-Branched-From: e29c028f391389a7a60ee37097e3ca9e396d6fa4-refs/heads/11.6.189@{#3}
+Cr-Branched-From: 95cbef20e2aa556a1ea75431a48b36c4de6b9934-refs/heads/main@{#88340}
+
+diff --git a/src/builtins/promise-all-element-closure.tq b/src/builtins/promise-all-element-closure.tq
+index db3fb0134cf5bf0065174153171ef44a726a6fff..036e3c7b7473eae98f39a6da4472e826420086c8 100644
+--- a/src/builtins/promise-all-element-closure.tq
++++ b/src/builtins/promise-all-element-closure.tq
+@@ -175,11 +175,22 @@ transitioning macro PromiseAllResolveElementClosure<F: type>(
+         *NativeContextSlot(
+         nativeContext, ContextSlot::JS_ARRAY_PACKED_ELEMENTS_MAP_INDEX);
+ 
+-    // If resolve and reject handlers close over values to keep track of whether
+-    // an input promise is already settled, mark the values array as COW before
+-    // letting it escape to user code.
+-    if (hasResolveAndRejectClosures) MakeFixedArrayCOW(values);
+-
++    // After this point, values escapes to user code.
++    //
++    // If resolve and reject handlers close over values to keep track of
++    // whether an input promise is already settled, mark the values array as
++    // COW. The original values array is still needed to guard against resolve
++    // or reject being called multiple times for an element.
++    //
++    // Otherwise, clear the slot.
++    if (hasResolveAndRejectClosures) {
++      MakeFixedArrayCOW(values);
++    } else {
++      *ContextSlot(
++          promiseContext,
++          PromiseAllResolveElementContextSlots::
++              kPromiseAllResolveElementValuesSlot) = kEmptyFixedArray;
++    }
+     const valuesArray = NewJSArray(arrayMap, values);
+     Call(promiseContext, resolve, Undefined, valuesArray);
+   }
+diff --git a/src/builtins/promise-all.tq b/src/builtins/promise-all.tq
+index 4d131abb44b7593d3e361d1d2f971380ee91850c..7205279526fa9edd93f154feb9694c22cae68606 100644
+--- a/src/builtins/promise-all.tq
++++ b/src/builtins/promise-all.tq
+@@ -278,15 +278,16 @@ Reject(JSAny) {
+ 
+   check(remainingElementsCount >= 0);
+ 
++  const valuesRef:&FixedArray = ContextSlot(
++      resolveElementContext,
++      PromiseAllResolveElementContextSlots::
++          kPromiseAllResolveElementValuesSlot);
++  const values = *valuesRef;
++
+   if (remainingElementsCount > 0) {
+     // Pre-allocate the backing store for the {values} to the desired
+     // capacity. We may already have elements in "values" - this happens
+     // when the Thenable calls the resolve callback immediately.
+-    const valuesRef:&FixedArray = ContextSlot(
+-        resolveElementContext,
+-        PromiseAllResolveElementContextSlots::
+-            kPromiseAllResolveElementValuesSlot);
+-    const values = *valuesRef;
+     // 'index' is a 1-based index and incremented after every Promise. Later we
+     // use 'values' as a 0-based array, so capacity 'index - 1' is enough.
+     const newCapacity = SmiUntag(index) - 1;
+@@ -301,19 +302,23 @@ Reject(JSAny) {
+       //     Let valuesArray be CreateArrayFromList(values).
+       //     Perform ? Call(resultCapability.[[Resolve]], undefined,
+       //                    « valuesArray »).
+-
+-      const values: FixedArray = *ContextSlot(
+-          resolveElementContext,
+-          PromiseAllResolveElementContextSlots::
+-              kPromiseAllResolveElementValuesSlot);
+       const arrayMap =
+           *NativeContextSlot(
+           nativeContext, ContextSlot::JS_ARRAY_PACKED_ELEMENTS_MAP_INDEX);
+ 
++      // After this point, values escapes to user code.
++      //
+       // If resolve and reject handlers close over values to keep track of
+       // whether an input promise is already settled, mark the values array as
+-      // COW before letting it escape to user code.
+-      if (hasResolveAndRejectClosures) MakeFixedArrayCOW(values);
++      // COW. The original values array is still needed to guard against resolve
++      // or reject being called multiple times for an element.
++      //
++      // Otherwise, clear the slot.
++      if (hasResolveAndRejectClosures) {
++        MakeFixedArrayCOW(values);
++      } else {
++        *valuesRef = kEmptyFixedArray;
++      }
+ 
+       const valuesArray = NewJSArray(arrayMap, values);
+       Call(nativeContext, UnsafeCast<JSAny>(resolve), Undefined, valuesArray);
+diff --git a/src/builtins/promise-any.tq b/src/builtins/promise-any.tq
+index 7e707e649f11bc946a6d1173180d7293fe94d8ce..45bafac0e6b09143b69b21a7292f9ed6b9c46239 100644
+--- a/src/builtins/promise-any.tq
++++ b/src/builtins/promise-any.tq
+@@ -313,10 +313,14 @@ Reject(JSAny) {
+ 
+       // We may already have elements in "errors" - this happens when the
+       // Thenable calls the reject callback immediately.
+-      const errors: FixedArray = *ContextSlot(
++      const errorsRef:&FixedArray = ContextSlot(
+           rejectElementContext,
+           PromiseAnyRejectElementContextSlots::
+               kPromiseAnyRejectElementErrorsSlot);
++      const errors: FixedArray = *errorsRef;
++
++      // After this point, errors escapes to user code. Clear the slot.
++      *errorsRef = kEmptyFixedArray;
+ 
+       check(errors.length == index - 1);
+       const error = ConstructAggregateError(errors);

+ 31 - 0
patches/v8/cherry-pick-d671b099a57d.patch

@@ -0,0 +1,31 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Tobias Tebbi <[email protected]>
+Date: Tue, 13 Jun 2023 11:50:11 +0200
+Subject: Merged: [compiler] check for read-only property on
+ AccessMode::kDefine
+
+(cherry picked from commit 95eda07ddbc1839e39b3d1ff5db329c0ceb6dfaa)
+
+Change-Id: Ic5799a32e7c312e41cd0a7e1e6073a235414d56f
+Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4665588
+Auto-Submit: Tobias Tebbi <[email protected]>
+Reviewed-by: Darius Mercadier <[email protected]>
+Commit-Queue: Darius Mercadier <[email protected]>
+Cr-Commit-Position: refs/branch-heads/11.4@{#53}
+Cr-Branched-From: 8a8a1e7086dacc426965d3875914efa66663c431-refs/heads/11.4.183@{#1}
+Cr-Branched-From: 5483d8e816e0bbce865cbbc3fa0ab357e6330bab-refs/heads/main@{#87241}
+
+diff --git a/src/compiler/access-info.cc b/src/compiler/access-info.cc
+index 3120077a7710264926b9f9cce348cc3f7c0ac858..fc1bfcf2cd6c24ca9ddf8a95ef6a48b087ed3fde 100644
+--- a/src/compiler/access-info.cc
++++ b/src/compiler/access-info.cc
+@@ -749,8 +749,7 @@ PropertyAccessInfo AccessInfoFactory::ComputePropertyAccessInfo(
+     }
+ 
+     if (index.is_found()) {
+-      if (access_mode == AccessMode::kStore ||
+-          access_mode == AccessMode::kStoreInLiteral) {
++      if (IsAnyStore(access_mode)) {
+         DCHECK(!map.is_dictionary_map());
+ 
+         // Don't bother optimizing stores to read-only properties.