Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 4949583905947648: Issue 1280 - Update v8, the second part (Closed)

Created:
Oct. 27, 2014, 10:01 p.m. by sergei
Modified:
Nov. 5, 2014, 8:20 p.m.
Visibility:
Public.

Description

Beside API changes there are some important notes: - Update subrepo to the recent gyp and v8. - generate solution and project for MSVS2013 - create Filter and Subscription from (JsValue&& value) instead of JsValuePtr. It makes the code stronger. It is what we really semantically need and what we are doing now. Before we did it implicitly, compiler generated copy ctr for V8ValueHolder implicitly transferred the owned object because std::auto_ptr was used (BTW, now there is v8::UniquePersistent which is the member of JsValue, V8ValueHolder is removed). If we need to have a shared instance of passed JsValue then I would say these classes should implement proxy pattern. - precise isolation initialization and de-initialization. IsolateManagerJsEngine and friends are a little bit ugly, but it's enough reliable. - make ctr of JsEngine public but with private arg to use it in std::make_shared - replace V8ValueHolder by v8::UniquePersistent in V8ValueHolder. It should be better for the memory management, it also might fixes some memory leakages. To whom who is aware about the code, please try to find a case when we need the longer object life than the scope of JsValue. - shell/src/Main.cpp there is no automatic conversion to bool any more, call `bool ios::good() const`. - In test/JsEngine.cpp move test specific stuff into the test scope. Otherwise the destruction order was incorrect, `callbackParams` were destroyed after isolation, which caused a fatal exit. - Increase waiting time in test/UpdateCheck.cpp otherwise that test was very unstable in debug mode with the debug heap. Tests should not rely on such stuff, it should be controlled by synchronization structures. - Sorry, there are a lot of cosmetic changes. Prerequisites: - fix v8, Replace `#include "include/v8-platform.h"` by `#include "../v8-platform.h"` in include/libplatform/libplatform.h - update gyp and fix (or find workaround for) generating (--random-seed) issue Depends on http://codereview.adblockplus.org/6584950149087232/

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+531 lines, -503 lines) Patch
M .hgsubstate View 1 chunk +2 lines, -2 lines 0 comments Download
M createsolution.bat View 1 chunk +2 lines, -2 lines 0 comments Download
M include/AdblockPlus/FilterEngine.h View 2 chunks +2 lines, -2 lines 0 comments Download
M include/AdblockPlus/JsEngine.h View 7 chunks +50 lines, -12 lines 0 comments Download
M include/AdblockPlus/JsValue.h View 3 chunks +4 lines, -11 lines 0 comments Download
R include/AdblockPlus/V8ValueHolder.h View 1 chunk +0 lines, -82 lines 0 comments Download
M libadblockplus.gyp View 2 chunks +9 lines, -3 lines 0 comments Download
M shell/src/Main.cpp View 1 chunk +1 line, -1 line 0 comments Download
M src/ConsoleJsObject.cpp View 3 chunks +23 lines, -23 lines 0 comments Download
M src/FileSystemJsObject.cpp View 6 chunks +78 lines, -40 lines 0 comments Download
M src/FilterEngine.cpp View 9 chunks +14 lines, -14 lines 0 comments Download
M src/GlobalJsObject.h View 1 chunk +1 line, -1 line 0 comments Download
M src/GlobalJsObject.cpp View 2 chunks +23 lines, -27 lines 0 comments Download
M src/JsContext.h View 1 chunk +5 lines, -4 lines 0 comments Download
M src/JsContext.cpp View 1 chunk +5 lines, -3 lines 0 comments Download
M src/JsEngine.cpp View 1 chunk +240 lines, -202 lines 0 comments Download
M src/JsError.h View 1 chunk +0 lines, -18 lines 0 comments Download
M src/JsError.cpp View 1 chunk +18 lines, -0 lines 0 comments Download
M src/JsValue.cpp View 9 chunks +33 lines, -33 lines 0 comments Download
M src/WebRequestJsObject.cpp View 3 chunks +9 lines, -9 lines 0 comments Download
M test/ConsoleJsObject.cpp View 1 chunk +3 lines, -5 lines 0 comments Download
M test/JsEngine.cpp View 2 chunks +8 lines, -8 lines 0 comments Download
M test/UpdateCheck.cpp View 1 chunk +1 line, -1 line 0 comments Download

Powered by Google App Engine
This is Rietveld