Browse Source

chore: cherry-pick 8 changes from Release-1-M113 (#38329)

* chore: [25-x-y] cherry-pick 8 changes from Release-1-M113

* 91fce3345668 from v8
* 2c8a019f39d2 from v8
* b8020e1973d7 from v8
* d6272b794cbb from chromium
* 48785f698b1c from chromium
* d0ee0197ddff from angle
* 9b6ca211234b from chromium
* 675562695049 from chromium

* chore: update patches

* chore: check test failure

* build: revert patch causing test failures

---------

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

+ 1 - 0
patches/angle/.patches

@@ -0,0 +1 @@
+cherry-pick-d0ee0197ddff.patch

+ 208 - 0
patches/angle/cherry-pick-d0ee0197ddff.patch

@@ -0,0 +1,208 @@
+From d0ee0197ddff25fe1a9876511c07542ac483702d Mon Sep 17 00:00:00 2001
+From: Shahbaz Youssefi <[email protected]>
+Date: Wed, 03 May 2023 13:41:36 -0400
+Subject: [PATCH] WebGL: Limit total size of private data
+
+... not just individual arrays.
+
+Bug: chromium:1431761
+Change-Id: I721e29aeceeaf12c3f6a67b668abffb8dfbc89b0
+Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/4503753
+Reviewed-by: Kenneth Russell <[email protected]>
+Commit-Queue: Shahbaz Youssefi <[email protected]>
+---
+
+diff --git a/src/compiler/translator/ValidateTypeSizeLimitations.cpp b/src/compiler/translator/ValidateTypeSizeLimitations.cpp
+index 6097b6d..2a033ad 100644
+--- a/src/compiler/translator/ValidateTypeSizeLimitations.cpp
++++ b/src/compiler/translator/ValidateTypeSizeLimitations.cpp
+@@ -35,7 +35,9 @@
+ {
+   public:
+     ValidateTypeSizeLimitationsTraverser(TSymbolTable *symbolTable, TDiagnostics *diagnostics)
+-        : TIntermTraverser(true, false, false, symbolTable), mDiagnostics(diagnostics)
++        : TIntermTraverser(true, false, false, symbolTable),
++          mDiagnostics(diagnostics),
++          mTotalPrivateVariablesSize(0)
+     {
+         ASSERT(diagnostics);
+     }
+@@ -93,18 +95,33 @@
+             const bool isPrivate = variableType.getQualifier() == EvqTemporary ||
+                                    variableType.getQualifier() == EvqGlobal ||
+                                    variableType.getQualifier() == EvqConst;
+-            if (layoutEncoder.getCurrentOffset() > kMaxPrivateVariableSizeInBytes && isPrivate)
++            if (isPrivate)
+             {
+-                error(asSymbol->getLine(),
+-                      "Size of declared private variable exceeds implementation-defined limit",
+-                      asSymbol->getName());
+-                return false;
++                if (layoutEncoder.getCurrentOffset() > kMaxPrivateVariableSizeInBytes)
++                {
++                    error(asSymbol->getLine(),
++                          "Size of declared private variable exceeds implementation-defined limit",
++                          asSymbol->getName());
++                    return false;
++                }
++                mTotalPrivateVariablesSize += layoutEncoder.getCurrentOffset();
+             }
+         }
+ 
+         return true;
+     }
+ 
++    void validateTotalPrivateVariableSize()
++    {
++        if (mTotalPrivateVariablesSize > kMaxPrivateVariableSizeInBytes)
++        {
++            mDiagnostics->error(
++                TSourceLoc{},
++                "Total size of declared private variables exceeds implementation-defined limit",
++                "");
++        }
++    }
++
+   private:
+     void error(TSourceLoc loc, const char *reason, const ImmutableString &token)
+     {
+@@ -213,6 +230,8 @@
+ 
+     TDiagnostics *mDiagnostics;
+     std::vector<int> mLoopSymbolIds;
++
++    size_t mTotalPrivateVariablesSize;
+ };
+ 
+ }  // namespace
+@@ -223,6 +242,7 @@
+ {
+     ValidateTypeSizeLimitationsTraverser validate(symbolTable, diagnostics);
+     root->traverse(&validate);
++    validate.validateTotalPrivateVariableSize();
+     return diagnostics->numErrors() == 0;
+ }
+ 
+diff --git a/src/tests/gl_tests/WebGLCompatibilityTest.cpp b/src/tests/gl_tests/WebGLCompatibilityTest.cpp
+index e62c8fb..996bad1 100644
+--- a/src/tests/gl_tests/WebGLCompatibilityTest.cpp
++++ b/src/tests/gl_tests/WebGLCompatibilityTest.cpp
+@@ -5271,11 +5271,12 @@
+     // fairly small array.
+     constexpr char kVSArrayOK[] =
+         R"(varying vec4 color;
+-const int array_size = 1000;
++const int array_size = 500;
+ void main()
+ {
+     mat2 array[array_size];
+-    if (array[0][0][0] == 2.0)
++    mat2 array2[array_size];
++    if (array[0][0][0] + array2[0][0][0] == 2.0)
+         color = vec4(0.0, 1.0, 0.0, 1.0);
+     else
+         color = vec4(1.0, 0.0, 0.0, 1.0);
+@@ -5353,6 +5354,103 @@
+     EXPECT_EQ(0u, program);
+ }
+ 
++// Reject attempts to allocate too much private memory.
++// This is an implementation-defined limit - crbug.com/1431761.
++TEST_P(WebGLCompatibilityTest, ValidateTotalPrivateSize)
++{
++    constexpr char kTooLargeGlobalMemory1[] =
++        R"(precision mediump float;
++
++// 1 MB / 16 bytes per vec4 = 65536
++vec4 array[32768];
++vec4 array2[32769];
++
++void main()
++{
++    if (array[0].x + array[1].x == 0.)
++        gl_FragColor = vec4(0.0, 1.0, 0.0, 1.0);
++    else
++        gl_FragColor = vec4(1.0, 0.0, 0.0, 1.0);
++})";
++
++    constexpr char kTooLargeGlobalMemory2[] =
++        R"(precision mediump float;
++
++// 1 MB / 16 bytes per vec4 = 65536
++vec4 array[32767];
++vec4 array2[32767];
++vec4 x, y, z;
++
++void main()
++{
++    if (array[0].x + array[1].x == x.w + y.w + z.w)
++        gl_FragColor = vec4(0.0, 1.0, 0.0, 1.0);
++    else
++        gl_FragColor = vec4(1.0, 0.0, 0.0, 1.0);
++})";
++
++    constexpr char kTooLargeGlobalAndLocalMemory1[] =
++        R"(precision mediump float;
++
++// 1 MB / 16 bytes per vec4 = 65536
++vec4 array[32768];
++
++void main()
++{
++    vec4 array2[32769];
++    if (array[0].x + array[1].x == 2.0)
++        gl_FragColor = vec4(0.0, 1.0, 0.0, 1.0);
++    else
++        gl_FragColor = vec4(1.0, 0.0, 0.0, 1.0);
++})";
++
++    // Note: The call stack is not taken into account for the purposes of total memory calculation.
++    constexpr char kTooLargeGlobalAndLocalMemory2[] =
++        R"(precision mediump float;
++
++// 1 MB / 16 bytes per vec4 = 65536
++vec4 array[32768];
++
++float f()
++{
++    vec4 array2[16384];
++    return array2[0].x;
++}
++
++float g()
++{
++    vec4 array3[16383];
++    return array3[0].x;
++}
++
++float h()
++{
++    vec4 value;
++    float value2
++    return value.x + value2;
++}
++
++void main()
++{
++    if (array[0].x + f() + g() + h() == 2.0)
++        gl_FragColor = vec4(0.0, 1.0, 0.0, 1.0);
++    else
++        gl_FragColor = vec4(1.0, 0.0, 0.0, 1.0);
++})";
++
++    GLuint program = CompileProgram(essl1_shaders::vs::Simple(), kTooLargeGlobalMemory1);
++    EXPECT_EQ(0u, program);
++
++    program = CompileProgram(essl1_shaders::vs::Simple(), kTooLargeGlobalMemory2);
++    EXPECT_EQ(0u, program);
++
++    program = CompileProgram(essl1_shaders::vs::Simple(), kTooLargeGlobalAndLocalMemory1);
++    EXPECT_EQ(0u, program);
++
++    program = CompileProgram(essl1_shaders::vs::Simple(), kTooLargeGlobalAndLocalMemory2);
++    EXPECT_EQ(0u, program);
++}
++
+ // Linking should fail when corresponding vertex/fragment uniform blocks have different precision
+ // qualifiers.
+ TEST_P(WebGL2CompatibilityTest, UniformBlockPrecisionMismatch)

