Opened 20 months ago

Last modified 15 months ago

#6526 new change

Stop using of deprecated V8 API. — at Version 3

Reported by: sergz Assignee:
Priority: P3 Milestone:
Module: Libadblockplus Keywords:
Cc: rjeschke, hfiguiere Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29734562/
https://codereview.adblockplus.org/29735555/
https://codereview.adblockplus.org/29809555/
https://codereview.adblockplus.org/29810586/
https://codereview.adblockplus.org/29812646/
https://codereview.adblockplus.org/29812649/
https://codereview.adblockplus.org/29813591/
https://gitlab.com/eyeo/adblockplus/libadblockplus/merge_requests/7
https://gitlab.com/eyeo/adblockplus/libadblockplus/merge_requests/9

Description (last modified by sergz)

Background

We are using pretty a lot of deprecated V8 API, and in order to simplify our life, avoid unnecessary delays in such issues like #6063, and be able to experiment with new techniques like WebAssembly, we should update our code base to use up-to-date API.

What to change

The list of API

  • [done] v8::TryCatch should accept v8::Isolate
  • [done] v8::Isolate::IdleNotification is renamed to v8::Isolate::IdleNotitifcationDeadline
  • look into v8::Script::Compile, related #6448
  • [review] use v8::Local instead of v8::Handle
  • [question is below] use v8 functions which return maybe values, e.g.
    • v8::Value::IntegerValue
    • v8::FunctionTemplate::GetFunction
    • v8::String::NewFromUtf8
    • v8::Value::BooleanValue
    • v8::Object::Get(uint32_t)
    • v8::Object::GetOwnPropertyNames
    • v8::Function::Call
    • v8::Message::GetLineNumber
    • etc
  • those maybe functions accept v8::Context, therefore we need to carefully check that we pass the correct context. Perhaps in the callbacks we should rather use v8::Isolate::GetCurrentContext instead of obtaining it from JsEngine. Additionally pay attention to JsContext which works with the context too.
  • to be continued ...

Here we need to decide regarding the behavior if there is no value in maybevalue. The options are basically either to use an artificial default value, crash or propagate such resembling monadic approach to our API.

Change History (3)

comment:1 Changed 20 months ago by sergz

  • Review URL(s) modified (diff)

comment:2 Changed 20 months ago by abpbot

A commit referencing this issue has landed:
Issue 6526 - update usage of V8 API

comment:3 Changed 20 months ago by sergz

  • Cc rjeschke hfiguiere added
  • Description modified (diff)
  • Review URL(s) modified (diff)

I would like to get your opinion especially regarding the dealing with the maybevalues in our code.

In order to get compiler warnings and errors one can add

  'defines': [
    'V8_DEPRECATION_WARNINGS',
    'V8_IMMINENT_DEPRECATION_WARNINGS'
  ],

to 'target_defaults' in common.gypi and compile our code with prebuilt V8.

Note: See TracTickets for help on using tickets.