Opened 3 years ago

Closed 3 years ago

Last modified 2 years ago

#5570 closed change (fixed)

Make V8 isolate injectable into JsEngine.

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

https://codereview.adblockplus.org/29527588/

Description

Background

In order to simplify usage of libadblockplus with an arbitrary V8 we should allow to inject v8::Isolate.

There is need to use V8 which is already used by an application where libadblockplus is integrated to, e.g. in chromium. It can save a couple of MBs because the second V8 won't be linked statically, so the size of an image of a binary loaded into the memory will be less (it's expected the difference to be not bigger than 5 MB, though). In addition sharing of already existing v8::Isolate can probably save a little bit more memory (it's expected the difference to be not bigger than a couple of MBs), though it can be challenging to avoid bugs. It turned out that initialization of V8 and instantiation of v8::Isolate can be pretty complicated and depends on particular project, so we should delegate it to embedders of libadblockplus.

What to change

Create an interface AdblockPlus::IV8IsolateProvider which has method v8::Isolate* Get() and provide a way to pass it to JsEngine at the construction stage. If it's value is nullptr then use the default implementation which should be currently existing ScopedV8Isolate.

Change History (6)

comment:1 Changed 3 years ago by sergz

  • Owner set to sergz

comment:2 Changed 3 years ago by sergz

  • Review URL(s) modified (diff)

comment:3 Changed 3 years ago by sergz

  • Status changed from new to reviewing

comment:4 Changed 3 years ago by abpbot

A commit referencing this issue has landed:
Issue 5570 - Make V8 isolate injectable into JsEngine

comment:5 Changed 3 years ago by sergz

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:6 Changed 2 years ago by sergz

  • Blocking 5643 added
Note: See TracTickets for help on using tickets.