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

Delta Between Two Patch Sets: src/JsValue.cpp

Issue 29813591: Issue 6526 - Use Maybe<> version of soon to be deprecated API in v8 6.7 (Closed) Base URL: https://hg.adblockplus.org/libadblockplus/
Left Patch Set: Rebased Created Aug. 6, 2018, 7:30 p.m.
Right Patch Set: Throw on empty value (AsInt() and As Bool()) Created Aug. 7, 2018, 2:36 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« no previous file with change/comment | « src/JsEngine.cpp ('k') | src/Utils.h » ('j') | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 /* 1 /*
2 * This file is part of Adblock Plus <https://adblockplus.org/>, 2 * This file is part of Adblock Plus <https://adblockplus.org/>,
3 * Copyright (C) 2006-present eyeo GmbH 3 * Copyright (C) 2006-present eyeo GmbH
4 * 4 *
5 * Adblock Plus is free software: you can redistribute it and/or modify 5 * Adblock Plus is free software: you can redistribute it and/or modify
6 * it under the terms of the GNU General Public License version 3 as 6 * it under the terms of the GNU General Public License version 3 as
7 * published by the Free Software Foundation. 7 * published by the Free Software Foundation.
8 * 8 *
9 * Adblock Plus is distributed in the hope that it will be useful, 9 * Adblock Plus is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of 10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
(...skipping 120 matching lines...) Expand 10 before | Expand all | Expand 10 after
131 { 131 {
132 const JsContext context(*jsEngine); 132 const JsContext context(*jsEngine);
133 return Utils::StringBufferFromV8String(jsEngine->GetIsolate(), UnwrapValue()); 133 return Utils::StringBufferFromV8String(jsEngine->GetIsolate(), UnwrapValue());
134 } 134 }
135 135
136 int64_t AdblockPlus::JsValue::AsInt() const 136 int64_t AdblockPlus::JsValue::AsInt() const
137 { 137 {
138 const JsContext context(*jsEngine); 138 const JsContext context(*jsEngine);
139 auto currentContext = jsEngine->GetIsolate()->GetCurrentContext(); 139 auto currentContext = jsEngine->GetIsolate()->GetCurrentContext();
140 auto value = UnwrapValue()->IntegerValue(currentContext); 140 auto value = UnwrapValue()->IntegerValue(currentContext);
141 return value.IsJust() ? value.FromJust() : 0; 141 return CHECKED_TO_VALUE(std::move(value));
sergei 2018/08/07 13:14:52 What about using of CheckedToValue here? Returning
hub 2018/08/07 14:36:48 I wanted to avoid throwing an exception. But it se
sergei 2018/08/07 16:47:35 I find the usage of std::move good here too, for t
142 } 142 }
143 143
144 bool AdblockPlus::JsValue::AsBool() const 144 bool AdblockPlus::JsValue::AsBool() const
145 { 145 {
146 const JsContext context(*jsEngine); 146 const JsContext context(*jsEngine);
147 auto currentContext = jsEngine->GetIsolate()->GetCurrentContext(); 147 auto currentContext = jsEngine->GetIsolate()->GetCurrentContext();
148 auto value = UnwrapValue()->BooleanValue(currentContext); 148 auto value = UnwrapValue()->BooleanValue(currentContext);
149 return value.IsJust() ? value.FromJust() : false; 149 return CHECKED_TO_VALUE(std::move(value));
150 } 150 }
151 151
152 AdblockPlus::JsValueList AdblockPlus::JsValue::AsList() const 152 AdblockPlus::JsValueList AdblockPlus::JsValue::AsList() const
153 { 153 {
154 if (!IsArray()) 154 if (!IsArray())
155 throw std::runtime_error("Cannot convert a non-array to list"); 155 throw std::runtime_error("Cannot convert a non-array to list");
156 156
157 const JsContext context(*jsEngine); 157 const JsContext context(*jsEngine);
158 auto isolate = jsEngine->GetIsolate(); 158 auto isolate = jsEngine->GetIsolate();
159 auto currentContext = isolate->GetCurrentContext(); 159 auto currentContext = isolate->GetCurrentContext();
(...skipping 147 matching lines...) Expand 10 before | Expand all | Expand 10 after
307 auto isolate = jsEngine->GetIsolate(); 307 auto isolate = jsEngine->GetIsolate();
308 308
309 const v8::TryCatch tryCatch(isolate); 309 const v8::TryCatch tryCatch(isolate);
310 v8::Local<v8::Function> func = v8::Local<v8::Function>::Cast(UnwrapValue()); 310 v8::Local<v8::Function> func = v8::Local<v8::Function>::Cast(UnwrapValue());
311 auto result = CHECKED_TO_LOCAL_WITH_TRY_CATCH( 311 auto result = CHECKED_TO_LOCAL_WITH_TRY_CATCH(
312 isolate, func->Call(isolate->GetCurrentContext(), 312 isolate, func->Call(isolate->GetCurrentContext(),
313 thisObj, args.size(), args.size() ? &args[0] : nullptr), tryCatch); 313 thisObj, args.size(), args.size() ? &args[0] : nullptr), tryCatch);
314 314
315 return JsValue(jsEngine, result); 315 return JsValue(jsEngine, result);
316 } 316 }
LEFTRIGHT

Powered by Google App Engine
This is Rietveld