diff --git a/Source/Noesis.Javascript/JavascriptContext.cpp b/Source/Noesis.Javascript/JavascriptContext.cpp index 2372ebb..06ff19d 100644 --- a/Source/Noesis.Javascript/JavascriptContext.cpp +++ b/Source/Noesis.Javascript/JavascriptContext.cpp @@ -171,8 +171,9 @@ JavascriptContext::JavascriptContext() isolate->SetFatalErrorHandler(FatalErrorCallback); - mExternals = gcnew System::Collections::Generic::Dictionary(); mTypeToConstructorMapping = gcnew System::Collections::Generic::Dictionary(); + mTypeToMethods = gcnew System::Collections::Generic::Dictionary^>(); + mFunctions = gcnew System::Collections::Generic::List(); HandleScope scope(isolate); mContext = new Persistent(isolate, Context::New(isolate)); @@ -186,8 +187,6 @@ JavascriptContext::~JavascriptContext() { v8::Locker v8ThreadLock(isolate); v8::Isolate::Scope isolate_scope(isolate); - for each (WrappedJavascriptExternal wrapped in mExternals->Values) - delete wrapped.Pointer; for each (System::WeakReference^ f in mFunctions) { JavascriptFunction ^function = safe_cast(f->Target); if (function != nullptr) @@ -197,8 +196,8 @@ JavascriptContext::~JavascriptContext() delete (void *)p; } delete mContext; - delete mExternals; delete mTypeToConstructorMapping; + delete mTypeToMethods; delete mFunctions; } if (isolate != NULL) @@ -298,6 +297,7 @@ void JavascriptContext::SetConstructor(System::String^ name, System::Type^ assoc functionTemplate->SetClassName(className); JavascriptInterop::InitObjectWrapperTemplate(functionTemplate->InstanceTemplate()); mTypeToConstructorMapping[associatedType] = System::IntPtr(new Persistent(isolate, functionTemplate)); + mTypeToMethods[associatedType] = gcnew System::Collections::Generic::Dictionary(); Local::New(isolate, *mContext)->Global()->Set(context, className, functionTemplate->GetFunction(context).ToLocalChecked()); } @@ -505,18 +505,7 @@ JavascriptContext::Collect() JavascriptExternal* JavascriptContext::WrapObject(System::Object^ iObject) { - WrappedJavascriptExternal external_wrapped; - if (mExternals->TryGetValue(iObject, external_wrapped)) - { - // We've wrapped this guy before. - return external_wrapped.Pointer; - } - else - { - JavascriptExternal* external = new JavascriptExternal(iObject); - mExternals[iObject] = WrappedJavascriptExternal(external); - return external; - } + return new JavascriptExternal(iObject, false); } //////////////////////////////////////////////////////////////////////////////////////////////////// @@ -529,6 +518,7 @@ JavascriptContext::GetObjectWrapperConstructorTemplate(System::Type ^type) Local constructor = FunctionTemplate::New(GetCurrentIsolate()); JavascriptInterop::InitObjectWrapperTemplate(constructor->InstanceTemplate()); mTypeToConstructorMapping[type] = System::IntPtr(new Persistent(isolate, constructor)); + mTypeToMethods[type] = gcnew System::Collections::Generic::Dictionary(); return constructor; } Persistent *constructor = (Persistent *)(void *)ptrToConstructor; @@ -537,6 +527,14 @@ JavascriptContext::GetObjectWrapperConstructorTemplate(System::Type ^type) //////////////////////////////////////////////////////////////////////////////////////////////////// +System::Collections::Generic::Dictionary^ +JavascriptContext::MethodsForType(System::Type^ type) +{ + return mTypeToMethods[type]; +} + +//////////////////////////////////////////////////////////////////////////////////////////////////// + void JavascriptContext::RegisterFunction(System::Object^ f) { diff --git a/Source/Noesis.Javascript/JavascriptContext.h b/Source/Noesis.Javascript/JavascriptContext.h index 553a6c8..8e81d0b 100644 --- a/Source/Noesis.Javascript/JavascriptContext.h +++ b/Source/Noesis.Javascript/JavascriptContext.h @@ -196,7 +196,9 @@ public ref class JavascriptContext: public System::IDisposable Local GetObjectWrapperConstructorTemplate(System::Type ^type); - void RegisterFunction(System::Object^ f); + System::Collections::Generic::Dictionary^ MethodsForType(System::Type^ type); + + void RegisterFunction(System::Object^ f); static void FatalErrorCallbackMember(const char* location, const char* message); @@ -212,18 +214,16 @@ public ref class JavascriptContext: public System::IDisposable // v8 context required to be active for all v8 operations. Persistent* mContext; - // Stores every JavascriptExternal we create. This saves time if the same - // objects are recreated frequently, and stops us building up a huge - // collection of JavascriptExternal objects that won't be freed until - // the context is destroyed. - System::Collections::Generic::Dictionary ^mExternals; - // Maps types to their constructor function templates // The mapping will either be defined by the user calling `SetConstructor` or autogenerated if no // mapping was provided. // The `IntPtr` points to a `Persistent`. System::Collections::Generic::Dictionary ^mTypeToConstructorMapping; + // For each distinct CLR type we've wrapped, this accumulates functions + // constructed to access its methods/properties. + System::Collections::Generic::Dictionary^>^ mTypeToMethods; + // Stores every JavascriptFunction we create. Ensures we dispose of them // all. System::Collections::Generic::List ^mFunctions; diff --git a/Source/Noesis.Javascript/JavascriptExternal.cpp b/Source/Noesis.Javascript/JavascriptExternal.cpp index 378ffdb..ddbbaa4 100644 --- a/Source/Noesis.Javascript/JavascriptExternal.cpp +++ b/Source/Noesis.Javascript/JavascriptExternal.cpp @@ -48,11 +48,13 @@ using namespace std; //////////////////////////////////////////////////////////////////////////////////////////////////// -JavascriptExternal::JavascriptExternal(System::Object^ iObject) +JavascriptExternal::JavascriptExternal(System::Object^ iObject, bool needs_methods) { mObjectHandle = System::Runtime::InteropServices::GCHandle::Alloc(iObject); mOptions = SetParameterOptions::None; - mMethods = gcnew System::Collections::Generic::Dictionary(); + System::Type^ type = iObject->GetType(); + if (needs_methods) + mMethods = JavascriptContext::GetCurrent()->MethodsForType(type); } //////////////////////////////////////////////////////////////////////////////////////////////////// @@ -76,16 +78,16 @@ Local JavascriptExternal::GetMethod(wstring iName) { v8::Isolate *isolate = JavascriptContext::GetCurrentIsolate(); - System::Collections::Generic::Dictionary ^methods = mMethods; - System::String^ memberName = gcnew System::String(iName.c_str()); + System::Collections::Generic::Dictionary^ methods = mMethods; + System::String^ memberName = gcnew System::String(iName.c_str()); WrappedMethod method; if (methods->TryGetValue(memberName, method)) return Local::New(isolate, *(method.Pointer)); else { - System::Object^ self = mObjectHandle.Target; - System::Type^ type = self->GetType(); - System::String^ memberName = gcnew System::String(iName.c_str()); + System::Object^ self = mObjectHandle.Target; + System::Type^ type = self->GetType(); + System::String^ memberName = gcnew System::String(iName.c_str()); cli::array^ objectInfo = gcnew cli::array(2); objectInfo->SetValue(self,0); objectInfo->SetValue(memberName,1); @@ -94,14 +96,14 @@ JavascriptExternal::GetMethod(wstring iName) cli::array^ members = type->GetMember(memberName); if (members->Length > 0 && members[0]->MemberType == MemberTypes::Method) { - JavascriptContext^ context = JavascriptContext::GetCurrent(); - Local external = External::New(isolate, context->WrapObject(objectInfo)); + JavascriptContext^ context = JavascriptContext::GetCurrent(); + Local external = External::New(isolate, context->WrapObject(objectInfo)); Local functionTemplate = FunctionTemplate::New(isolate, JavascriptInterop::Invoker, external); Local function = functionTemplate->GetFunction(isolate->GetCurrentContext()).ToLocalChecked(); Persistent *function_ptr = new Persistent(isolate, function); WrappedMethod wrapped(function_ptr); - methods[memberName] = wrapped; + methods[memberName] = wrapped; return function; } diff --git a/Source/Noesis.Javascript/JavascriptExternal.h b/Source/Noesis.Javascript/JavascriptExternal.h index 6eaf995..5beaaca 100644 --- a/Source/Noesis.Javascript/JavascriptExternal.h +++ b/Source/Noesis.Javascript/JavascriptExternal.h @@ -57,7 +57,7 @@ class JavascriptExternal //////////////////////////////////////////////////////////// public: - JavascriptExternal(System::Object^ iObject); + JavascriptExternal(System::Object^ iObject, bool needs_methods); ~JavascriptExternal(); @@ -98,8 +98,8 @@ class JavascriptExternal SetParameterOptions mOptions; - // Owned by JavascriptContext. - gcroot ^> mMethods; + // Owned by JavascriptContext. Only set when the object is with `JavascriptInterop::Getter()` + gcroot^> mMethods; std::unique_ptr> mIterator; static void IteratorCallback(const v8::FunctionCallbackInfo& iArgs); diff --git a/Source/Noesis.Javascript/JavascriptInterop.cpp b/Source/Noesis.Javascript/JavascriptInterop.cpp index 89af423..1b354be 100644 --- a/Source/Noesis.Javascript/JavascriptInterop.cpp +++ b/Source/Noesis.Javascript/JavascriptInterop.cpp @@ -259,7 +259,14 @@ JavascriptInterop::ConvertToV8(System::Object^ iObject) //////////////////////////////////////////////////////////////////////////////////////////////////// -// TODO: should return Local +// a sort-of destructor in v8 land +void JavascriptExternalMakeWeak(const v8::WeakCallbackInfo& data) { + v8::Isolate* isolate = JavascriptContext::GetCurrentIsolate(); + JavascriptExternal *external = data.GetParameter(); + delete external; +} + +// TO DO: should return Local Local JavascriptInterop::WrapObject(System::Object^ iObject) { @@ -267,11 +274,17 @@ JavascriptInterop::WrapObject(System::Object^ iObject) if (context != nullptr) { - Local templ = context->GetObjectWrapperConstructorTemplate(iObject->GetType()); + System::Type ^object_type = iObject->GetType(); + Local templ = context->GetObjectWrapperConstructorTemplate(object_type); v8::Isolate *isolate = JavascriptContext::GetCurrentIsolate(); Local instanceTemplate = templ->InstanceTemplate(); Local object = instanceTemplate->NewInstance(isolate->GetCurrentContext()).ToLocalChecked(); - object->SetInternalField(0, External::New(isolate, context->WrapObject(iObject))); + JavascriptExternal* external = new JavascriptExternal(iObject, true); + object->SetInternalField(0, External::New(isolate, external)); + + // So we're notified when this object is no longer needed. + v8::Persistent> pobj(isolate, object); + pobj.SetWeak(external, JavascriptExternalMakeWeak, v8::WeakCallbackType::kParameter); return object; } @@ -281,7 +294,7 @@ JavascriptInterop::WrapObject(System::Object^ iObject) //////////////////////////////////////////////////////////////////////////////////////////////////// -// TODO: should use Local iExternal +// TO DO: should use Local iExternal System::Object^ JavascriptInterop::UnwrapObject(Local iValue) {