+ 3 - 0
patches/chromium/.patches

@@ -128,3 +128,6 @@ add_gin_converter_support_for_arraybufferview.patch
 chore_defer_usb_service_getdevices_request_until_usb_service_is.patch
 cherry-pick-48a136e77e6d.patch
 cherry-pick-e6e23ba00379.patch
+cherry-pick-d6272b794cbb.patch
+cherry-pick-48785f698b1c.patch
+cherry-pick-675562695049.patch

+ 107 - 0
patches/chromium/cherry-pick-48785f698b1c.patch

@@ -0,0 +1,107 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Arthur Sonzogni <[email protected]>
+Date: Tue, 2 May 2023 09:40:37 +0000
+Subject: Avoid buffer overflow read in HFSReadNextNonIgnorableCodePoint
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+Unicode codepoints goes beyond 0xFFFF.
+
+It exists upper and lower case characters there: `𞤡 `vs `𞥃`.
+
+The buffer overflow occurred when using the lookup table:
+```
+lower_case_table[codepoint >> 8]
+```
+
+Bug: 1425115
+Fixed: 1425115
+Change-Id: I679da02dbe570283a68176fbd3c0c620caa4f9ce
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4481260
+Reviewed-by: Alexander Timin <[email protected]>
+Commit-Queue: Arthur Sonzogni <[email protected]>
+Cr-Commit-Position: refs/heads/main@{#1138234}
+
+diff --git a/base/files/file_path.cc b/base/files/file_path.cc
+index 12c684bed273721bb5b36f5441ed833485a1fe15..59bbc6e15da4b6cb531003fa6ec1fb197f985447 100644
+--- a/base/files/file_path.cc
++++ b/base/files/file_path.cc
+@@ -789,7 +789,7 @@ int FilePath::CompareIgnoreCase(StringPieceType string1,
+ #elif BUILDFLAG(IS_APPLE)
+ // Mac OS X specific implementation of file string comparisons.
+ 
+-// cf. http://developer.apple.com/mac/library/technotes/tn/tn1150.html#UnicodeSubtleties
++// cf. https://developer.apple.com/library/archive/technotes/tn/tn1150.html#UnicodeSubtleties
+ //
+ // "When using CreateTextEncoding to create a text encoding, you should set
+ // the TextEncodingBase to kTextEncodingUnicodeV2_0, set the
+@@ -815,11 +815,12 @@ int FilePath::CompareIgnoreCase(StringPieceType string1,
+ // Ignored characters are mapped to zero.
+ //
+ // cf. downloadable file linked in
+-// http://developer.apple.com/mac/library/technotes/tn/tn1150.html#StringComparisonAlgorithm
++// https://developer.apple.com/library/archive/technotes/tn/tn1150.html#Downloads
+ 
+ namespace {
+ 
+-const UInt16 lower_case_table[] = {
++// clang-format off
++const UInt16 lower_case_table[11 * 256] = {
+   // High-byte indices ( == 0 iff no case mapping and no ignorables )
+ 
+   /* 0 */ 0x0100, 0x0200, 0x0000, 0x0300, 0x0400, 0x0500, 0x0000, 0x0000,
+@@ -1205,11 +1206,12 @@ const UInt16 lower_case_table[] = {
+   /* F */ 0xFFF0, 0xFFF1, 0xFFF2, 0xFFF3, 0xFFF4, 0xFFF5, 0xFFF6, 0xFFF7,
+           0xFFF8, 0xFFF9, 0xFFFA, 0xFFFB, 0xFFFC, 0xFFFD, 0xFFFE, 0xFFFF,
+ };
++// clang-format on
+ 
+-// Returns the next non-ignorable codepoint within string starting from the
+-// position indicated by index, or zero if there are no more.
+-// The passed-in index is automatically advanced as the characters in the input
+-// HFS-decomposed UTF-8 strings are read.
++// Returns the next non-ignorable codepoint within `string` starting from the
++// position indicated by `index`, or zero if there are no more.
++// The passed-in `index` is automatically advanced as the characters in the
++// input HFS-decomposed UTF-8 strings are read.
+ inline base_icu::UChar32 HFSReadNextNonIgnorableCodepoint(const char* string,
+                                                           size_t length,
+                                                           size_t* index) {
+@@ -1220,12 +1222,16 @@ inline base_icu::UChar32 HFSReadNextNonIgnorableCodepoint(const char* string,
+     CBU8_NEXT(reinterpret_cast<const uint8_t*>(string), *index, length,
+               codepoint);
+     DCHECK_GT(codepoint, 0);
+-    if (codepoint > 0) {
++
++    // Note: Here, there are no lower case conversion implemented in the
++    // Supplementary Multilingual Plane (codepoint > 0xFFFF).
++
++    if (codepoint > 0 && codepoint <= 0xFFFF) {
+       // Check if there is a subtable for this upper byte.
+       int lookup_offset = lower_case_table[codepoint >> 8];
+       if (lookup_offset != 0)
+         codepoint = lower_case_table[lookup_offset + (codepoint & 0x00FF)];
+-      // Note: codepoint1 may be again 0 at this point if the character was
++      // Note: `codepoint` may be again 0 at this point if the character was
+       // an ignorable.
+     }
+   }
+diff --git a/base/files/file_path_unittest.cc b/base/files/file_path_unittest.cc
+index 7b30b15b87f1349c85e5c2e36c6a8d96873d7567..9673a48bc30274e579a8ec454c1f97a2a48eb70a 100644
+--- a/base/files/file_path_unittest.cc
++++ b/base/files/file_path_unittest.cc
+@@ -1203,6 +1203,13 @@ TEST_F(FilePathTest, CompareIgnoreCase) {
+     {{FPL("K\u0301U\u032DO\u0304\u0301N"), FPL("\u1E31\u1E77\u1E53n")}, 0},
+     {{FPL("k\u0301u\u032Do\u0304\u0301n"), FPL("\u1E30\u1E76\u1E52n")}, 0},
+     {{FPL("k\u0301u\u032Do\u0304\u0302n"), FPL("\u1E30\u1E76\u1E52n")}, 1},
++
++    // Codepoints > 0xFFFF
++    // Here, we compare the `Adlam Letter Shu` in its capital and small version.
++    {{FPL("\U0001E921"), FPL("\U0001E943")}, -1},
++    {{FPL("\U0001E943"), FPL("\U0001E921")}, 1},
++    {{FPL("\U0001E921"), FPL("\U0001E921")}, 0},
++    {{FPL("\U0001E943"), FPL("\U0001E943")}, 0},
+ #endif
+   };
+ 

+ 142 - 0
patches/chromium/cherry-pick-675562695049.patch

@@ -0,0 +1,142 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Rakina Zata Amni <[email protected]>
+Date: Mon, 15 May 2023 03:21:49 +0000
+Subject: Return after ReadyCommitNavigation call in CommitErrorPage if it
+ deletes NavigationRequest
+
+NavigationRequest::ReadyToCommitNavigation() can cause deletion of the
+NavigationRequest, so callers should check for that possibility after
+calling the function. A caller in CommitErrorPage is missing that
+check, which this CL adds, along with a regression test.
+
+(cherry picked from commit 42db806805ef2be64ee92803d3a784631b2a7df0)
+
+Bug: 1444360
+Change-Id: I3964da4909a6709b7730d25d6497b19c098f4f21
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4520493
+Commit-Queue: Charlie Reis <[email protected]>
+Reviewed-by: Charlie Reis <[email protected]>
+Cr-Original-Commit-Position: refs/heads/main@{#1143298}
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4531446
+Reviewed-by: Prudhvikumar Bommana <[email protected]>
+Commit-Queue: Rakina Zata Amni <[email protected]>
+Commit-Queue: Prudhvikumar Bommana <[email protected]>
+Owners-Override: Prudhvikumar Bommana <[email protected]>
+Cr-Commit-Position: refs/branch-heads/5735@{#607}
+Cr-Branched-From: 2f562e4ddbaf79a3f3cb338b4d1bd4398d49eb67-refs/heads/main@{#1135570}
+
+diff --git a/content/browser/renderer_host/navigation_request.cc b/content/browser/renderer_host/navigation_request.cc
+index d0f07ecdc9fae8d7c38a34ef071b79b97c080e5b..40c2dd985007a4b4b408814391834eec6e3bc123 100644
+--- a/content/browser/renderer_host/navigation_request.cc
++++ b/content/browser/renderer_host/navigation_request.cc
+@@ -5453,7 +5453,13 @@ void NavigationRequest::CommitErrorPage(
+     topics_eligible_ = false;
+   }
+ 
++  base::WeakPtr<NavigationRequest> weak_self(weak_factory_.GetWeakPtr());
+   ReadyToCommitNavigation(true /* is_error */);
++  // The caller above might result in the deletion of `this`. Return immediately
++  // if so.
++  if (!weak_self) {
++    return;
++  }
+ 
+   PopulateDocumentTokenForCrossDocumentNavigation();
+   // Use a separate cache shard, and no cookies, for error pages.
+diff --git a/content/browser/renderer_host/navigation_request_browsertest.cc b/content/browser/renderer_host/navigation_request_browsertest.cc
+index f55e3f4f96181d92ad768e431a4844ae774a7c9e..58f3526fe0a60b499fd06943b0577a52210338fb 100644
+--- a/content/browser/renderer_host/navigation_request_browsertest.cc
++++ b/content/browser/renderer_host/navigation_request_browsertest.cc
+@@ -46,6 +46,7 @@
+ #include "content/public/test/prerender_test_util.h"
+ #include "content/public/test/test_frame_navigation_observer.h"
+ #include "content/public/test/test_navigation_observer.h"
++#include "content/public/test/test_service.mojom.h"
+ #include "content/public/test/test_utils.h"
+ #include "content/public/test/url_loader_interceptor.h"
+ #include "content/shell/browser/shell.h"
+@@ -4384,4 +4385,84 @@ IN_PROC_BROWSER_TEST_P(NavigationRequestMPArchBrowserTest,
+   }
+ }
+ 
++// Tests that when trying to commit an error page for a failed navigation, but
++// the renderer process of the, the navigation won't commit and won't crash.
++// Regression test for https://crbug.com/1444360.
++IN_PROC_BROWSER_TEST_F(NavigationRequestBrowserTest,
++                       RendererCrashedBeforeCommitErrorPage) {
++  // Navigate to `url_a` first.
++  GURL url_a(embedded_test_server()->GetURL("a.com", "/title1.html"));
++  ASSERT_TRUE(NavigateToURL(shell(), url_a));
++
++  // Set up an URLLoaderInterceptor which will cause future navigations to fail.
++  auto url_loader_interceptor = std::make_unique<URLLoaderInterceptor>(
++      base::BindRepeating([](URLLoaderInterceptor::RequestParams* params) {
++        network::URLLoaderCompletionStatus status;
++        status.error_code = net::ERR_NOT_IMPLEMENTED;
++        params->client->OnComplete(status);
++        return true;
++      }));
++
++  // Do a navigation to `url_b1` that will fail and commit an error page. This
++  // is important so that the next error page navigation won't need to create a
++  // speculative RenderFrameHost (unless RenderDocument is enabled) and won't
++  // get cancelled earlier than commit time due to speculative RFH deletion.
++  GURL url_b1(embedded_test_server()->GetURL("b.com", "/title1.html"));
++  EXPECT_FALSE(NavigateToURL(shell(), url_b1));
++  EXPECT_EQ(shell()->web_contents()->GetLastCommittedURL(), url_b1);
++  EXPECT_TRUE(
++      shell()->web_contents()->GetPrimaryMainFrame()->IsErrorDocument());
++
++  // For the next navigation, set up a throttle that will be used to wait for
++  // WillFailRequest() and then defer the navigation, so that we can crash the
++  // error page process first.
++  TestNavigationThrottleInstaller installer(
++      shell()->web_contents(),
++      NavigationThrottle::PROCEED /* will_start_result */,
++      NavigationThrottle::PROCEED /* will_redirect_result */,
++      NavigationThrottle::DEFER /* will_fail_result */,
++      NavigationThrottle::PROCEED /* will_process_result */,
++      NavigationThrottle::PROCEED /* will_commit_without_url_loader_result */);
++
++  // Start a navigation to `url_b2` that will also fail, but before it commits
++  // an error page, cause the error page process to crash.
++  GURL url_b2(embedded_test_server()->GetURL("b.com", "/title2.html"));
++  TestNavigationManager manager(shell()->web_contents(), url_b2);
++  shell()->LoadURL(url_b2);
++  EXPECT_TRUE(manager.WaitForRequestStart());
++
++  // Resume the navigation and wait for WillFailRequest(). After this point, we
++  // will have picked the final RenderFrameHost & RenderProcessHost for the
++  // failed navigation.
++  manager.ResumeNavigation();
++  installer.WaitForThrottleWillFail();
++
++  // Kill the error page process. This will cause for the navigation to `url_b2`
++  // to return early in `NavigationRequest::ReadyToCommitNavigation()` and not
++  // commit a new error page.
++  RenderProcessHost* process_to_kill =
++      manager.GetNavigationHandle()->GetRenderFrameHost()->GetProcess();
++  ASSERT_TRUE(process_to_kill->IsInitializedAndNotDead());
++  {
++    // Trigger a renderer kill by calling DoSomething() which will cause a bad
++    // message to be reported.
++    RenderProcessHostBadIpcMessageWaiter kill_waiter(process_to_kill);
++    mojo::Remote<mojom::TestService> service;
++    process_to_kill->BindReceiver(service.BindNewPipeAndPassReceiver());
++    service->DoSomething(base::DoNothing());
++    EXPECT_EQ(bad_message::RPH_MOJO_PROCESS_ERROR, kill_waiter.Wait());
++  }
++  ASSERT_FALSE(process_to_kill->IsInitializedAndNotDead());
++
++  // Resume the navigation, which won't commit.
++  if (!ShouldCreateNewHostForAllFrames()) {
++    installer.navigation_throttle()->ResumeNavigation();
++  }
++  EXPECT_TRUE(manager.WaitForNavigationFinished());
++  EXPECT_FALSE(WaitForLoadStop(shell()->web_contents()));
++
++  // The tab stayed at `url_b1` as the `url_b2` navigation didn't commit.
++  EXPECT_EQ(shell()->web_contents()->GetLastCommittedURL(), url_b1);
++}
++
+ }  // namespace content

+ 108 - 0
patches/chromium/cherry-pick-d6272b794cbb.patch

@@ -0,0 +1,108 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Simon=20Z=C3=BCnd?= <[email protected]>
+Date: Tue, 2 May 2023 06:05:35 +0000
+Subject: Delete PendingRequest first in DevToolsDataSource
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+The way URLDataSources are used in Chromium, it can happen that the
+"content::URLDataSource::GotDataCallback" closure is the last shared
+owner of the data source itself. This means that the URLDataSource
+is deleted after the callback is done running.
+
+This CL fixes an invalid access to DevToolsDataSource, where we
+access `this` in the OnLoadComplete method after we call the
+GotDataCallback.
+
[email protected]
+
+Fixed: 1435166
+Change-Id: I32e4a717ca27bc011449c8f8efeaffe70aaa8898
+Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4487280
+Reviewed-by: Andrey Kosyakov <[email protected]>
+Commit-Queue: Simon Zünd <[email protected]>
+Reviewed-by: Danil Somsikov <[email protected]>
+Cr-Commit-Position: refs/heads/main@{#1138173}
+
+diff --git a/chrome/browser/ui/webui/devtools_ui_data_source.cc b/chrome/browser/ui/webui/devtools_ui_data_source.cc
+index c60d70fb3271e2459634709f015901b70db5b64c..991599d11df139173b356d50d3027c38a356eeb8 100644
+--- a/chrome/browser/ui/webui/devtools_ui_data_source.cc
++++ b/chrome/browser/ui/webui/devtools_ui_data_source.cc
+@@ -365,11 +365,13 @@ void DevToolsDataSource::StartFileRequest(const std::string& path,
+ void DevToolsDataSource::OnLoadComplete(
+     std::list<PendingRequest>::iterator request_iter,
+     std::unique_ptr<std::string> response_body) {
+-  std::move(request_iter->callback)
+-      .Run(response_body ? base::MakeRefCounted<base::RefCountedString>(
+-                               std::move(*response_body))
+-                         : CreateNotFoundResponse());
++  GotDataCallback callback = std::move(request_iter->callback);
+   pending_requests_.erase(request_iter);
++  std::move(callback).Run(response_body
++                              ? base::MakeRefCounted<base::RefCountedString>(
++                                    std::move(*response_body))
++                              : CreateNotFoundResponse());
++  // `this` might no longer be valid after `callback` has run.
+ }
+ 
+ DevToolsDataSource::PendingRequest::PendingRequest() = default;
+diff --git a/chrome/browser/ui/webui/devtools_ui_data_source_unittest.cc b/chrome/browser/ui/webui/devtools_ui_data_source_unittest.cc
+index b4f41411c578b0a0c1b3fcd65e170744b4e62ee3..c07c43b6a139bfbdbe4a0bbaa7e4dfb425442d99 100644
+--- a/chrome/browser/ui/webui/devtools_ui_data_source_unittest.cc
++++ b/chrome/browser/ui/webui/devtools_ui_data_source_unittest.cc
+@@ -9,13 +9,17 @@
+ #include "base/command_line.h"
+ #include "base/functional/bind.h"
+ #include "base/memory/ref_counted_memory.h"
++#include "base/memory/scoped_refptr.h"
+ #include "base/strings/strcat.h"
+ #include "base/strings/string_piece.h"
++#include "base/test/bind.h"
+ #include "build/build_config.h"
+ #include "chrome/common/chrome_switches.h"
+ #include "chrome/common/url_constants.h"
+ #include "content/public/browser/url_data_source.h"
++#include "content/public/test/browser_task_environment.h"
+ #include "services/network/public/cpp/weak_wrapper_shared_url_loader_factory.h"
++#include "services/network/test/test_shared_url_loader_factory.h"
+ #include "testing/gtest/include/gtest/gtest.h"
+ 
+ namespace {
+@@ -356,3 +360,36 @@ TEST_F(DevToolsUIDataSourceTest, TestDevToolsNoRouteWithSwitch) {
+   ASSERT_TRUE(base::StartsWith(data(), kDevToolsUITest404Response,
+                                base::CompareCase::SENSITIVE));
+ }
++
++class DevToolsUIDataSourceWithTaskEnvTest : public testing::Test {
++ public:
++  DevToolsUIDataSourceWithTaskEnvTest()
++      : task_environment_(content::BrowserTaskEnvironment::IO_MAINLOOP) {}
++
++ private:
++  content::BrowserTaskEnvironment task_environment_;
++};
++
++TEST_F(DevToolsUIDataSourceWithTaskEnvTest,
++       GotDataCallbackOwnsDevToolsDataSource) {
++  scoped_refptr<network::SharedURLLoaderFactory> factory =
++      base::MakeRefCounted<network::TestSharedURLLoaderFactory>();
++  DevToolsDataSource* data_source = new DevToolsDataSource(factory);
++
++  DevToolsDataSource::GotDataCallback callback = base::BindOnce(
++      [](DevToolsDataSource* data_source,
++         scoped_refptr<base::RefCountedMemory> payload) {
++        // Do nothing in the callback.
++      },
++      base::Owned(data_source));
++
++  // `callback` controls the life-time of the data_source now, so data_source is
++  // deleted after the callback is done running. This is similar to what
++  // WebUIURLLoaderFactory is doing.
++
++  const GURL path =
++      DevToolsUrl().Resolve(DevToolsRemotePath(kDevToolsUITestFrontEndUrl));
++  content::WebContents::Getter wc_getter;
++  data_source->StartDataRequest(path, std::move(wc_getter),
++                                std::move(callback));
++}

+ 1 - 0
patches/v8/.patches

@@ -9,3 +9,4 @@ fix_disable_implies_dcheck_for_node_stream_array_buffers.patch
 force_cppheapcreateparams_to_be_noncopyable.patch
 chore_allow_customizing_microtask_policy_per_context.patch
 build_revert_builtins_pgo.patch
+cherry-pick-b8020e1973d7.patch

+ 358 - 0
patches/v8/cherry-pick-b8020e1973d7.patch

@@ -0,0 +1,358 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Igor Sheludko <[email protected]>
+Date: Thu, 27 Apr 2023 11:11:32 +0200
+Subject: Fix v8::Object::SetAccessorProperty
+
+... by using JavaScript spec compliant JSReceiver::DefineOwnProperty.
+
+Drive-by:
+- cleanup comments in include/v8-object.h, insert links to
+respective pages of https://tc39.es/ecma262/ when referencing spec,
+- rename JSObject::DefineAccessor() to
+  JSObject::DefineOwnAccessorIgnoreAttributes().
+
+Bug: chromium:1433211
+Change-Id: Ia9edaadd68f1986f18581156ad8f79c438b77744
+Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4458947
+Commit-Queue: Igor Sheludko <[email protected]>
+Reviewed-by: Toon Verwaest <[email protected]>
+Cr-Commit-Position: refs/heads/main@{#87302}
+
+diff --git a/include/v8-object.h b/include/v8-object.h
+index 6c03d63c482c4096777b887d6f6a22336f82147e..6954c00b50a504d2a0205fb67f314e4e50fb455a 100644
+--- a/include/v8-object.h
++++ b/include/v8-object.h
+@@ -247,13 +247,16 @@ class V8_EXPORT Object : public Value {
+   V8_WARN_UNUSED_RESULT Maybe<bool> Set(Local<Context> context, uint32_t index,
+                                         Local<Value> value);
+ 
+-  // Implements CreateDataProperty (ECMA-262, 7.3.4).
+-  //
+-  // Defines a configurable, writable, enumerable property with the given value
+-  // on the object unless the property already exists and is not configurable
+-  // or the object is not extensible.
+-  //
+-  // Returns true on success.
++  /**
++   * Implements CreateDataProperty(O, P, V), see
++   * https://tc39.es/ecma262/#sec-createdataproperty.
++   *
++   * Defines a configurable, writable, enumerable property with the given value
++   * on the object unless the property already exists and is not configurable
++   * or the object is not extensible.
++   *
++   * Returns true on success.
++   */
+   V8_WARN_UNUSED_RESULT Maybe<bool> CreateDataProperty(Local<Context> context,
+                                                        Local<Name> key,
+                                                        Local<Value> value);
+@@ -261,29 +264,35 @@ class V8_EXPORT Object : public Value {
+                                                        uint32_t index,
+                                                        Local<Value> value);
+ 
+-  // Implements DefineOwnProperty.
+-  //
+-  // In general, CreateDataProperty will be faster, however, does not allow
+-  // for specifying attributes.
+-  //
+-  // Returns true on success.
++  /**
++   * Implements [[DefineOwnProperty]] for data property case, see
++   * https://tc39.es/ecma262/#table-essential-internal-methods.
++   *
++   * In general, CreateDataProperty will be faster, however, does not allow
++   * for specifying attributes.
++   *
++   * Returns true on success.
++   */
+   V8_WARN_UNUSED_RESULT Maybe<bool> DefineOwnProperty(
+       Local<Context> context, Local<Name> key, Local<Value> value,
+       PropertyAttribute attributes = None);
+ 
+-  // Implements Object.DefineProperty(O, P, Attributes), see Ecma-262 19.1.2.4.
+-  //
+-  // The defineProperty function is used to add an own property or
+-  // update the attributes of an existing own property of an object.
+-  //
+-  // Both data and accessor descriptors can be used.
+-  //
+-  // In general, CreateDataProperty is faster, however, does not allow
+-  // for specifying attributes or an accessor descriptor.
+-  //
+-  // The PropertyDescriptor can change when redefining a property.
+-  //
+-  // Returns true on success.
++  /**
++   * Implements Object.defineProperty(O, P, Attributes), see
++   * https://tc39.es/ecma262/#sec-object.defineproperty.
++   *
++   * The defineProperty function is used to add an own property or
++   * update the attributes of an existing own property of an object.
++   *
++   * Both data and accessor descriptors can be used.
++   *
++   * In general, CreateDataProperty is faster, however, does not allow
++   * for specifying attributes or an accessor descriptor.
++   *
++   * The PropertyDescriptor can change when redefining a property.
++   *
++   * Returns true on success.
++   */
+   V8_WARN_UNUSED_RESULT Maybe<bool> DefineProperty(
+       Local<Context> context, Local<Name> key, PropertyDescriptor& descriptor);
+ 
+@@ -302,14 +311,15 @@ class V8_EXPORT Object : public Value {
+       Local<Context> context, Local<Value> key);
+ 
+   /**
+-   * Returns Object.getOwnPropertyDescriptor as per ES2016 section 19.1.2.6.
++   * Implements Object.getOwnPropertyDescriptor(O, P), see
++   * https://tc39.es/ecma262/#sec-object.getownpropertydescriptor.
+    */
+   V8_WARN_UNUSED_RESULT MaybeLocal<Value> GetOwnPropertyDescriptor(
+       Local<Context> context, Local<Name> key);
+ 
+   /**
+-   * Object::Has() calls the abstract operation HasProperty(O, P) described
+-   * in ECMA-262, 7.3.10. Has() returns
++   * Object::Has() calls the abstract operation HasProperty(O, P), see
++   * https://tc39.es/ecma262/#sec-hasproperty. Has() returns
+    * true, if the object has the property, either own or on the prototype chain.
+    * Interceptors, i.e., PropertyQueryCallbacks, are called if present.
+    *
+@@ -347,7 +357,7 @@ class V8_EXPORT Object : public Value {
+ 
+   void SetAccessorProperty(Local<Name> name, Local<Function> getter,
+                            Local<Function> setter = Local<Function>(),
+-                           PropertyAttribute attribute = None,
++                           PropertyAttribute attributes = None,
+                            AccessControl settings = DEFAULT);
+ 
+   /**
+diff --git a/src/api/api-natives.cc b/src/api/api-natives.cc
+index 05a883f2d560e360b6933639dab3ec956464cb1a..905f29bf253c443204f3f9f36fd4c948ed434f15 100644
+--- a/src/api/api-natives.cc
++++ b/src/api/api-natives.cc
+@@ -96,10 +96,10 @@ MaybeHandle<Object> DefineAccessorProperty(Isolate* isolate,
+     Handle<Code> trampoline = BUILTIN_CODE(isolate, DebugBreakTrampoline);
+     Handle<JSFunction>::cast(setter)->set_code(*trampoline);
+   }
+-  RETURN_ON_EXCEPTION(
+-      isolate,
+-      JSObject::DefineAccessor(object, name, getter, setter, attributes),
+-      Object);
++  RETURN_ON_EXCEPTION(isolate,
++                      JSObject::DefineOwnAccessorIgnoreAttributes(
++                          object, name, getter, setter, attributes),
++                      Object);
+   return object;
+ }
+ 
+diff --git a/src/api/api.cc b/src/api/api.cc
+index d664893fca9e378853eaaae5b174679dadc75d91..a30c50948ab634313f33d5eb96cef98fa671d121 100644
+--- a/src/api/api.cc
++++ b/src/api/api.cc
+@@ -5138,7 +5138,7 @@ Maybe<bool> Object::SetAccessor(Local<Context> context, Local<Name> name,
+ 
+ void Object::SetAccessorProperty(Local<Name> name, Local<Function> getter,
+                                  Local<Function> setter,
+-                                 PropertyAttribute attribute,
++                                 PropertyAttribute attributes,
+                                  AccessControl settings) {
+   // TODO(verwaest): Remove |settings|.
+   DCHECK_EQ(v8::DEFAULT, settings);
+@@ -5150,9 +5150,20 @@ void Object::SetAccessorProperty(Local<Name> name, Local<Function> getter,
+   i::Handle<i::Object> getter_i = v8::Utils::OpenHandle(*getter);
+   i::Handle<i::Object> setter_i = v8::Utils::OpenHandle(*setter, true);
+   if (setter_i.is_null()) setter_i = i_isolate->factory()->null_value();
+-  i::JSObject::DefineAccessor(i::Handle<i::JSObject>::cast(self),
+-                              v8::Utils::OpenHandle(*name), getter_i, setter_i,
+-                              static_cast<i::PropertyAttributes>(attribute));
++
++  i::PropertyDescriptor desc;
++  desc.set_enumerable(!(attributes & v8::DontEnum));
++  desc.set_configurable(!(attributes & v8::DontDelete));
++  desc.set_get(getter_i);
++  desc.set_set(setter_i);
++
++  i::Handle<i::Name> name_i = v8::Utils::OpenHandle(*name);
++  // DefineOwnProperty might still throw if the receiver is a JSProxy and it
++  // might fail if the receiver is non-extensible or already has this property
++  // as non-configurable.
++  Maybe<bool> success = i::JSReceiver::DefineOwnProperty(
++      i_isolate, self, name_i, &desc, Just(i::kDontThrow));
++  USE(success);
+ }
+ 
+ Maybe<bool> Object::SetNativeDataProperty(
+diff --git a/src/init/bootstrapper.cc b/src/init/bootstrapper.cc
+index 06d945e25356958b206cadc352829ae37610ec16..139acc88ac7efa74182f2c43dacf039c87e08f32 100644
+--- a/src/init/bootstrapper.cc
++++ b/src/init/bootstrapper.cc
+@@ -638,7 +638,9 @@ V8_NOINLINE void SimpleInstallGetterSetter(Isolate* isolate,
+   Handle<JSFunction> setter =
+       SimpleCreateFunction(isolate, setter_name, call_setter, 1, true);
+ 
+-  JSObject::DefineAccessor(base, name, getter, setter, DONT_ENUM).Check();
++  JSObject::DefineOwnAccessorIgnoreAttributes(base, name, getter, setter,
++                                              DONT_ENUM)
++      .Check();
+ }
+ 
+ void SimpleInstallGetterSetter(Isolate* isolate, Handle<JSObject> base,
+@@ -662,7 +664,8 @@ V8_NOINLINE Handle<JSFunction> SimpleInstallGetter(Isolate* isolate,
+ 
+   Handle<Object> setter = isolate->factory()->undefined_value();
+ 
+-  JSObject::DefineAccessor(base, property_name, getter, setter, DONT_ENUM)
++  JSObject::DefineOwnAccessorIgnoreAttributes(base, property_name, getter,
++                                              setter, DONT_ENUM)
+       .Check();
+ 
+   return getter;
+diff --git a/src/objects/js-objects.cc b/src/objects/js-objects.cc
+index 8fbf7a135e36085e8a32c8224c35db45702b4ab1..f3a45f1a1220f68a277617617cc0c534d5e542a2 100644
+--- a/src/objects/js-objects.cc
++++ b/src/objects/js-objects.cc
+@@ -1541,7 +1541,8 @@ Maybe<bool> JSReceiver::ValidateAndApplyPropertyDescriptor(
+                 ? desc->set()
+                 : Handle<Object>::cast(isolate->factory()->null_value()));
+         MaybeHandle<Object> result =
+-            JSObject::DefineAccessor(it, getter, setter, desc->ToAttributes());
++            JSObject::DefineOwnAccessorIgnoreAttributes(it, getter, setter,
++                                                        desc->ToAttributes());
+         if (result.is_null()) return Nothing<bool>();
+       }
+     }
+@@ -1725,8 +1726,8 @@ Maybe<bool> JSReceiver::ValidateAndApplyPropertyDescriptor(
+               : current->has_set()
+                     ? current->set()
+                     : Handle<Object>::cast(isolate->factory()->null_value()));
+-      MaybeHandle<Object> result =
+-          JSObject::DefineAccessor(it, getter, setter, attrs);
++      MaybeHandle<Object> result = JSObject::DefineOwnAccessorIgnoreAttributes(
++          it, getter, setter, attrs);
+       if (result.is_null()) return Nothing<bool>();
+     }
+   }
+@@ -4703,22 +4704,19 @@ bool JSObject::HasEnumerableElements() {
+   UNREACHABLE();
+ }
+ 
+-MaybeHandle<Object> JSObject::DefineAccessor(Handle<JSObject> object,
+-                                             Handle<Name> name,
+-                                             Handle<Object> getter,
+-                                             Handle<Object> setter,
+-                                             PropertyAttributes attributes) {
++MaybeHandle<Object> JSObject::DefineOwnAccessorIgnoreAttributes(
++    Handle<JSObject> object, Handle<Name> name, Handle<Object> getter,
++    Handle<Object> setter, PropertyAttributes attributes) {
+   Isolate* isolate = object->GetIsolate();
+ 
+   PropertyKey key(isolate, name);
+   LookupIterator it(isolate, object, key, LookupIterator::OWN_SKIP_INTERCEPTOR);
+-  return DefineAccessor(&it, getter, setter, attributes);
++  return DefineOwnAccessorIgnoreAttributes(&it, getter, setter, attributes);
+ }
+ 
+-MaybeHandle<Object> JSObject::DefineAccessor(LookupIterator* it,
+-                                             Handle<Object> getter,
+-                                             Handle<Object> setter,
+-                                             PropertyAttributes attributes) {
++MaybeHandle<Object> JSObject::DefineOwnAccessorIgnoreAttributes(
++    LookupIterator* it, Handle<Object> getter, Handle<Object> setter,
++    PropertyAttributes attributes) {
+   Isolate* isolate = it->isolate();
+ 
+   it->UpdateProtector();
+diff --git a/src/objects/js-objects.h b/src/objects/js-objects.h
+index 80a64a63a16e2aaed7aeacbd734f4488df34e91b..870eb81553f0debadea08a20aad44ff216f09226 100644
+--- a/src/objects/js-objects.h
++++ b/src/objects/js-objects.h
+@@ -538,13 +538,14 @@ class JSObject : public TorqueGeneratedJSObject<JSObject, JSReceiver> {
+   GetPropertyAttributesWithFailedAccessCheck(LookupIterator* it);
+ 
+   // Defines an AccessorPair property on the given object.
+-  V8_EXPORT_PRIVATE static MaybeHandle<Object> DefineAccessor(
+-      Handle<JSObject> object, Handle<Name> name, Handle<Object> getter,
+-      Handle<Object> setter, PropertyAttributes attributes);
+-  static MaybeHandle<Object> DefineAccessor(LookupIterator* it,
+-                                            Handle<Object> getter,
+-                                            Handle<Object> setter,
+-                                            PropertyAttributes attributes);
++  V8_EXPORT_PRIVATE static MaybeHandle<Object>
++  DefineOwnAccessorIgnoreAttributes(Handle<JSObject> object, Handle<Name> name,
++                                    Handle<Object> getter,
++                                    Handle<Object> setter,
++                                    PropertyAttributes attributes);
++  static MaybeHandle<Object> DefineOwnAccessorIgnoreAttributes(
++      LookupIterator* it, Handle<Object> getter, Handle<Object> setter,
++      PropertyAttributes attributes);
+ 
+   // Defines an AccessorInfo property on the given object.
+   V8_WARN_UNUSED_RESULT static MaybeHandle<Object> SetAccessor(
+diff --git a/src/runtime/runtime-object.cc b/src/runtime/runtime-object.cc
+index d72c1fd4e08e0d4620ff63da6aa49c20123ea6dc..c83ee5ed097c4e0c1cb8d4f703389df5c48799ad 100644
+--- a/src/runtime/runtime-object.cc
++++ b/src/runtime/runtime-object.cc
+@@ -1082,7 +1082,8 @@ RUNTIME_FUNCTION(Runtime_DefineAccessorPropertyUnchecked) {
+   auto attrs = PropertyAttributesFromInt(args.smi_value_at(4));
+ 
+   RETURN_FAILURE_ON_EXCEPTION(
+-      isolate, JSObject::DefineAccessor(obj, name, getter, setter, attrs));
++      isolate, JSObject::DefineOwnAccessorIgnoreAttributes(obj, name, getter,
++                                                           setter, attrs));
+   return ReadOnlyRoots(isolate).undefined_value();
+ }
+ 
+@@ -1209,8 +1210,8 @@ RUNTIME_FUNCTION(Runtime_DefineGetterPropertyUnchecked) {
+ 
+   RETURN_FAILURE_ON_EXCEPTION(
+       isolate,
+-      JSObject::DefineAccessor(object, name, getter,
+-                               isolate->factory()->null_value(), attrs));
++      JSObject::DefineOwnAccessorIgnoreAttributes(
++          object, name, getter, isolate->factory()->null_value(), attrs));
+   return ReadOnlyRoots(isolate).undefined_value();
+ }
+ 
+@@ -1354,8 +1355,8 @@ RUNTIME_FUNCTION(Runtime_DefineSetterPropertyUnchecked) {
+ 
+   RETURN_FAILURE_ON_EXCEPTION(
+       isolate,
+-      JSObject::DefineAccessor(object, name, isolate->factory()->null_value(),
+-                               setter, attrs));
++      JSObject::DefineOwnAccessorIgnoreAttributes(
++          object, name, isolate->factory()->null_value(), setter, attrs));
+   return ReadOnlyRoots(isolate).undefined_value();
+ }
+ 
+diff --git a/src/sandbox/testing.cc b/src/sandbox/testing.cc
+index adf86969f0eeeb8aa92056b40f7998eed8a473fe..0dec24c43fd02e7333380cf80b355e2e078b8ec3 100644
+--- a/src/sandbox/testing.cc
++++ b/src/sandbox/testing.cc
+@@ -160,7 +160,8 @@ void InstallGetter(Isolate* isolate, Handle<JSObject> object,
+   Handle<String> property_name = factory->NewStringFromAsciiChecked(name);
+   Handle<JSFunction> getter = CreateFunc(isolate, func, property_name, false);
+   Handle<Object> setter = factory->null_value();
+-  JSObject::DefineAccessor(object, property_name, getter, setter, FROZEN);
++  JSObject::DefineOwnAccessorIgnoreAttributes(object, property_name, getter,
++                                              setter, FROZEN);
+ }
+ 
+ void InstallFunction(Isolate* isolate, Handle<JSObject> holder,
+diff --git a/test/cctest/test-code-stub-assembler.cc b/test/cctest/test-code-stub-assembler.cc
+index 5efe5281c26937cdf6e79e07cfb01b8233c3a1f4..4c2c8ed11eb7cdf504c9549d6c8c8ba61f7ce765 100644
+--- a/test/cctest/test-code-stub-assembler.cc
++++ b/test/cctest/test-code-stub-assembler.cc
+@@ -1178,7 +1178,9 @@ void AddProperties(Handle<JSObject> object, Handle<Name> names[],
+       Handle<AccessorPair> pair = Handle<AccessorPair>::cast(value);
+       Handle<Object> getter(pair->getter(), isolate);
+       Handle<Object> setter(pair->setter(), isolate);
+-      JSObject::DefineAccessor(object, names[i], getter, setter, NONE).Check();
++      JSObject::DefineOwnAccessorIgnoreAttributes(object, names[i], getter,
++                                                  setter, NONE)
++          .Check();
+     } else {
+       JSObject::AddProperty(isolate, object, names[i], value, NONE);
+     }