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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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

0 comments on commit 9006b4d

Please sign in to comment.