node/test/cctest
Anna Henningsen f5d42532a3
src: refactor BaseObject internal field management
- Instead of storing a pointer whose type refers to the specific
  subclass of `BaseObject`, just store a `BaseObject*` directly.
  This means in particular that one can cast to classes along
  the way of the inheritance chain without issues, and that
  `BaseObject*` no longer needs to be the first superclass
  in the case of multiple inheritance.

  In particular, this renders hack-y solutions to this problem (like
  ddc19be6de) obsolete and addresses
  a `TODO` comment of mine.

- Move wrapping/unwrapping methods to the `BaseObject` class.
  We use these almost exclusively for `BaseObject`s, and I hope
  that this gives a better idea of how (and for what) these are used
  in our code.

- Perform initialization/deinitialization of the internal field
  in the `BaseObject*` constructor/destructor. This makes the code
  a bit more obviously correct, avoids explicit calls for this
  in subclass constructors, and in particular allows us to avoid
  crash situations when we previously called `ClearWrap()`
  during GC.

  This also means that we enforce that the object passed to the
  `BaseObject` constructor needs to have an internal field.
  This is the only reason for the test change.

- Change the signature of `MakeWeak()` to not require a pointer
  argument. Previously, this would always have been the same
  as `this`, and no other value made sense. Also, the parameter
  was something that I personally found somewhat confusing
  when becoming familiar with Node’s code.

- Add a `TODO` comment that motivates switching to real inheritance
  for the JS types we expose from the native side. This patch
  brings us a lot closer to being able to do that.

- Some less significant drive-by cleanup.

Since we *effectively* already store the `BaseObject*` pointer
anyway since ddc19be6de, I do not
think that this is going to have any impact on diagnostic tooling.

Fixes: https://github.com/nodejs/node/issues/18897
PR-URL: https://github.com/nodejs/node/pull/20455
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
2018-05-08 14:22:25 -07:00
..
node_test_fixture.cc test: introduce SetUpTestCase/TearDownTestCase 2018-02-06 06:50:14 +01:00
node_test_fixture.h src: call CleanupHandles in FreeEnvironment 2018-03-15 15:39:11 +01:00
test_aliased_buffer.cc src: add incr/decr operators for Reference 2018-03-09 08:01:20 -08:00
test_base64.cc test: add basic cctest for base64.h 2017-04-08 11:57:40 +03:00
test_environment.cc src: set thread local env in CreateEnvironment 2018-02-16 07:27:08 +01:00
test_inspector_socket.cc inspector: report client-visible host and port 2018-04-02 10:20:30 -07:00
test_inspector_socket_server.cc test: properly tag anonymous namespaces 2018-02-08 18:43:36 -05:00
test_node_postmortem_metadata.cc src: refactor BaseObject internal field management 2018-05-08 14:22:25 -07:00
test_platform.cc src: limit foreground tasks draining loop 2018-05-04 11:49:32 -04:00
test_url.cc module: Allow runMain to be ESM 2017-09-07 15:18:32 -05:00
test_util.cc build: enable cctest to use generated objects 2017-03-24 08:28:43 -07:00