src: add kNoBrowserGlobals flag for Environment by zcbenz · Pull Request #40532 · nodejs/node

zcbenz · 2021-10-20T09:52:02Z

This is the runtime equivalent of the configure --no-browser-globals build flag.

A runtime flag is needed because embedders can have multiple modes that, Node.js may both run in a browser environment, and in an independent environment that has no browser globals. For example, Node.js script running in web page spawning a script with child_process.fork.

This PR also moves the creation of browser globals into a separate script, so it can be controlled with C++ flag without affecting bootstrapping in snapshot.

/cc @nodejs/embedders @joyeecheung @addaleax

src/node.cc

joyeecheung

LGTM. Thanks for your patience!

nodejs-github-bot · 2021-10-25T04:49:40Z

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

nodejs-github-bot · 2021-10-27T15:24:59Z

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

targos · 2022-01-14T15:50:32Z

Should this be backported to v17.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.
There is a conflict because of previous semver-major changes.

zcbenz · 2022-01-17T11:00:06Z

We don't need this backported to 17 in Electron.

danielleadams · 2022-01-31T04:50:53Z

@zcbenz this does not land cleanly in v16.x-staging. Do you mind creating a backport PR?

targos · 2022-01-31T05:59:48Z

@danielleadams this didn't land on v17.x. I'm surprised it shows up in your branch diff

PR-URL: nodejs#40532 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Minwoo Jung <nodecorelab@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

BethGriggs · 2022-02-01T11:45:38Z

@danielleadams this didn't land on v17.x. I'm surprised it shows up in your branch diff

I wonder if that's potentially due to nodejs/branch-diff#45?

nodejs/node#40532

* chore: update to Node.js v18 * child_process: improve argument validation nodejs/node#41305 * bootstrap: support configure-time user-land snapshot nodejs/node#42466 * chore: update GN patch * src: disambiguate terms used to refer to builtins and addons nodejs/node#44135 * src: use a typed array internally for process._exiting nodejs/node#43883 * chore: lib/internal/bootstrap -> lib/internal/process * src: disambiguate terms used to refer to builtins and addons nodejs/node#44135 * chore: remove redudant browserGlobals patch * chore: update BoringSSL patch * src: allow embedder-provided PageAllocator in NodePlatform nodejs/node#38362 * chore: fixup Node.js crypto tests - nodejs/node#44171 - nodejs/node#41600 * lib: add Promise methods to avoid-prototype-pollution lint rule nodejs/node#43849 * deps: update V8 to 10.1 nodejs/node#42657 * src: add kNoBrowserGlobals flag for Environment nodejs/node#40532 * chore: consolidate asar initialization patches * deps: update V8 to 10.1 nodejs/node#42657 * deps: update V8 to 9.8 nodejs/node#41610 * src,crypto: remove AllocatedBuffers from crypto_spkac nodejs/node#40752 * build: enable V8's shared read-only heap nodejs/node#42809 * src: fix ssize_t error from nghttp2.h nodejs/node#44393 * chore: fixup ESM patch * chore: fixup patch indices * src: merge NativeModuleEnv into NativeModuleLoader nodejs/node#43824 * [API] Pass OOMDetails to OOMErrorCallback https://chromium-review.googlesource.com/c/v8/v8/+/3647827 * src: iwyu in cleanup_queue.cc * src: return Maybe from a couple of functions nodejs/node#39603 * src: clean up embedder API nodejs/node#35897 * src: refactor DH groups to delete crypto_groups.h nodejs/node#43896 * deps,src: use SIMD for normal base64 encoding nodejs/node#39775 * chore: remove deleted source file * chore: update patches * chore: remove deleted source file * lib: add fetch nodejs/node#41749 * chore: remove nonexistent node specs * test: split report OOM tests nodejs/node#44389 * src: trace fs async api nodejs/node#44057 * http: trace http request / response nodejs/node#44102 * test: split test-crypto-dh.js nodejs/node#40451 * crypto: introduce X509Certificate API nodejs/node#36804 * src: split property helpers from node::Environment nodejs/node#44056 * nodejs/node#38905 bootstrap: implement run-time user-land snapshots via --build-snapshot and --snapshot-blob * lib,src: implement WebAssembly Web API nodejs/node#42701 * fixup! deps,src: use SIMD for normal base64 encoding * fixup! src: refactor DH groups to delete crypto_groups.h * chore: fixup base64 GN file * fix: check that node::InitializeContext() returns true * chore: delete _noBrowserGlobals usage * chore: disable fetch in renderer procceses * dns: default to verbatim=true in dns.lookup() nodejs/node#39987 Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>

nodejs-github-bot added 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 Oct 20, 2021

zcbenz force-pushed the no-browser-globals branch from 51d469f to 8278b99 Compare October 20, 2021 10:04

addaleax approved these changes Oct 20, 2021

View reviewed changes

addaleax reviewed Oct 20, 2021

View reviewed changes

src/node.cc Outdated Show resolved Hide resolved

joyeecheung approved these changes Oct 20, 2021

View reviewed changes

This comment has been minimized.

Sign in to view

jasnell approved these changes Oct 20, 2021

View reviewed changes

github-actions bot mentioned this pull request Oct 21, 2021

