Opened on 03/27/2017 at 09:40:06 AM

Closed on 03/27/2017 at 02:20:18 PM

#5038 closed defect (fixed)

Using of a wrong JsEngine instance in PrefsTest

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

https://codereview.adblockplus.org/29395627/

Description

In PrefsTest we do the following:

  • create JsEngine, let's call it jsEngine1
  • create JsValue for jsEngine1
  • create a new JsEngine, let's call it jsEngine2
  • use JsValue created for jsEngine1 in jsEngine2.

The sharing of objects between v8 contexts is a bit vaguely documented and it seems it should work. However, despite currently we share the same v8 isolate between almost all instances of JsEngine in tests, it's a temporary hack which will be removed. So the tests should be fixed the way they use the same instance of JsEngine, thus the same isolate, and don't use objects from one isolate in another one.

Attachments (0)

Change History (3)

comment:1 Changed on 03/27/2017 at 09:51:47 AM by sergz

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:2 Changed on 03/27/2017 at 02:19:15 PM by abpbot

A commit referencing this issue has landed:
Issue 5038 - fix usage of JsEngine in PrefsTests

comment:3 Changed on 03/27/2017 at 02:20:18 PM by sergz

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

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from sergz.
 
Note: See TracTickets for help on using tickets.