src: remove dependency on wrapper-descriptor-based cpp heap #53086

joyeecheung · 2024-05-21T15:30:54Z

This is baking for the V8 12.5 upgrade so the base branch is canary-base. Original PR and reviews are in nodejs/node-v8#284

src: remove dependency on wrapper-descriptor-based CppHeap.

As V8 has moved away from wrapper-descriptor-based CppHeap, this
patch:

Create the CppHeap without using wrapper descirptors.
Deprecates node::SetCppgcReference() in favor of
v8::Object::Wrap() since the wrapper descriptor is no longer
relevant. It is still kept as a compatibility layer for addons
that need to also work on Node.js versions without
v8::Object::Wrap().

Fixes: nodejs/node-v8#283

nodejs-github-bot · 2024-05-21T16:07:07Z

CI: https://ci.nodejs.org/job/node-test-pull-request/59334/

targos · 2024-05-22T06:12:47Z

Looks like some things are broken.

joyeecheung · 2024-05-22T11:55:26Z

Looks like we have a timing conflict with #30846 (comment) - if we migrate to the new CppHeap management, the CppHeap is torn down during Isolate disposal, and at this point the Isolate needs to be registered with the Platform for it to look up the task runner. But due to #30846 (comment) we also had to unregister the Isolate before the disposal to avoid the mentioned race condition.

I wonder if this can be worked around by using something else as the Isolate map keys in the platform to avoid the said race condition & allow us to tear down the isolate while it's still discoverable by the platform...maybe using a self-incrementing ID as key is already enough? cc @addaleax

addaleax · 2024-05-23T21:35:20Z

@joyeecheung Yeah ... I assume in the end any key that we can look up based on the Isolate* pointer, without needing the Isolate instance itself, will work? But obviously the pointer itself is a pretty natural choice ... would it be possible to split Isolate disposal into two parts, like Isolate creation (i.e. split into allocation + initialization)? That requires a V8 change but might be the best solution apart from that

joyeecheung · 2024-05-27T10:44:57Z

I wonder if there's another way to fix the isolate disposal race condition...by shutting down the platform data/task runner after we remove it from the map? In that case it's not possible for the conflicting new Isolate to be mapped to the platform data being shut down since it's already out of the map and is doing its own thing. Locally that seems fine and doesn't regress the test, although I am unable to reproduce the flake in #30846 after reverting the dispose-unregister order either, I'll open a separate PR for this change and investigate.

Major V8 updates are usually API/ABI incompatible with previous versions. This commit adapts NODE_MODULE_VERSION for V8 12.7. Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md

dllexport introduces issues when compiling with MSVC. PR-URL: nodejs#47251 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Richard Lau <[email protected]>

It introduces process hangs on some platforms because Node.js doesn't tear down V8 correctly. Disable it while we work on a solution. Refs: nodejs#47297 Refs: https://bugs.chromium.org/p/v8/issues/detail?id=13902 PR-URL: nodejs#47450 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Rich Trott <[email protected]> Reviewed-By: James M Snell <[email protected]>

Refs: https://bugs.chromium.org/p/v8/issues/detail?id=14221 PR-URL: nodejs#49639 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

We are not ready to compile with C++20 support yet. This is only a DCHECK that can be removed without affecting the behavior of release builds. PR-URL: nodejs#49639 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

PR-URL: nodejs#49639 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

PR-URL: nodejs#52293 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Richard Lau <[email protected]>

After enabling -std:c++20 on Windows, patch is now much smaller. PR-URL: nodejs#52465 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Michael Dawson <[email protected]>

On Windows debug builds, it is not allowed to dereference empty iterators. Refs: https://chromium-review.googlesource.com/c/v8/v8/+/5331688 PR-URL: nodejs#52465 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Michael Dawson <[email protected]>

Refs: v8/v8@5495678

Refs: https://chromium-review.googlesource.com/c/v8/v8/+/5300826

