Skip to content

Commit

Permalink
feat(android): Update V8 to 7.8.279.23 (#11294)
Browse files Browse the repository at this point in the history
* fix(android): call NewDefaultPlatform(), not CreateDefaultPlatform()

CreateDefaultPlatform() was deprecated and removed by v8

* feat(android): point to v8 7.4 build

* fix(android): adapt to new GetOwnPropertyNames() GetPropertyNames() APIs

* fix(android): update v8 checksum

* build: initial cpmpiling version against V8 7.8

* build(android): avoid unecessary header includes

* build(android): update v8 build and checksum for 7.8

* perf(android): move attempts to load core modules after checking prefix

don't attempt to load core module if request begins with '.' or '/'

* build(android): add debug logging, update v8 api usage

* build(android): add debug logging, update v8 api usage

* fix(android): update to newer V8 apis

* fix(android): update v8 version to 7.8.279.23

* chore(android): implement remote snapshot generation

* fix(android): update snapshot processing status code

* fix(android): mark request-promise-native as devDependency
  • Loading branch information
sgtcoolguy authored and lokeshchdhry committed Dec 11, 2019
1 parent ef53675 commit 9006b4d
Show file tree
Hide file tree
Showing 29 changed files with 228 additions and 221 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,10 @@ fails, a JS exception is returned.
<#local coerced = true>
<#local type = "MaybeLocal<Number>">
<#local valueExpr = expr + "->ToNumber(context)">
<#elseif info.jsType == "Boolean">
<#local coerced = false>
<#local type = "Local<Boolean>">
<#local valueExpr = expr + "->ToBoolean(isolate)">
<#elseif info?keys?seq_contains("jsHandleCast")> // Array
<#local coerced = false>
<#local type = "Local<" + info.jsType + ">">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,8 @@

#include "${packageName}.${proxyClassName}.h"

#include "AndroidUtil.h"
#include "JNIUtil.h"
#include "JSException.h"
#include "TypeConverter.h"
#include "V8Util.h"

<#if isModule>

Expand Down Expand Up @@ -239,8 +236,7 @@ Local<FunctionTemplate> ${className}::getProxyTemplate(v8::Isolate* isolate)
</@Proxy.listPropertyAccessors>

<#if interceptor??>
instanceTemplate->SetNamedPropertyHandler(${className}::interceptor);
// instanceTemplate->SetHandler(NamedPropertyHandlerConfiguration(${className}::interceptor, 0, 0, 0, 0, Local<Value>(), PropertyHandlerFlags::kOnlyInterceptStrings));
instanceTemplate->SetHandler(NamedPropertyHandlerConfiguration(reinterpret_cast<GenericNamedPropertyGetterCallback>(${className}::interceptor), nullptr, nullptr, nullptr, nullptr, Local<Value>(), PropertyHandlerFlags::kOnlyInterceptStrings));
</#if>
return scope.Escape(t);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
/** This is generated, do not edit by hand. **/
<#import "ProxyBinding.fm" as Proxy>

#include <jni.h>

#include "Proxy.h"

<@Proxy.openNamespace/>
Expand Down
4 changes: 2 additions & 2 deletions android/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
],
"architectures": ["arm64-v8a", "armeabi-v7a", "x86"],
"v8": {
"version": "7.3.492.26",
"version": "7.8.279.23",
"mode": "release",
"checksum": "7798aae19b2efff53b1f18d53345291ab16a574e0524350eed3db6d45757bc476a158303b8df6c5f359439f9184818d61eba6bd820cec404e268ab0796cc7c5a"
"checksum": "b7e3768e95463ea77842c54f6d46b87e9f79761ff2e351d770af23943e11e26bb9066eaa9966f621085ff79cba4a972e339038afefbf83d462917d708a2b0c43"
},
"minSDKVersion": "19",
"compileSDKVersion": "29",
Expand Down
24 changes: 13 additions & 11 deletions android/runtime/common/src/js/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -238,19 +238,10 @@ Module.prototype.loadExternalModule = function (id, externalBinding, context) {
* @return {Object} The loaded module
*/
Module.prototype.require = function (request, context) {
var start, // hack up the start of the string to check relative/absolute/"naked" module id
loaded; // variable to hold the possibly loaded module...

// 1. If X is a core module,
loaded = this.loadCoreModule(request, context);
if (loaded) {
// a. return the core module
// b. STOP
return loaded;
}
const start = request.substring(0, 2); // hack up the start of the string to check relative/absolute/"naked" module id
let loaded; // variable to hold the possibly loaded module...

// 2. If X begins with './' or '/' or '../'
start = request.substring(0, 2);
if (start === './' || start === '..') {
loaded = this.loadAsFileOrDirectory(path.normalize(this.path + '/' + request), context);
if (loaded) {
Expand All @@ -263,6 +254,17 @@ Module.prototype.require = function (request, context) {
return loaded.exports;
}
} else {
// Despite being step 1 in Node.JS psuedo-code, we moved it down here because we don't allow native modules
// to start with './', '..' or '/' - so this avoids a lot of misses on requires starting that way

// 1. If X is a core module,
loaded = this.loadCoreModule(request, context);
if (loaded) {
// a. return the core module
// b. STOP
return loaded;
}

// Look for CommonJS module
if (request.indexOf('/') === -1) {
// For CommonJS we need to look for module.id/module.id.js first...
Expand Down
2 changes: 0 additions & 2 deletions android/runtime/v8/src/native/EventEmitter.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
#ifndef EVENT_EMITTER_H
#define EVENT_EMITTER_H

#include <v8.h>

#include "NativeObject.h"

namespace titanium {
Expand Down
2 changes: 1 addition & 1 deletion android/runtime/v8/src/native/JSDebugger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ void JSDebugger::init(JNIEnv *env, jobject jsDebugger, v8::Local<v8::Context> co
assert(waitForMessage__ != nullptr);

if (debugger__ != nullptr) {
client__ = new InspectorClient(context, V8Runtime::platform);
client__ = new InspectorClient(context, V8Runtime::platform.get());
}
}

Expand Down
3 changes: 1 addition & 2 deletions android/runtime/v8/src/native/JSException.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,12 @@
#ifndef JSEXCEPTION_H
#define JSEXCEPTION_H

#include <jni.h>
#include <v8.h>

#include "JNIUtil.h"

#define THROW(isolate, msg) \
isolate->ThrowException(v8::String::NewFromUtf8(isolate, msg))
isolate->ThrowException(v8::String::NewFromUtf8(isolate, msg, v8::NewStringType::kNormal).ToLocalChecked())

namespace titanium {

Expand Down
2 changes: 0 additions & 2 deletions android/runtime/v8/src/native/JavaObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,15 +122,13 @@ void JavaObject::MakeJSWeak()
// to a weak reference in the JVM. (where we can track when that gets GC'd by the JVM to call back and kill this)
if (!isDetached() && !persistent().IsEmpty()) {
persistent().SetWeak(this, DetachCallback, v8::WeakCallbackType::kFinalizer); // MUST BE kFinalizer or our object cannot be resurrected!
persistent().MarkIndependent();
}
}

void JavaObject::detach()
{
// WAIT A SECOND V8!!! DON'T KILL MY OBJECT YET! THE JVM MAY STILL WANT IT!
persistent().ClearWeak(); // Make JS Strong Again!
persistent().MarkActive();

// if the JVM side is a weak reference or we have no object wrapped, don't do anything else
if (isDetached()) {
Expand Down
1 change: 0 additions & 1 deletion android/runtime/v8/src/native/JavaObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
#ifndef TI_KROLL_NATIVE_OBJECT_H
#define TI_KROLL_NATIVE_OBJECT_H

#include <v8.h>
#include <jni.h>

#include "EventEmitter.h"
Expand Down
25 changes: 16 additions & 9 deletions android/runtime/v8/src/native/KrollBindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,9 @@
* Please see the LICENSE included with this distribution for details.
*/
#include <dlfcn.h>
#include <map>
#include <cstring>
#include <vector>
#include <v8.h>
#include <jni.h>

#include "AndroidUtil.h"
#include "EventEmitter.h"
#include "JNIUtil.h"
#include "JSException.h"
#include "Proxy.h"
#include "ProxyFactory.h"
Expand Down Expand Up @@ -58,7 +52,8 @@ void KrollBindings::initNatives(Local<Object> exports, Local<Context> context)
HandleScope scope(isolate);
for (int i = 0; natives[i].name; ++i) {
if (natives[i].source == kroll_native) continue;
Local<String> name = String::NewFromUtf8(isolate, natives[i].name);
// FIXME: I don't think the name/source strings should ever be "empty" here, but should we do better error handling for it? If it ever is, it'll crash like this...
Local<String> name = String::NewFromUtf8(isolate, natives[i].name, v8::NewStringType::kNormal).ToLocalChecked();
Local<String> source = IMMUTABLE_STRING_LITERAL_FROM_ARRAY(isolate, natives[i].source, natives[i].source_length);
exports->Set(context, name, source);
}
Expand Down Expand Up @@ -146,6 +141,11 @@ Local<Object> KrollBindings::instantiateBinding(Isolate* isolate, bindings::Bind
return exports;
}

#ifdef TI_DEBUG
v8::String::Utf8Value bindingValue(isolate, key);
LOGD(TAG, "No binding found/supplied for %s, returning empty object", *bindingValue);
#endif

return Local<Object>();
}

Expand All @@ -157,6 +157,9 @@ bindings::BindEntry* KrollBindings::getExternalBinding(const char *name, unsigne

void KrollBindings::addExternalBinding(const char *name, struct bindings::BindEntry *binding)
{
#ifdef TI_DEBUG
LOGD(TAG, "Registered external (native module) binding for name %s", name);
#endif
externalBindings[std::string(name)] = binding;
}

Expand Down Expand Up @@ -203,7 +206,7 @@ Local<Object> KrollBindings::getBinding(v8::Isolate* isolate, Local<String> bind
}

// try native modules by lookup function
// This uses an array of functions we call to aks for a given binding
// This uses an array of functions we call to ask for a given binding
// Not sure why we have this *and* the external bindings in a map from name -> binding
for (int i = 0; i < KrollBindings::externalLookups.size(); i++) {
titanium::LookupFunction lookupFunction = KrollBindings::externalLookups[i];
Expand Down Expand Up @@ -261,7 +264,11 @@ void KrollBindings::dispose(v8::Isolate* isolate)
uint32_t length = propertyNames->Length();

for (uint32_t i = 0; i < length; i++) {
v8::String::Utf8Value binding(isolate, propertyNames->Get(context, i).ToLocalChecked()); // FIXME Handle when empty!
MaybeLocal<Value> propertyName = propertyNames->Get(context, i);
if (propertyName.IsEmpty()) {
continue;
}
v8::String::Utf8Value binding(isolate, propertyName.ToLocalChecked());
int bindingLength = binding.length();

struct titanium::bindings::BindEntry *generated = bindings::generated::lookupGeneratedInit(*binding, bindingLength);
Expand Down
1 change: 0 additions & 1 deletion android/runtime/v8/src/native/NativeObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ class NativeObject

inline void MakeWeak(void) {
persistent().SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter);
persistent().MarkIndependent();
}

/* Ref() marks the object as being attached to an event loop.
Expand Down
6 changes: 4 additions & 2 deletions android/runtime/v8/src/native/Proxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,8 @@ static void onPropertyChangedForProxy(Isolate* isolate, Local<String> property,
void Proxy::onPropertyChanged(Local<Name> property, Local<Value> value, const v8::PropertyCallbackInfo<void>& info)
{
Isolate* isolate = info.GetIsolate();
onPropertyChangedForProxy(isolate, property->ToString(isolate), value, info.Holder());
Local<Context> context = isolate->GetCurrentContext();
onPropertyChangedForProxy(isolate, property->ToString(context).ToLocalChecked(), value, info.Holder());
}

// This variant is used when accessing a property through a getter method (i.e. setText('whatever'))
Expand All @@ -215,7 +216,8 @@ void Proxy::onPropertyChanged(const v8::FunctionCallbackInfo<v8::Value>& args)
LOGW(TAG, "Automatic setter methods for properties are deprecated in SDK 8.0.0 and will be removed in SDK 9.0.0. Please modify the property in standard JS style: obj.%s = value; or obj['%s'] = value;", *propertyKey, *propertyKey);

Local<Value> value = args[0];
onPropertyChangedForProxy(isolate, name->ToString(isolate), value, args.Holder());
Local<Context> context = isolate->GetCurrentContext();
onPropertyChangedForProxy(isolate, name->ToString(context).ToLocalChecked(), value, args.Holder());
}

void Proxy::getIndexedProperty(uint32_t index, const PropertyCallbackInfo<Value>& info)
Expand Down
9 changes: 4 additions & 5 deletions android/runtime/v8/src/native/Proxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@
#ifndef PROXY_H
#define PROXY_H

#include <jni.h>
#include <v8.h>

#include "JavaObject.h"
#include "V8Util.h"

namespace titanium {

Expand Down Expand Up @@ -124,12 +122,13 @@ class Proxy : public JavaObject
v8::Isolate* isolate = args.GetIsolate();
v8::HandleScope scope(isolate);
v8::Local<v8::Function> fn = args[0].As<v8::Function>();
v8::Local<v8::Context> context = isolate->GetCurrentContext();
v8::Local<v8::FunctionTemplate> newType = inheritProxyTemplate(
isolate,
ProxyClass::getProxyTemplate(isolate),
ProxyClass::javaClass,
fn->GetName()->ToString(isolate), fn);
args.GetReturnValue().Set(newType->GetFunction());
fn->GetName()->ToString(context).FromMaybe(STRING_NEW(isolate, "unknown")), fn);
args.GetReturnValue().Set(newType->GetFunction(context).FromMaybe(v8::Local<v8::Function>()));
}

/**
Expand Down
14 changes: 8 additions & 6 deletions android/runtime/v8/src/native/TypeConverter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ jstring TypeConverter::jsStringToJavaString(Isolate* isolate, Local<String> jsSt

jstring TypeConverter::jsStringToJavaString(JNIEnv *env, Local<String> jsString)
{
String::Value string(jsString);
String::Value string(V8Runtime::v8_isolate, jsString);
return env->NewString(reinterpret_cast<const jchar*>(*string), string.length());
}

Expand All @@ -136,7 +136,8 @@ jstring TypeConverter::jsValueToJavaString(Isolate* isolate, JNIEnv *env, Local<
return NULL;
}

return TypeConverter::jsStringToJavaString(isolate, env, jsValue->ToString(isolate));
Local<Context> context = isolate->GetCurrentContext();
return TypeConverter::jsStringToJavaString(isolate, env, jsValue->ToString(context).ToLocalChecked());
}

Local<Value> TypeConverter::javaStringToJsString(Isolate* isolate, jstring javaString)
Expand Down Expand Up @@ -223,7 +224,7 @@ Local<Date> TypeConverter::javaDateToJsDate(Isolate* isolate, JNIEnv *env, jobje

Local<Date> TypeConverter::javaLongToJsDate(Isolate* isolate, jlong javaLong)
{
return Date::New(isolate, (double) javaLong).As<Date>(); // perversely, the date constructor returns Local<value> so we need to cast it
return Date::New(isolate->GetCurrentContext(), (double) javaLong).ToLocalChecked().As<Date>(); // perversely, the date constructor returns Local<value> so we need to cast it
}

jobject TypeConverter::jsObjectToJavaFunction(Isolate* isolate, Local<Object> jsObject)
Expand All @@ -239,7 +240,7 @@ jobject TypeConverter::jsObjectToJavaFunction(Isolate* isolate, JNIEnv *env, Loc
{
Local<Function> func = jsObject.As<Function>();
Persistent<Function, CopyablePersistentTraits<Function>> jsFunction(isolate, func);
jsFunction.MarkIndependent();
// jsFunction.MarkIndependent(); // Method has been removed!

// Place the persistent into some global table with incrementing index, use the index as the "ptr" here
// Then when we re-construct, use the ptr value as index into the table to grab the persistent!
Expand Down Expand Up @@ -943,6 +944,7 @@ Local<Object> TypeConverter::javaHashMapToJsValue(Isolate* isolate, JNIEnv *env,

int hashMapKeysLength = env->GetArrayLength(hashMapKeys);
bool isStringHashMap = env->IsInstanceOf(hashMapKeys, JNIUtil::stringArrayClass);
Local<Context> context = isolate->GetCurrentContext();

for (int i = 0; i < hashMapKeysLength; i++) {
jobject javaPairKey = env->GetObjectArrayElement(hashMapKeys, i);
Expand All @@ -956,7 +958,7 @@ Local<Object> TypeConverter::javaHashMapToJsValue(Isolate* isolate, JNIEnv *env,
jobject javaPairValue = env->CallObjectMethod(javaObject, JNIUtil::hashMapGetMethod, javaPairKey);
env->DeleteLocalRef(javaPairKey);

jsObject->Set(jsPairKey, TypeConverter::javaObjectToJsValue(isolate, env, javaPairValue));
jsObject->Set(context, jsPairKey, TypeConverter::javaObjectToJsValue(isolate, env, javaPairValue));
env->DeleteLocalRef(javaPairValue);
}

Expand Down Expand Up @@ -1241,7 +1243,7 @@ Local<Object> TypeConverter::javaThrowableToJSError(v8::Isolate* isolate, JNIEnv
Local<Context> context = isolate->GetCurrentContext();

// Now explicitly assign our properly generated stacktrace
Local<String> javaStack = String::NewFromUtf8(isolate, stackStream.str().c_str());
Local<String> javaStack = String::NewFromUtf8(isolate, stackStream.str().c_str(), v8::NewStringType::kNormal).ToLocalChecked();
error->Set(context, STRING_NEW(isolate, "nativeStack"), javaStack);

// If we're using our custom error interface we can ask for a map of additional properties ot set on the JS Error
Expand Down
2 changes: 1 addition & 1 deletion android/runtime/v8/src/native/V8Object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ Java_org_appcelerator_kroll_runtime_v8_V8Object_nativeFireEvent
Local<Value> jsEvent = TypeConverter::javaStringToJsString(V8Runtime::v8_isolate, env, event);

#ifdef TI_DEBUG
v8::String::Utf8Value eventName(isolate, jsEvent);
v8::String::Utf8Value eventName(V8Runtime::v8_isolate, jsEvent);
LOGV(TAG, "firing event \"%s\"", *eventName);
#endif

Expand Down
12 changes: 6 additions & 6 deletions android/runtime/v8/src/native/V8Runtime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Persistent<Object> V8Runtime::moduleObject;
Persistent<Function> V8Runtime::runModuleFunction;

jobject V8Runtime::javaInstance;
Platform* V8Runtime::platform = nullptr;
std::unique_ptr<v8::Platform> V8Runtime::platform;
Isolate* V8Runtime::v8_isolate = nullptr;
bool V8Runtime::debuggerEnabled = false;
bool V8Runtime::DBG = false;
Expand Down Expand Up @@ -165,8 +165,8 @@ void V8Runtime::bootstrap(Local<Context> context)

// Set the __dirname and __filename for the app.js.
// For other files, it will be injected via the `NativeModule` JavaScript class
global->Set(NEW_SYMBOL(isolate, "__filename"), STRING_NEW(isolate, "/app.js"));
global->Set(NEW_SYMBOL(isolate, "__dirname"), STRING_NEW(isolate, "/"));
global->Set(context, NEW_SYMBOL(isolate, "__filename"), STRING_NEW(isolate, "/app.js"));
global->Set(context, NEW_SYMBOL(isolate, "__dirname"), STRING_NEW(isolate, "/"));

Local<Function> mainFunction = result.As<Function>();
Local<Value> args[] = { kroll };
Expand All @@ -188,7 +188,7 @@ static void logV8Exception(Local<Message> msg, Local<Value> data)
*String::Utf8Value(V8Runtime::v8_isolate, msg->GetScriptResourceName()),
msg->GetLineNumber(context).FromMaybe(-1),
*String::Utf8Value(V8Runtime::v8_isolate,
msg->GetSourceLine(context).FromMaybe(-1)));
msg->GetSourceLine(context).ToLocalChecked()));
}

} // namespace titanium
Expand All @@ -208,8 +208,8 @@ JNIEXPORT void JNICALL Java_org_appcelerator_kroll_runtime_v8_V8Runtime_nativeIn
// Initialize V8.
// TODO Enable this when we use snapshots?
//V8::InitializeExternalStartupData(argv[0]);
V8Runtime::platform = platform::CreateDefaultPlatform();
V8::InitializePlatform(V8Runtime::platform);
V8Runtime::platform = platform::NewDefaultPlatform();
V8::InitializePlatform(V8Runtime::platform.get());
V8::Initialize();
V8Runtime::initialized = true;
}
Expand Down
2 changes: 1 addition & 1 deletion android/runtime/v8/src/native/V8Runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class V8Runtime
static Persistent<Array> moduleContexts;

static Isolate* v8_isolate;
static Platform* platform;
static std::unique_ptr<v8::Platform> platform;

static jobject javaInstance;

Expand Down
Loading

0 comments on commit 9006b4d

Please sign in to comment.