GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — src: add kNoBrowserGlobals flag for Environment by zcbenz · Pull Request #40532 · nodejs/node

Conversation

@zcbenz

@zcbenz zcbenz commented Oct 20, 2021

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bricss reacted with eyes emoji
All reactions
  • 👀 1 reaction
@nodejs-github-bot nodejs-github-bot added lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Oct 20, 2021
@zcbenz
zcbenz force-pushed the no-browser-globals branch from 51d469f to 8278b99 Compare October 20, 2021 10:04
addaleax
addaleax approved these changes Oct 20, 2021
View reviewed changes
addaleax
addaleax reviewed Oct 20, 2021
View reviewed changes
Comment thread src/node.cc Resolved

Uh oh!

There was an error while loading. Please reload this page.

joyeecheung
joyeecheung approved these changes Oct 20, 2021
View reviewed changes

@joyeecheung joyeecheung left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for your patience!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@nodejs-github-bot

This comment has been minimized.

Sign in to view
jasnell
jasnell approved these changes Oct 20, 2021
View reviewed changes
@github-actions github-actions Bot mentioned this pull request Oct 21, 2021
CI Reliability 2021-10-21 nodejs/reliability#93
Open
30 tasks
@zcbenz
zcbenz force-pushed the no-browser-globals branch from 8278b99 to c935b6a Compare October 21, 2021 00:03
JungMinu
JungMinu approved these changes Oct 21, 2021
View reviewed changes
benjamingr
benjamingr approved these changes Oct 21, 2021
View reviewed changes
@addaleax addaleax added request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. author ready PRs with CI started, the required approvals, and no outstanding review comments. and removed needs-ci PRs that need a full CI run. labels Oct 21, 2021
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Oct 21, 2021
@nodejs-github-bot

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 VoltrexKeyva added request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. and removed request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Oct 25, 2021
@nodejs-github-bot

nodejs-github-bot commented Oct 25, 2021

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@github-actions github-actions Bot mentioned this pull request Oct 26, 2021
CI Reliability 2021-10-26 nodejs/reliability#98
Open
25 tasks
JungMinu
JungMinu approved these changes Oct 26, 2021
View reviewed changes
@github-actions github-actions Bot mentioned this pull request Oct 27, 2021
CI Reliability 2021-10-27 nodejs/reliability#99
Open
26 tasks
@nodejs-github-bot

nodejs-github-bot commented Oct 27, 2021

Copy link
Copy Markdown
Collaborator

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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 github-actions Bot mentioned this pull request Jan 9, 2022
CI Reliability 2022-01-09 nodejs/reliability#170
Open
18 tasks
@targos targos added the backport-requested-v17.x label Jan 14, 2022
@targos

targos commented Jan 14, 2022

Copy link
Copy Markdown
Member

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@zcbenz

zcbenz commented Jan 17, 2022

Copy link
Copy Markdown
Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@targos targos added dont-land-on-v17.x and removed author ready PRs with CI started, the required approvals, and no outstanding review comments. backport-requested-v17.x labels Jan 17, 2022
@danielleadams

danielleadams commented Jan 31, 2022

Copy link
Copy Markdown
Contributor

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

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

targos commented Jan 31, 2022

Copy link
Copy Markdown
Member

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

danielleadams reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@targos targos added dont-land-on-v16.x and removed backport-requested-v16.x labels Jan 31, 2022
@targos targos mentioned this pull request Feb 1, 2022
[v16.x] Fix JS linting warnings and errors #41805
Closed
@BethGriggs

BethGriggs commented Feb 1, 2022

Copy link
Copy Markdown
Member

@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?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@BethGriggs BethGriggs mentioned this pull request Apr 9, 2022
2022-04-19, Version 18.0.0 (Current) #42262
Merged
@aduh95 aduh95 mentioned this pull request Apr 20, 2022
lib: stop installing fetch if no_browser_globals is true #41969
Merged
@juanarbol 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
@codebytere
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
@codebytere
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
@codebytere
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
@codebytere
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
@codebytere
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
@codebytere
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
@codebytere
src: add kNoBrowserGlobals flag for Environment
ae769ce
nodejs/node#40532
codebytere added a commit to electron/electron that referenced this pull request Nov 10, 2022
@codebytere @patchup
chore: upgrade to Node.js v18 (#35999)
75d2caf
* 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>
khalwa pushed a commit to solarwindscloud/electron that referenced this pull request Feb 22, 2023
@codebytere @patchup @khalwa
chore: upgrade to Node.js v18 (electron#35999)
b19d67a
* 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>
@MarshallOfSound MarshallOfSound mentioned this pull request Feb 27, 2023
[Bug]: BroadcastChannel support broken in version 23 electron/electron#37417
Closed
3 tasks done
This was referenced Feb 27, 2023
fix: BroadcastChannel initialization location electron/electron#37421
Merged
lib: fix BroadcastChannel initialization location #46864
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@addaleax addaleax addaleax approved these changes
@jasnell jasnell jasnell approved these changes
@benjamingr benjamingr benjamingr approved these changes
@joyeecheung joyeecheung joyeecheung approved these changes
@JungMinu JungMinu JungMinu approved these changes
@aduh95 aduh95 aduh95 approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

lib / src Issues and PRs involving general changes in the lib/ or src/ directories.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

12 participants

@zcbenz @nodejs-github-bot @aduh95 @joyeecheung @targos @danielleadams @BethGriggs @jasnell @addaleax @benjamingr @JungMinu @VoltrexKeyva
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.