Refs: https://chromium-review.googlesource.com/c/v8/v8/+/5447267 Co-authored-by: Michaël Zasso <[email protected]>

Refs: v8/v8@e30e228

Refs: v8/v8@f8fa220

targos · 2024-05-28T09:08:31Z

I just had to force-push canary-base to remove 9197598 (it's now on main).

targos · 2024-05-29T15:30:00Z

v8#187 may be relevant

github-actions · 2024-05-29T18:31:23Z

Failed to start CI

   ⚠  No approving reviews found
   ✘  Refusing to run CI on potentially unsafe PR

https://github.com/nodejs/node/actions/runs/9291289224

nodejs-github-bot · 2024-05-29T18:32:00Z

CI: https://ci.nodejs.org/job/node-test-pull-request/59525/

joyeecheung · 2024-05-29T18:32:42Z

Pushed a commit here to fix the -Werror=comment errors in v8-sandbox.h to see if Linux CI is happy with it..

src/env.cc

jasnell · 2024-05-29T21:19:48Z

src/env.cc

+// Now V8 has deprecated this layout-based tracing enablement, embeders
+// should simply use v8::Object::Wrap() and v8::Object::Unwrap(). We preserve
+// this layout only to distinguish internally how the memory of a Node.js
+// wrapper is managed or whether a wrapper is managed by Node.js.


Just to clarify for myself on this comment... this means you're still setting an embedder_id in slot 0 in base objects in order to differentiate our own API objects from any other v8::Object for which IsApiWrapper() returns true, correct?

Yes, basically for printing things and snapshots.

src/env.cc

jasnell · 2024-05-29T21:23:35Z

src/node.h

+// objects on Node.js versions without v8::Object::Wrap(). Addons created to
+// work with only Node.js versions with v8::Object::Wrap() should use that
+// instead.
+NODE_DEPRECATED("Use v8::Object::Wrap()",


For this, should we also re-export the enum value to be used with v8::Object::Wrap/Unwrap to make it easier? It's fairly non-obvious that folks would need to add an include to v8-sandbox.h to get that. Alternatively, we could consider exposing our own wrapper wrap/unwrap methods here that delegate to the new v8 APIs to hide that detail.

It's fairly non-obvious that folks would need to add an include to v8-sandbox.h to get that.

That sounds more like something that should be documented in V8's header?

Alternatively, we could consider exposing our own wrapper wrap/unwrap methods here that delegate to the new v8 APIs to hide that detail.

For our side I think that can be left as a TODO, this is mostly for just migrating away from the existing deprecated APIs. SetCppgcReference() is already basically just v8::Object::Wrap, we can add a new API to provide GetCppgcReference() in a different PR but it feels out of the place for this one (which is for canary-base, not main).

targos · 2024-05-30T07:03:00Z

I don't know if it's related but there are many crashes with heapdump/heapsnapshot tests: https://ci.nodejs.org/job/node-test-commit-arm-debug/13095/

joyeecheung · 2024-05-30T08:50:19Z

The heap snapshot failures are the same as nodejs/node-v8#282 which are being fixed in https://chromium-review.googlesource.com/c/v8/v8/+/5531355

joyeecheung · 2024-05-30T12:00:54Z

v8-sandbox.h fixes upstreamed in https://chromium-review.googlesource.com/c/v8/v8/+/5584738

Before it fails to compile with -Werrors=comment because some of the `//` comments uses `\` at the end of the line. Refs: nodejs/node#53086 Change-Id: I10e5b970cff4a8eda78c305f5649d8bf08d7da3c Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5584738 Reviewed-by: Michael Lippautz <[email protected]> Commit-Queue: Joyee Cheung <[email protected]> Cr-Commit-Position: refs/heads/main@{#94165}

Original commit message: src: fix v8-sandbox.h multiline comments Before it fails to compile with -Werrors=comment because some of the `//` comments uses `\` at the end of the line. Refs: nodejs#53086 Change-Id: I10e5b970cff4a8eda78c305f5649d8bf08d7da3c Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/5584738 Reviewed-by: Michael Lippautz <[email protected]> Commit-Queue: Joyee Cheung <[email protected]> Cr-Commit-Position: refs/heads/main@{#94165} Refs: v8/v8@300451e

As V8 has moved away from wrapper-descriptor-based CppHeap, this patch: 1. Create the CppHeap without using wrapper descirptors. 2. Deprecates node::SetCppgcReference() in favor of v8::Object::Wrap() since the wrapper descriptor is no longer relevant. It is still kept as a compatibility layer for addons that need to also work on Node.js versions without v8::Object::Wrap().

nodejs-github-bot · 2024-06-01T13:14:06Z

CI: https://ci.nodejs.org/job/node-test-pull-request/59598/

joyeecheung · 2024-06-04T08:57:08Z

Not sure what's the way to make progress here...the CI failures are unrelated AFAICT, but the test won't be passing since canary-base isn't working yet. What's the policy for landing patches on canary-base?

Also cc @addaleax @legendecas who reviewed the original PR.

targos · 2024-06-04T09:02:30Z

There isn't really a policy. From my PoV, if it is brings us towards a passing canary build, this can land.

targos · 2024-06-04T11:23:58Z

I updated canary-base so it includes v8/v8@300451e
and pushed 30329d0 to it.
Thank you!

nodejs/node#53086

* chore: bump chromium in DEPS to 126.0.6470.0 * 5492605: Migrate TODOs referencing old crbug IDs to the new issue tracker IDs | https://chromium-review.googlesource.com/c/chromium/src/+/5492605 * 5513277: Move subresource-filter-ruleset to GCS | https://chromium-review.googlesource.com/c/chromium/src/+/5513277 * 5512656: Remove CustomizeChromeSupportsChromeRefresh2023 | https://chromium-review.googlesource.com/c/chromium/src/+/5512656 * 5516009: Accept mouse events in inactive window for Top Chrome WebUIs | https://chromium-review.googlesource.com/c/chromium/src/+/5516009 * 5376861: Change references to RWHVB in RWHIER and RenderWidgetTargeter to RWHVI. | https://chromium-review.googlesource.com/c/chromium/src/+/5376861 * 5490530: Use partition_alloc PA_BUILDFLAG(...) outside PA. #cleanup | https://chromium-review.googlesource.com/c/chromium/src/+/5490530 * 5296870: network: Allow trusted loaders to learn the sent request cookies. | https://chromium-review.googlesource.com/c/chromium/src/+/5296870 * 5453438: Delegate delegated ink trails to RWHI from RWHIER. | https://chromium-review.googlesource.com/c/chromium/src/+/5453438 * chore: update patches * chore: bump chromium in DEPS to 126.0.6472.0 * chore: bump chromium in DEPS to 126.0.6474.0 * chore: update patches * chore: bump chromium in DEPS to 126.0.6476.0 * chore: bump chromium in DEPS to 126.0.6478.0 * chore: bump chromium in DEPS to 126.0.6478.3 * chore: bump chromium in DEPS to 126.0.6478.8 * update patches * only disable enterprise_cloud_content_analysis * 5403888: [api] support v8::Data in v8::TracedReference and v8::EmbedderGraph https://chromium-review.googlesource.com/c/v8/v8/+/5403888 * chore: bump chromium in DEPS to 127.0.6484.0 * chore: bump chromium in DEPS to 127.0.6485.0 * 5539004: Use NOTREACHED_IN_MIGRATION() in remaining chrome/ | https://chromium-review.googlesource.com/c/chromium/src/+/5539004 * src: cast to v8::Value before using v8::EmbedderGraph::V8Node | https://github.com/nodejs/node/pull/52638/files * chore: update patches * chore: update v8 patches * chore: bump chromium in DEPS to 127.0.6486.0 * chore: bump chromium in DEPS to 127.0.6488.0 * chore: bump chromium in DEPS to 127.0.6490.0 * chore: bump chromium in DEPS to 127.0.6492.0 * chore: update patches For some reason, `feat_expose_raw_response_headers_from_urlloader.patch` got messed up in an earlier commit. * chore: update patches printing.patch was updated due to https://chromium-review.googlesource.com/c/chromium/src/+/5535938 * 5527572: Move Connectors prefs files to components/enterprise/connectors/ https://chromium-review.googlesource.com/c/chromium/src/+/5527572 * chore: bump chromium in DEPS to 127.0.6494.0 * chore: bump chromium in DEPS to 127.0.6495.0 * chore: bump chromium in DEPS to 127.0.6496.0 * 5465511: [api] Mark v8::ObjectTemplate::SetAccessor(..) for deprecation https://chromium-review.googlesource.com/c/v8/v8/+/5465511 * chore: revert v8 deprecation See patch message for more details. https://chromium-review.googlesource.com/c/v8/v8/+/5526611 * chore: update patches * 5538771: Remove srcdoc else-if block in CalculateOrigin() https://chromium-review.googlesource.com/c/chromium/src/+/5538771 * 5522321: [devtools] Support saving base64 encoded files via host bindings https://chromium-review.googlesource.com/c/chromium/src/+/5522321 * 5376861: Change references to RWHVB in RWHIER and RenderWidgetTargeter to RWHVI. https://chromium-review.googlesource.com/c/chromium/src/+/5376861 * 5530163: [media] Use VideoFrame::Plane typed enum instead of nameless enum https://chromium-review.googlesource.com/c/chromium/src/+/5530163 * 5463431: iwa: Only create IsolatedWebAppURLLoaderFactory for subresources in IWAs https://chromium-review.googlesource.com/c/chromium/src/+/5463431 * fixup! 5465511: [api] Mark v8::ObjectTemplate::SetAccessor(..) for deprecation https://chromium-review.googlesource.com/c/v8/v8/+/5465511 * 5512176: Remove OnEnvironmentEstimationComplete() https://chromium-review.googlesource.com/c/chromium/src/+/5512176 * 5528282: Move Web Speech API .mojom files to //media/mojo/mojom https://chromium-review.googlesource.com/c/chromium/src/+/5528282 * 5513740: Reland "[Extensions] Restructure extensions::ProcessMap" https://chromium-review.googlesource.com/c/chromium/src/+/5513740 * 5483406: [PEPC] Make PEPC permission subscription take into account device status https://chromium-review.googlesource.com/c/chromium/src/+/5483406 * 5526034: [DoH] Remove kDnsOverHttps feature flag https://chromium-review.googlesource.com/c/chromium/src/+/5526034 The title is a bit misleading. They removed handling for the feature flag and generally intend to remove it but haven't yet. I only changed our code to address the flag that was removed. A quick search on GitHub for `DnsOverHttpsFallback` yielded a few results, but they were all C++ chromium code or patches, 0 app code or discussion results. Since I couldn't find any evidence of this flag being used in developer applications, I've chosen to exclude this change from the breaking changes docs. * chore: revert v8 removal https://chromium-review.googlesource.com/c/v8/v8/+/5497515 See patch message for more details. * chore: cherry-pick Node.js patch for V8 API removal fix Node.js PR: nodejs/node#52996 V8 API Removal CL: https://chromium-review.googlesource.com/c/v8/v8/+/5539888 See the patch description for more details. * 5492183: Extensions: CodeHealth: Give enums some class https://chromium-review.googlesource.com/c/chromium/src/+/5492183 * fixup! 5528282: Move Web Speech API .mojom files to //media/mojo/mojom https://chromium-review.googlesource.com/c/chromium/src/+/5528282 * 5514687: Reland "Add a secret handshake to the base::Feature constructor" https://chromium-review.googlesource.com/c/chromium/src/+/5514687 * fixup! 5530163: [media] Use VideoFrame::Plane typed enum instead of nameless enum https://chromium-review.googlesource.com/c/chromium/src/+/5530163 * 5466238: PDF Viewer: add metrics to record if PDF is opened with a11y https://chromium-review.googlesource.com/c/chromium/src/+/5466238 * 5502081: Migrate OnDisplayRemoved to OnDisplaysRemoved https://chromium-review.googlesource.com/c/chromium/src/+/5502081 * 5539888: [api] Remove several APIs deprecated in version 12.6 https://chromium-review.googlesource.com/c/v8/v8/+/5539888 This commit essentially only removes the `only_terminate_in_safe_scope` isolate creation parameter. This undoes some work that was originally done in #35766. * 5498236: Make browser_tests force full async initialization for OSCrypt Async https://chromium-review.googlesource.com/c/chromium/src/+/5498236 * fixup! 5528282: Move Web Speech API .mojom files to //media/mojo/mojom https://chromium-review.googlesource.com/c/chromium/src/+/5528282 * 5545807: Migrate most remaining NOTREACHED() https://chromium-review.googlesource.com/c/chromium/src/+/5545807 I took a systematic approach to modifying all of our uses of `NOTREACHED` that were causing errors: * If there was a `return` or `break` (etc.) immediately after `NOTREACHED`, I removed the control flow instruction and left the `NOTREACHED` unmodified * All other instances were migrated to `NOTREACHED_IN_MIGRATION` We should revisit pretty much all usage of `NOTREACHED` as an upgrade follow-up item. * fixup! 5526034: [DoH] Remove kDnsOverHttps feature flag https://chromium-review.googlesource.com/c/chromium/src/+/5526034 Turns out the feature flags were removed in the `.cc` file, but not the `.h` feature list file. This means that the feature flags are pretty much officially gone. (The leftover symbols in the header are likely an oversight from what I can gather.) We may potentially decide to put this in the breaking changes doc if we decide this feature flag is important enough to highlight. * chore: bump chromium in DEPS to 127.0.6498.3 * chore: bump chromium in DEPS to 127.0.6500.0 * chore: bump chromium in DEPS to 127.0.6502.0 * chore: bump chromium in DEPS to 127.0.6504.0 * chore: bump chromium in DEPS to 127.0.6505.0 * chore: bump chromium in DEPS to 127.0.6508.0 * build: use Sha256Sum in script/sysroots.json Xref: https://chromium-review.googlesource.com/c/chromium/src/+/5506275 * chore: update chore_add_electron_deps_to_gitignores.patch Xref: no manual changes; patch applied with fuzz 2 * chore: update feat_allow_code_cache_in_custom_schemes.patch Xref: no manual changes; patch applied with fuzz 1 * chore: e patches all * fixup! build: use Sha256Sum in script/sysroots.json `sync` succeeds now * chore: replace absl::optional with std::optional Xref: https://chromium-review.googlesource.com/c/chromium/src/+/5253843 * chore: update CalculatePreferredSize() to new upstream semantics Xref: https://chromium-review.googlesource.com/c/chromium/src/+/5459174 Xref: https://chromium-review.googlesource.com/c/chromium/src/+/5541220 Xref: https://chromium-review.googlesource.com/c/chromium/src/+/5514708 Xref: https://chromium-review.googlesource.com/c/chromium/src/+/5504212 Xref: https://chromium-review.googlesource.com/516542 * chore: replace absl::optional with std::optional Xref: https://chromium-review.googlesource.com/c/chromium/src/+/5296147 * chore: add kPip to enumeration as a no-op https://chromium-review.googlesource.com/c/chromium/src/+/5546257 * [Autofill] Remove RenderFrame::ElementBoundsInWindow() Xref: https://chromium-review.googlesource.com/c/chromium/src/+/5553982 * chore: fix feat_add_streaming-protocol_registry_to_multibuffer_data_source.patch need new header to pick up definition of BLINK_PLATFORM_EXPORT macro Xref: https://chromium-review.googlesource.com/c/chromium/src/+/5463143 * chore: bump chromium in DEPS to 127.0.6510.0 * chore: update patches * chore: fix include path for native_web_keyboard_event.h Xref: https://chromium-review.googlesource.com/c/chromium/src/+/5541976 * chore: add currently-unused should_include_device_status arg to GetPermissionStatusForCurrentDocument() Xref: https://chromium-review.googlesource.com/c/chromium/src/+/5545382 * chore: bump chromium in DEPS to 127.0.6512.0 * chore: update mas_avoid_private_macos_api_usage.patch.patch No manual changes; patch applied with fuzz 1 * chore: update feat_add_streaming-protocol_registry_to_multibuffer_data_source.patch No manual changes; patch applied with fuzz 1 * chore: update webview_fullscreen.patch No manual changes; patch applied with fuzz 1 * chore=: remove cherry-pick-22db6918bac9.patch already present upstream * chore: remove nonexistent patchfiles from .patches * chore: remove cherry-pick-3e037e195e50.patch no longer needed; merged upstream * Update namespace for files moved to //components/input Xref: https://chromium-review.googlesource.com/c/chromium/src/+/5563251 * Require client for InitParams to always specify an ownership mode. Xref: https://chromium-review.googlesource.com/c/chromium/src/+/5532482 Xref: https://chromium-review.googlesource.com/c/chromium/src/+/5578714 * chore: e patches all * fixup! Update namespace for files moved to //components/input * chore: remove profile_keyed_service_factory, profile_selections from chromium_src already being linked in via chrome browser for printing * chore: bump chromium in DEPS to 127.0.6515.0 * chore: bump chromium in DEPS to 127.0.6516.0 * chore: update render_widget_host_view_base.patch Xref: https://chromium-review.googlesource.com/c/chromium/src/+/5547803 patch applied manually due to simple upstream shear * chore: update feat_allow_code_cache_in_custom_schemes.patch No manual changes; patch applied with fuzz 1 * chore: e patches all * Pull RWHIER and RWT to //content/common/input. Xref: https://chromium-review.googlesource.com/c/chromium/src/+/5397681 * chore: bump chromium in DEPS to 127.0.6517.0 * chore: update patches * fixup: Update namespace for files moved to //components/input * Remove 0-arg (default) constructor for views::Widget::InitParams. https://chromium-review.googlesource.com/c/chromium/src/+/5578714 * fixup: only disable enterprise_cloud_content_analysis The original commit a5480ac, was due to this CL 5527572: Move Connectors prefs files to components/enterprise/connectors/ | https://chromium-review.googlesource.com/c/chromium/src/+/5527572 * chore: bump chromium in DEPS to 127.0.6519.0 * chore: update patches * src: do not use deprecated V8 API nodejs/node#53084 * src: remove dependency on wrapper-descriptor-based cpp heap nodejs/node#53086 * 5344413: [DevTools] Add `getHostConfig` UI binding for sending status of `base::Features` to DevTools https://chromium-review.googlesource.com/c/chromium/src/+/5344413 * 5585788: Extensions: ManifestHandler: Separate Registry like ExtensionRegistry https://chromium-review.googlesource.com/c/chromium/src/+/5585788 * chore: update filenames.libcxx.gni * 5506857: Reland "Migrate clang-format to gcs first class deps" https://chromium-review.googlesource.com/c/chromium/src/+/5506857 * fixup: 5539888: [api] Remove several APIs deprecated in version 12.6 * fixup: 5506857: Reland Migrate clang-format to gcs first class deps * chore: bump chromium in DEPS to 127.0.6521.0 * chore: update patches * spec: update navigator.keyboard should lock the keyboard * Block or allow all MIDI using the existing SysEx permission Refs https://chromium-review.googlesource.com/c/chromium/src/+/5154368 Refs https://chromium-review.googlesource.com/c/chromium/src/+/5499157 * spec: update test/parallel/test-v8-stats * views: remove CalculatePreferredSize() Refs https://chromium-review.googlesource.com/c/chromium/src/+/5504212 * chore: update patches after rebase * 5560288: Re-enable ChromeOS XNNPack on Intel only https://chromium-review.googlesource.com/c/chromium/src/+/5560288 * chore: add nan patches for v8 changes Refs 5539888: [api] Remove several APIs deprecated in version 12.6 | https://chromium-review.googlesource.com/c/v8/v8/+/5539888 and 5539852: [heap][api] Remove deprecated v8::Isolate::IdleNotificationDeadline | https://chromium-review.googlesource.com/c/v8/v8/+/5539852 * 5573603: Modularize //chrome/browser/themes https://chromium-review.googlesource.com/c/chromium/src/+/5573603 * 5539888: [api] Remove several APIs deprecated in version 12.6 https://chromium-review.googlesource.com/c/v8/v8/+/5539888 * chore: update patches * test: fixup navigator.keyboard.lock on Windows * chore: remove unneeded profile target --------- Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com> Co-authored-by: Keeley Hammond <[email protected]> Co-authored-by: VerteDinde <[email protected]> Co-authored-by: Jeremy Rose <[email protected]> Co-authored-by: clavin <[email protected]> Co-authored-by: Charles Kerr <[email protected]> Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com> Co-authored-by: John Kleinschmidt <[email protected]> Co-authored-by: deepak1556 <[email protected]>

nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels May 21, 2024

joyeecheung mentioned this pull request May 21, 2024

src: remove dependency on wrapper-descriptor-based cpp heap nodejs/node-v8#284

Closed

joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label May 21, 2024

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 21, 2024

targos and others added 16 commits May 28, 2024 10:26

deps: update V8 to 12.7.137

3566324

build: reset embedder string to "-node.0"

512a7b9

src: update NODE_MODULE_VERSION to 128

47be3ba

Major V8 updates are usually API/ABI incompatible with previous versions. This commit adapts NODE_MODULE_VERSION for V8 12.7. Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md

deps: avoid compilation error with ASan

9b83b8e

Refs: https://bugs.chromium.org/p/v8/issues/detail?id=14221 PR-URL: nodejs#49639 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

deps: patch V8 to avoid duplicated zlib symbol

4da65e9

PR-URL: nodejs#49639 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>

deps: silence internal V8 deprecation warning

35ebfb1

PR-URL: nodejs#52293 Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Richard Lau <[email protected]>

deps: patch V8 to support compilation with MSVC

38a5e0b

After enabling -std:c++20 on Windows, patch is now much smaller. PR-URL: nodejs#52465 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Michael Dawson <[email protected]>

build: include v8-sandbox.h header in distribution

9d8823a

Refs: v8/v8@5495678

src: add source location to v8::TaskRunner

8d34ef7

Refs: https://chromium-review.googlesource.com/c/v8/v8/+/5300826

build: disable ICF for mksnapshot

f553d5d

Refs: https://chromium-review.googlesource.com/c/v8/v8/+/5447267 Co-authored-by: Michaël Zasso <[email protected]>

test: update v8-stats test for V8 12.6

54ad3e4

Refs: v8/v8@e30e228

tools: update V8 gypfiles for 12.6

a1c9178

Refs: v8/v8@f8fa220

targos force-pushed the canary-base branch from 9dc716f to a1c9178 Compare May 28, 2024 09:07

joyeecheung force-pushed the cppheap-auto branch from 9ea7f9b to 1a14799 Compare May 29, 2024 18:28

github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels May 29, 2024

joyeecheung mentioned this pull request May 29, 2024

src: use V8-owned CppHeap #53205

Draft

jasnell reviewed May 29, 2024

View reviewed changes

src/env.cc Outdated Show resolved Hide resolved

jasnell reviewed May 29, 2024

View reviewed changes

src/env.cc Outdated Show resolved Hide resolved

jasnell reviewed May 29, 2024

View reviewed changes

joyeecheung added 2 commits May 30, 2024 15:45

joyeecheung force-pushed the cppheap-auto branch from d48d61a to 6c7c7c5 Compare May 30, 2024 13:45

legendecas approved these changes Jun 4, 2024

View reviewed changes

targos force-pushed the canary-base branch from a1c9178 to 30329d0 Compare June 4, 2024 11:22

targos closed this Jun 4, 2024

targos mentioned this pull request Jun 4, 2024

v8::WrapperDescriptor is deprecated nodejs/node-v8#283

Closed

jkleinsc added a commit to electron/electron that referenced this pull request Jun 4, 2024

src: remove dependency on wrapper-descriptor-based cpp heap

747dc81

nodejs/node#53086

jkleinsc added a commit to electron/electron that referenced this pull request Jun 6, 2024

src: remove dependency on wrapper-descriptor-based cpp heap

97f37bd

nodejs/node#53086

jkleinsc added a commit to electron/electron that referenced this pull request Jun 7, 2024

src: remove dependency on wrapper-descriptor-based cpp heap

7b70c5d

nodejs/node#53086

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

src: remove dependency on wrapper-descriptor-based cpp heap #53086

src: remove dependency on wrapper-descriptor-based cpp heap #53086

joyeecheung commented May 21, 2024 •

edited

Loading

nodejs-github-bot commented May 21, 2024

targos commented May 22, 2024

joyeecheung commented May 22, 2024

addaleax commented May 23, 2024

joyeecheung commented May 27, 2024 •

edited

Loading

targos commented May 28, 2024

targos commented May 29, 2024

github-actions bot commented May 29, 2024

nodejs-github-bot commented May 29, 2024

joyeecheung commented May 29, 2024

jasnell May 29, 2024

joyeecheung May 30, 2024

jasnell May 29, 2024

joyeecheung May 30, 2024 •

edited

Loading

targos commented May 30, 2024

joyeecheung commented May 30, 2024

joyeecheung commented May 30, 2024

nodejs-github-bot commented Jun 1, 2024

joyeecheung commented Jun 4, 2024

targos commented Jun 4, 2024

targos commented Jun 4, 2024

src: remove dependency on wrapper-descriptor-based cpp heap #53086

src: remove dependency on wrapper-descriptor-based cpp heap #53086

Conversation

joyeecheung commented May 21, 2024 • edited Loading

nodejs-github-bot commented May 21, 2024

targos commented May 22, 2024

joyeecheung commented May 22, 2024

addaleax commented May 23, 2024

joyeecheung commented May 27, 2024 • edited Loading

targos commented May 28, 2024

targos commented May 29, 2024

github-actions bot commented May 29, 2024

nodejs-github-bot commented May 29, 2024

joyeecheung commented May 29, 2024

jasnell May 29, 2024

Choose a reason for hiding this comment

joyeecheung May 30, 2024

Choose a reason for hiding this comment

jasnell May 29, 2024

Choose a reason for hiding this comment

joyeecheung May 30, 2024 • edited Loading

Choose a reason for hiding this comment

targos commented May 30, 2024

joyeecheung commented May 30, 2024

joyeecheung commented May 30, 2024

nodejs-github-bot commented Jun 1, 2024

joyeecheung commented Jun 4, 2024

targos commented Jun 4, 2024

targos commented Jun 4, 2024

joyeecheung commented May 21, 2024 •

edited

Loading

joyeecheung commented May 27, 2024 •

edited

Loading

joyeecheung May 30, 2024 •

edited

Loading