CI Reliability 2021-10-21 nodejs/reliability#93

Open

30 tasks

zcbenz force-pushed the no-browser-globals branch from 8278b99 to c935b6a Compare October 21, 2021 00:03

JungMinu approved these changes Oct 21, 2021

View reviewed changes

benjamingr approved these changes Oct 21, 2021

View reviewed changes

addaleax added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. and removed needs-ci PRs that need a full CI run. labels Oct 21, 2021

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

This comment has been minimized.

Sign in to view

This was referenced Oct 22, 2021

CI Reliability 2021-10-22 nodejs/reliability#94

Open

CI Reliability 2021-10-23 nodejs/reliability#95

Open

CI Reliability 2021-10-24 nodejs/reliability#96

Open

CI Reliability 2021-10-25 nodejs/reliability#97

Open

VoltrexKeyva added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Oct 25, 2021

github-actions bot mentioned this pull request Oct 26, 2021

CI Reliability 2021-10-26 nodejs/reliability#98

Open

25 tasks

JungMinu approved these changes Oct 26, 2021

View reviewed changes

github-actions bot mentioned this pull request Oct 27, 2021

CI Reliability 2021-10-27 nodejs/reliability#99

Open

26 tasks

This was referenced Oct 28, 2021

CI Reliability 2021-10-28 nodejs/reliability#100

Open

CI Reliability 2021-10-29 nodejs/reliability#101

Open

github-actions bot mentioned this pull request Jan 9, 2022

CI Reliability 2022-01-09 nodejs/reliability#170

Open

18 tasks

targos added the backport-requested-v17.x label Jan 14, 2022

targos added dont-land-on-v17.x and removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backport-requested-v17.x labels Jan 17, 2022

danielleadams added the backport-requested-v16.x label Jan 31, 2022

targos added dont-land-on-v16.x and removed backport-requested-v16.x labels Jan 31, 2022

targos mentioned this pull request Feb 1, 2022

[v16.x] Fix JS linting warnings and errors #41805

Closed

BethGriggs mentioned this pull request Apr 9, 2022

2022-04-19, Version 18.0.0 (Current) #42262

Merged

aduh95 mentioned this pull request Apr 20, 2022

lib: stop installing fetch if no_browser_globals is true #41969

Merged

juanarbol mentioned this pull request Sep 30, 2022

perf_hooks: add resourcetiming buffer limit #44220

Closed

codebytere added a commit to electron/electron that referenced this pull request Oct 12, 2022

src: add kNoBrowserGlobals flag for Environment

fa6c1e7

nodejs/node#40532

codebytere added a commit to electron/electron that referenced this pull request Oct 17, 2022

src: add kNoBrowserGlobals flag for Environment

28b9d50

nodejs/node#40532

codebytere added a commit to electron/electron that referenced this pull request Oct 19, 2022

src: add kNoBrowserGlobals flag for Environment

8be4e55

nodejs/node#40532

codebytere added a commit to electron/electron that referenced this pull request Oct 19, 2022

src: add kNoBrowserGlobals flag for Environment

e2724e3

nodejs/node#40532

codebytere added a commit to electron/electron that referenced this pull request Oct 24, 2022

src: add kNoBrowserGlobals flag for Environment

c7c981b

nodejs/node#40532

codebytere added a commit to electron/electron that referenced this pull request Nov 8, 2022

src: add kNoBrowserGlobals flag for Environment

dfd9437

nodejs/node#40532

codebytere added a commit to electron/electron that referenced this pull request Nov 8, 2022

src: add kNoBrowserGlobals flag for Environment

ae769ce

nodejs/node#40532

MarshallOfSound mentioned this pull request Feb 27, 2023

[Bug]: BroadcastChannel support broken in version 23 electron/electron#37417

Closed

3 tasks

This was referenced Feb 27, 2023

fix: BroadcastChannel initialization location electron/electron#37421

Merged

lib: fix BroadcastChannel initialization location #46864

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

src: add kNoBrowserGlobals flag for Environment#40532

src: add kNoBrowserGlobals flag for Environment#40532
nodejs-github-bot merged 1 commit intonodejs:masterfrom
zcbenz:no-browser-globals

zcbenz commented Oct 20, 2021

Uh oh!

Uh oh!

joyeecheung left a comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

nodejs-github-bot commented Oct 25, 2021

Uh oh!

nodejs-github-bot commented Oct 27, 2021

Uh oh!

targos commented Jan 14, 2022

Uh oh!

zcbenz commented Jan 17, 2022

Uh oh!

danielleadams commented Jan 31, 2022

Uh oh!

targos commented Jan 31, 2022

Uh oh!

BethGriggs commented Feb 1, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

Uh oh!

Conversation

zcbenz commented Oct 20, 2021

Uh oh!

Uh oh!

joyeecheung left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

nodejs-github-bot commented Oct 25, 2021

Uh oh!

nodejs-github-bot commented Oct 27, 2021

Uh oh!

targos commented Jan 14, 2022

Uh oh!

zcbenz commented Jan 17, 2022

Uh oh!

danielleadams commented Jan 31, 2022

Uh oh!

targos commented Jan 31, 2022

Uh oh!

BethGriggs commented Feb 1, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants