New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

buffer: fix ArrayBuffer checks #8453

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
6 participants
@mscdex
Contributor

mscdex commented Sep 8, 2016

Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)
  • buffer
Description of change

This commit fixes detection of ArrayBuffers from different V8 contexts. This is especially a problem for environments like nw.js where the node and browser V8 contexts are not shared.

buffer: fix ArrayBuffer checks
This commit fixes detection of ArrayBuffers from different V8 contexts.
This is especially a problem for environments like nw.js where the
node and browser V8 contexts are not shared.

@mscdex mscdex added the buffer label Sep 8, 2016

@mscdex

This comment has been minimized.

Show comment
Hide comment
@mscdex
Contributor

mscdex commented Sep 8, 2016

@mscdex

This comment has been minimized.

Show comment
Hide comment
@addaleax

This comment has been minimized.

Show comment
Hide comment
@addaleax

addaleax Sep 8, 2016

Member

LGTM

Member

addaleax commented Sep 8, 2016

LGTM

@bnoordhuis

This comment has been minimized.

Show comment
Hide comment
@bnoordhuis

bnoordhuis Sep 9, 2016

Member

LGTM

Member

bnoordhuis commented Sep 9, 2016

LGTM

@jasnell

This comment has been minimized.

Show comment
Hide comment
@jasnell

jasnell Sep 9, 2016

Member

LGTM

Member

jasnell commented Sep 9, 2016

LGTM

@thefourtheye

This comment has been minimized.

Show comment
Hide comment
@thefourtheye

thefourtheye Sep 10, 2016

Contributor

LGTM

Contributor

thefourtheye commented Sep 10, 2016

LGTM

jasnell added a commit that referenced this pull request Sep 12, 2016

buffer: fix ArrayBuffer checks
This commit fixes detection of ArrayBuffers from different V8 contexts.
This is especially a problem for environments like nw.js where the
node and browser V8 contexts are not shared.

PR-URL: #8453
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
@jasnell

This comment has been minimized.

Show comment
Hide comment
@jasnell

jasnell Sep 12, 2016

Member

Landed in 73bafa0

Member

jasnell commented Sep 12, 2016

Landed in 73bafa0

@jasnell jasnell closed this Sep 12, 2016

@mscdex mscdex deleted the mscdex:buffer-fix-arraybuffer-check branch Sep 12, 2016

Fishrock123 added a commit that referenced this pull request Sep 14, 2016

buffer: fix ArrayBuffer checks
This commit fixes detection of ArrayBuffers from different V8 contexts.
This is especially a problem for environments like nw.js where the
node and browser V8 contexts are not shared.

PR-URL: #8453
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>

 Conflicts:
	test/parallel/test-buffer-alloc.js
@MylesBorins

This comment has been minimized.

Show comment
Hide comment
@MylesBorins
Member

MylesBorins commented Sep 30, 2016

@mscdex lts?

@mscdex

This comment has been minimized.

Show comment
Hide comment
@mscdex

mscdex Oct 1, 2016

Contributor

@thealphanerd I would say so, but haven't looked to see if it's applicable.

Contributor

mscdex commented Oct 1, 2016

@thealphanerd I would say so, but haven't looked to see if it's applicable.

@MylesBorins

This comment has been minimized.

Show comment
Hide comment
@MylesBorins

MylesBorins Oct 11, 2016

Member

@mscdex it does not land cleanly fwiw

Member

MylesBorins commented Oct 11, 2016

@mscdex it does not land cleanly fwiw

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment