v8::ArrayBuffer::Externalize and v8::ArrayBuffer::IsExternal are deprecated

[targos]

We have warnings in canary.

node_v8.cc:

[855/962] CXX obj/src/libnode.node_v8.o
../../src/node_v8.cc: In function ‘void node::Initialize(v8::Local<v8::Object>, v8::Local<v8::Value>, v8::Local<v8::Context>, void*)’:
../../src/node_v8.cc:175:39: warning: ‘bool v8::ArrayBuffer::IsExternal() const’ is deprecated: With v8::BackingStore externalized ArrayBuffers are the same as ordinary ArrayBuffers. See http://crbug.com/v8/9908. [-Wdeprecated-declarations]
  175 |   if (!heap_statistics_ab->IsExternal())
      |                                       ^
In file included from ../../src/node.h:63,
                 from ../../src/node_v8.cc:22:
../../deps/v8/include/v8.h:5078:8: note: declared here
 5078 |   bool IsExternal() const;
      |        ^~~~~~~~~~
../../src/node_v8.cc:177:46: warning: ‘void v8::ArrayBuffer::Externalize(const std::shared_ptr<v8::BackingStore>&)’ is deprecated: This will be removed together with IsExternal. [-Wdeprecated-declarations]
  177 |         heap_statistics_ab->GetBackingStore());
      |                                              ^
In file included from ../../src/node.h:63,
                 from ../../src/node_v8.cc:22:
../../deps/v8/include/v8.h:5115:8: note: declared here
 5115 |   void Externalize(const std::shared_ptr<BackingStore>& backing_store);
      |        ^~~~~~~~~~~
../../src/node_v8.cc:212:44: warning: ‘bool v8::ArrayBuffer::IsExternal() const’ is deprecated: With v8::BackingStore externalized ArrayBuffers are the same as ordinary ArrayBuffers. See http://crbug.com/v8/9908. [-Wdeprecated-declarations]
  212 |   if (!heap_code_statistics_ab->IsExternal())
      |                                            ^
In file included from ../../src/node.h:63,
                 from ../../src/node_v8.cc:22:
../../deps/v8/include/v8.h:5078:8: note: declared here
 5078 |   bool IsExternal() const;
      |        ^~~~~~~~~~
../../src/node_v8.cc:214:51: warning: ‘void v8::ArrayBuffer::Externalize(const std::shared_ptr<v8::BackingStore>&)’ is deprecated: This will be removed together with IsExternal. [-Wdeprecated-declarations]
  214 |         heap_code_statistics_ab->GetBackingStore());
      |                                                   ^
In file included from ../../src/node.h:63,
                 from ../../src/node_v8.cc:22:
../../deps/v8/include/v8.h:5115:8: note: declared here
 5115 |   void Externalize(const std::shared_ptr<BackingStore>& backing_store);
      |        ^~~~~~~~~~~
../../src/node_v8.cc:277:45: warning: ‘bool v8::ArrayBuffer::IsExternal() const’ is deprecated: With v8::BackingStore externalized ArrayBuffers are the same as ordinary ArrayBuffers. See http://crbug.com/v8/9908. [-Wdeprecated-declarations]
  277 |   if (!heap_space_statistics_ab->IsExternal())
      |                                             ^
In file included from ../../src/node.h:63,
                 from ../../src/node_v8.cc:22:
../../deps/v8/include/v8.h:5078:8: note: declared here
 5078 |   bool IsExternal() const;
      |        ^~~~~~~~~~
../../src/node_v8.cc:279:52: warning: ‘void v8::ArrayBuffer::Externalize(const std::shared_ptr<v8::BackingStore>&)’ is deprecated: This will be removed together with IsExternal. [-Wdeprecated-declarations]
  279 |         heap_space_statistics_ab->GetBackingStore());
      |                                                    ^
In file included from ../../src/node.h:63,
                 from ../../src/node_v8.cc:22:
../../deps/v8/include/v8.h:5115:8: note: declared here
 5115 |   void Externalize(const std::shared_ptr<BackingStore>& backing_store);
      |        ^~~~~~~~~~~

node_messaging.cc:

[835/962] CXX obj/src/libnode.node_messaging.o
../../src/node_messaging.cc: In member function ‘v8::Maybe<bool> node::worker::Message::Serialize(node::Environment*, v8::Local<v8::Context>, v8::Local<v8::Value>, const TransferList&, v8::Local<v8::Object>)’:
../../src/node_messaging.cc:380:25: warning: ‘bool v8::ArrayBuffer::IsExternal() const’ is deprecated: With v8::BackingStore externalized ArrayBuffers are the same as ordinary ArrayBuffers. See http://crbug.com/v8/9908. [-Wdeprecated-declarations]
  380 |     if (!ab->IsExternal())
      |                         ^
In file included from ../../src/util.h:27,
                 from ../../src/aliased_buffer.h:7,
                 from ../../src/env.h:27,
                 from ../../src/node_messaging.h:6,
                 from ../../src/node_messaging.cc:1:
../../deps/v8/include/v8.h:5078:8: note: declared here
 5078 |   bool IsExternal() const;
      |        ^~~~~~~~~~
../../src/node_messaging.cc:381:36: warning: ‘void v8::ArrayBuffer::Externalize(const std::shared_ptr<v8::BackingStore>&)’ is deprecated: This will be removed together with IsExternal. [-Wdeprecated-declarations]
  381 |       ab->Externalize(backing_store);
      |                                    ^
In file included from ../../src/util.h:27,
                 from ../../src/aliased_buffer.h:7,
                 from ../../src/env.h:27,
                 from ../../src/node_messaging.h:6,
                 from ../../src/node_messaging.cc:1:
../../deps/v8/include/v8.h:5115:8: note: declared here
 5115 |   void Externalize(const std::shared_ptr<BackingStore>& backing_store);
      |        ^~~~~~~~~~~

node_http2.cc:

[832/962] CXX obj/src/libnode.node_http2.o
../../src/node_http2.cc: In constructor ‘node::http2::Http2Session::Http2Session(node::Environment*, v8::Local<v8::Object>, node::http2::nghttp2_session_type)’:
../../src/node_http2.cc:579:25: warning: ‘bool v8::ArrayBuffer::IsExternal() const’ is deprecated: With v8::BackingStore externalized ArrayBuffers are the same as ordinary ArrayBuffers. See http://crbug.com/v8/9908. [-Wdeprecated-declarations]
  579 |     if (!ab->IsExternal())
      |                         ^
In file included from ../../src/util.h:27,
                 from ../../src/aliased_buffer.h:7,
                 from ../../src/node_http2.cc:1:
../../deps/v8/include/v8.h:5078:8: note: declared here
 5078 |   bool IsExternal() const;
      |        ^~~~~~~~~~
../../src/node_http2.cc:580:44: warning: ‘void v8::ArrayBuffer::Externalize(const std::shared_ptr<v8::BackingStore>&)’ is deprecated: This will be removed together with IsExternal. [-Wdeprecated-declarations]
  580 |       ab->Externalize(ab->GetBackingStore());
      |                                            ^
In file included from ../../src/util.h:27,
                 from ../../src/aliased_buffer.h:7,
                 from ../../src/node_http2.cc:1:
../../deps/v8/include/v8.h:5115:8: note: declared here
 5115 |   void Externalize(const std::shared_ptr<BackingStore>& backing_store);
      |        ^~~~~~~~~~~

node_buffer.cc:

[820/962] CXX obj/src/libnode.node_buffer.o
../../src/node_buffer.cc: In function ‘v8::MaybeLocal<v8::Object> node::Buffer::New(node::Environment*, char*, size_t, node::Buffer::FreeCallback, void*)’:
../../src/node_buffer.cc:420:23: warning: ‘bool v8::ArrayBuffer::IsExternal() const’ is deprecated: With v8::BackingStore externalized ArrayBuffers are the same as ordinary ArrayBuffers. See http://crbug.com/v8/9908. [-Wdeprecated-declarations]
  420 |   if (!ab->IsExternal())
      |                       ^
In file included from ../../src/node.h:63,
                 from ../../src/node_buffer.h:25,
                 from ../../src/node_buffer.cc:22:
../../deps/v8/include/v8.h:5078:8: note: declared here
 5078 |   bool IsExternal() const;
      |        ^~~~~~~~~~
../../src/node_buffer.cc:421:42: warning: ‘void v8::ArrayBuffer::Externalize(const std::shared_ptr<v8::BackingStore>&)’ is deprecated: This will be removed together with IsExternal. [-Wdeprecated-declarations]
  421 |     ab->Externalize(ab->GetBackingStore());
      |                                          ^
In file included from ../../src/node.h:63,
                 from ../../src/node_buffer.h:25,
                 from ../../src/node_buffer.cc:22:
../../deps/v8/include/v8.h:5115:8: note: declared here
 5115 |   void Externalize(const std::shared_ptr<BackingStore>& backing_store);
      |        ^~~~~~~~~~~
../../src/node_buffer.cc: In function ‘void node::Buffer::{anonymous}::Initialize(v8::Local<v8::Object>, v8::Local<v8::Value>, v8::Local<v8::Context>, void*)’:
../../src/node_buffer.cc:1214:35: warning: ‘bool v8::ArrayBuffer::IsExternal() const’ is deprecated: With v8::BackingStore externalized ArrayBuffers are the same as ordinary ArrayBuffers. See http://crbug.com/v8/9908. [-Wdeprecated-declarations]
 1214 |     if (!array_buffer->IsExternal())
      |                                   ^
In file included from ../../src/node.h:63,
                 from ../../src/node_buffer.h:25,
                 from ../../src/node_buffer.cc:22:
../../deps/v8/include/v8.h:5078:8: note: declared here
 5078 |   bool IsExternal() const;
      |        ^~~~~~~~~~
../../src/node_buffer.cc:1215:64: warning: ‘void v8::ArrayBuffer::Externalize(const std::shared_ptr<v8::BackingStore>&)’ is deprecated: This will be removed together with IsExternal. [-Wdeprecated-declarations]
 1215 |       array_buffer->Externalize(array_buffer->GetBackingStore());
      |                                                                ^
In file included from ../../src/node.h:63,
                 from ../../src/node_buffer.h:25,
                 from ../../src/node_buffer.cc:22:
../../deps/v8/include/v8.h:5115:8: note: declared here
 5115 |   void Externalize(const std::shared_ptr<BackingStore>& backing_store);
      |        ^~~~~~~~~~~

js_native_api_v8.cc:

[814/962] CXX obj/src/libnode.js_native_api_v8.o
../../src/js_native_api_v8.cc: In function ‘napi_status napi_create_external_arraybuffer(napi_env, void*, size_t, napi_finalize, void*, napi_value__**)’:
../../src/js_native_api_v8.cc:2625:27: warning: ‘bool v8::ArrayBuffer::IsExternal() const’ is deprecated: With v8::BackingStore externalized ArrayBuffers are the same as ordinary ArrayBuffers. See http://crbug.com/v8/9908. [-Wdeprecated-declarations]
 2625 |   if (!buffer->IsExternal())
      |                           ^
In file included from ../../src/util.h:27,
                 from ../../src/aliased_buffer.h:7,
                 from ../../src/env-inl.h:27,
                 from ../../src/js_native_api_v8.cc:5:
../../deps/v8/include/v8.h:5078:8: note: declared here
 5078 |   bool IsExternal() const;
      |        ^~~~~~~~~~
../../src/js_native_api_v8.cc:2626:50: warning: ‘void v8::ArrayBuffer::Externalize(const std::shared_ptr<v8::BackingStore>&)’ is deprecated: This will be removed together with IsExternal. [-Wdeprecated-declarations]
 2626 |     buffer->Externalize(buffer->GetBackingStore());
      |                                                  ^
In file included from ../../src/util.h:27,
                 from ../../src/aliased_buffer.h:7,
                 from ../../src/env-inl.h:27,
                 from ../../src/js_native_api_v8.cc:5:
../../deps/v8/include/v8.h:5115:8: note: declared here
 5115 |   void Externalize(const std::shared_ptr<BackingStore>& backing_store);
      |        ^~~~~~~~~~~
In file included from ../../src/js_native_api_v8.cc:6:
../../src/js_native_api_v8.cc: In function ‘napi_status napi_detach_arraybuffer(napi_env, napi_value)’:
../../src/js_native_api_v8.cc:3078:27: warning: ‘bool v8::ArrayBuffer::IsExternal() const’ is deprecated: With v8::BackingStore externalized ArrayBuffers are the same as ordinary ArrayBuffers. See http://crbug.com/v8/9908. [-Wdeprecated-declarations]
 3078 |       env, it->IsExternal(), napi_detachable_arraybuffer_expected);
      |                           ^
../../src/js_native_api_v8.h:107:11: note: in definition of macro ‘RETURN_STATUS_IF_FALSE’
  107 |     if (!(condition)) {                                                 \
      |           ^~~~~~~~~
In file included from ../../src/util.h:27,
                 from ../../src/aliased_buffer.h:7,
                 from ../../src/env-inl.h:27,
                 from ../../src/js_native_api_v8.cc:5:
../../deps/v8/include/v8.h:5078:8: note: declared here
 5078 |   bool IsExternal() const;
      |        ^~~~~~~~~~

[addaleax]

I think @thangktran left comments in a few places stating that these can be removed once V8 is updated to V8 8.0. I think that’s accurate.

[addaleax]

Actually, some of the ArrayBuffers introduced there should maybe have the arraybuffer_untransferable_private_symbol set, like we do in node_buffer.cc, at least those whose memory isn’t actually being owned by the BackingStore, like the HTTP2 js_fields_ one.

[thangktran]

Actually, some of the ArrayBuffers introduced there should maybe have the arraybuffer_untransferable_private_symbol set, like we do in node_buffer.cc, at least those whose memory isn’t actually being owned by the BackingStore, like the HTTP2 js_fields_ one.

should i provide a PR for this?

[addaleax]

@thangktran I think that would be great, yes 👍

[targos]

I think this is resolved