Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(184)

Issue 29810586: Issue 6526 - Use the maybe version of Compile() and Run() (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
1 year, 5 months ago by hub
Modified:
1 year, 5 months ago
Reviewers:
sergei
Base URL:
https://hg.adblockplus.org/libadblockplus/
Visibility:
Public.

Description

Issue 6526 - Use the maybe version of Compile() and Run()

Patch Set 1 #

Total comments: 1

Patch Set 2 : Don't have a different error type. #

Patch Set 3 : Remove an extra ';' #

Total comments: 2

Patch Set 4 : Change the function to check MaybeLocal<> #

Total comments: 6

Patch Set 5 : Move CheckedToLocal() to Utils. #

Patch Set 6 : Actually remove the move code. #

Patch Set 7 : r-value CheckedToLocal #

Unified diffs Side-by-side diffs Delta from patch set Stats (+56 lines, -16 lines) Patch
M src/JsEngine.cpp View 1 2 3 4 5 6 2 chunks +8 lines, -14 lines 0 comments Download
M src/JsError.h View 1 2 3 1 chunk +3 lines, -1 line 0 comments Download
M src/JsError.cpp View 1 2 3 1 chunk +16 lines, -1 line 0 comments Download
M src/Utils.h View 1 2 3 4 5 6 1 chunk +23 lines, -0 lines 0 comments Download
M src/Utils.cpp View 1 2 3 4 1 chunk +6 lines, -0 lines 0 comments Download

Messages

Total messages: 13
hub
1 year, 5 months ago (2018-06-19 17:14:25 UTC) #1
hub
https://codereview.adblockplus.org/29810586/diff/29810587/src/JsError.h File src/JsError.h (right): https://codereview.adblockplus.org/29810586/diff/29810587/src/JsError.h#newcode38 src/JsError.h:38: class JsValueError : public std::runtime_error On second thought, maybe ...
1 year, 5 months ago (2018-06-19 17:17:04 UTC) #2
hub
updated patch
1 year, 5 months ago (2018-06-19 17:40:29 UTC) #3
sergei
https://codereview.adblockplus.org/29810586/diff/29810600/src/JsEngine.cpp File src/JsEngine.cpp (right): https://codereview.adblockplus.org/29810586/diff/29810600/src/JsEngine.cpp#newcode210 src/JsEngine.cpp:210: return JsValue(shared_from_this(), result.ToLocalChecked()); I think it's very important to ...
1 year, 5 months ago (2018-06-20 15:09:56 UTC) #4
hub
updated patch https://codereview.adblockplus.org/29810586/diff/29810600/src/JsEngine.cpp File src/JsEngine.cpp (right): https://codereview.adblockplus.org/29810586/diff/29810600/src/JsEngine.cpp#newcode210 src/JsEngine.cpp:210: return JsValue(shared_from_this(), result.ToLocalChecked()); On 2018/06/20 15:09:55, sergei ...
1 year, 5 months ago (2018-06-20 16:09:21 UTC) #5
sergei
https://codereview.adblockplus.org/29810586/diff/29811601/src/JsEngine.cpp File src/JsEngine.cpp (right): https://codereview.adblockplus.org/29810586/diff/29811601/src/JsEngine.cpp#newcode214 src/JsEngine.cpp:214: v8::MaybeLocal<v8::Script> script = CompileScript(isolate, source, what do you think ...
1 year, 5 months ago (2018-06-20 16:33:00 UTC) #6
hub
https://codereview.adblockplus.org/29810586/diff/29811601/src/JsEngine.cpp File src/JsEngine.cpp (right): https://codereview.adblockplus.org/29810586/diff/29811601/src/JsEngine.cpp#newcode214 src/JsEngine.cpp:214: v8::MaybeLocal<v8::Script> script = CompileScript(isolate, source, On 2018/06/20 16:32:59, sergei ...
1 year, 5 months ago (2018-06-20 16:41:48 UTC) #7
hub
Moved CheckedToLocal() to Utils because we'll need then elsewhere too. Better do it now.
1 year, 5 months ago (2018-06-20 18:08:09 UTC) #8
sergei
https://codereview.adblockplus.org/29810586/diff/29811601/src/JsEngine.cpp File src/JsEngine.cpp (right): https://codereview.adblockplus.org/29810586/diff/29811601/src/JsEngine.cpp#newcode214 src/JsEngine.cpp:214: v8::MaybeLocal<v8::Script> script = CompileScript(isolate, source, On 2018/06/20 16:41:47, hub ...
1 year, 5 months ago (2018-06-20 19:01:50 UTC) #9
hub
https://codereview.adblockplus.org/29810586/diff/29811601/src/JsEngine.cpp File src/JsEngine.cpp (right): https://codereview.adblockplus.org/29810586/diff/29811601/src/JsEngine.cpp#newcode214 src/JsEngine.cpp:214: v8::MaybeLocal<v8::Script> script = CompileScript(isolate, source, On 2018/06/20 19:01:50, sergei ...
1 year, 5 months ago (2018-06-21 01:13:43 UTC) #10
sergei
https://codereview.adblockplus.org/29810586/diff/29811601/src/JsEngine.cpp File src/JsEngine.cpp (right): https://codereview.adblockplus.org/29810586/diff/29811601/src/JsEngine.cpp#newcode214 src/JsEngine.cpp:214: v8::MaybeLocal<v8::Script> script = CompileScript(isolate, source, On 2018/06/21 01:13:43, hub ...
1 year, 5 months ago (2018-06-21 08:45:46 UTC) #11
hub
update patch https://codereview.adblockplus.org/29810586/diff/29811601/src/JsEngine.cpp File src/JsEngine.cpp (right): https://codereview.adblockplus.org/29810586/diff/29811601/src/JsEngine.cpp#newcode214 src/JsEngine.cpp:214: v8::MaybeLocal<v8::Script> script = CompileScript(isolate, source, On 2018/06/21 ...
1 year, 5 months ago (2018-06-21 13:02:17 UTC) #12
sergei
1 year, 5 months ago (2018-06-21 13:33:55 UTC) #13
LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 87257f5