Diff
Modified: trunk/Source/_javascript_Core/API/JSContext.mm (244322 => 244323)
--- trunk/Source/_javascript_Core/API/JSContext.mm 2019-04-16 02:03:41 UTC (rev 244322)
+++ trunk/Source/_javascript_Core/API/JSContext.mm 2019-04-16 02:12:26 UTC (rev 244323)
@@ -369,11 +369,6 @@
return [[self wrapperMap] objcWrapperForJSValueRef:value inContext:self];
}
-- (void)removeWrapper:(JSValue *)value
-{
- return [[self wrapperMap] removeWrapper:value];
-}
-
+ (JSContext *)contextWithJSGlobalContextRef:(JSGlobalContextRef)globalContext
{
JSContext *context = (__bridge JSContext *)toJSGlobalObject(globalContext)->apiWrapper();
Modified: trunk/Source/_javascript_Core/API/JSContextInternal.h (244322 => 244323)
--- trunk/Source/_javascript_Core/API/JSContextInternal.h 2019-04-16 02:03:41 UTC (rev 244322)
+++ trunk/Source/_javascript_Core/API/JSContextInternal.h 2019-04-16 02:12:26 UTC (rev 244323)
@@ -54,7 +54,6 @@
- (JSWrapperMap *)wrapperMap;
- (JSValue *)wrapperForObjCObject:(id)object;
- (JSValue *)wrapperForJSObject:(JSValueRef)value;
-- (void)removeWrapper:(JSValue *)value;
@end
Modified: trunk/Source/_javascript_Core/API/JSValue.mm (244322 => 244323)
--- trunk/Source/_javascript_Core/API/JSValue.mm 2019-04-16 02:03:41 UTC (rev 244322)
+++ trunk/Source/_javascript_Core/API/JSValue.mm 2019-04-16 02:12:26 UTC (rev 244323)
@@ -71,7 +71,6 @@
- (void)dealloc
{
- [_context removeWrapper:self];
JSValueUnprotect([_context JSGlobalContextRef], m_value);
[_context release];
_context = nil;
@@ -1076,7 +1075,6 @@
if (!self)
return nil;
- ASSERT(context);
_context = [context retain];
m_value = value;
JSValueProtect([_context JSGlobalContextRef], m_value);
Modified: trunk/Source/_javascript_Core/API/JSWrapperMap.h (244322 => 244323)
--- trunk/Source/_javascript_Core/API/JSWrapperMap.h 2019-04-16 02:03:41 UTC (rev 244322)
+++ trunk/Source/_javascript_Core/API/JSWrapperMap.h 2019-04-16 02:12:26 UTC (rev 244323)
@@ -37,8 +37,6 @@
- (JSValue *)objcWrapperForJSValueRef:(JSValueRef)value inContext:(JSContext *)context;
-- (void)removeWrapper:(JSValue *)wrapper;
-
@end
id tryUnwrapObjcObject(JSGlobalContextRef, JSValueRef);
Modified: trunk/Source/_javascript_Core/API/JSWrapperMap.mm (244322 => 244323)
--- trunk/Source/_javascript_Core/API/JSWrapperMap.mm 2019-04-16 02:03:41 UTC (rev 244322)
+++ trunk/Source/_javascript_Core/API/JSWrapperMap.mm 2019-04-16 02:12:26 UTC (rev 244323)
@@ -581,77 +581,10 @@
@end
-struct WrapperKey {
- static constexpr uintptr_t hashTableDeletedValue() { return 1; }
-
- WrapperKey() = default;
-
- explicit WrapperKey(WTF::HashTableDeletedValueType)
- : m_wrapper(reinterpret_cast<JSValue *>(hashTableDeletedValue()))
- {
- }
-
- explicit WrapperKey(JSValue *wrapper)
- : m_wrapper(wrapper)
- {
- }
-
- bool isHashTableDeletedValue() const
- {
- return reinterpret_cast<uintptr_t>(m_wrapper) == hashTableDeletedValue();
- }
-
- __unsafe_unretained JSValue *m_wrapper { nil };
-
- struct Hash {
- static unsigned hash(const WrapperKey& key)
- {
- return DefaultHash<JSValueRef>::Hash::hash([key.m_wrapper JSValueRef]);
- }
-
- static bool equal(const WrapperKey& lhs, const WrapperKey& rhs)
- {
- return lhs.m_wrapper == rhs.m_wrapper;
- }
-
- static const bool safeToCompareToEmptyOrDeleted = false;
- };
-
- struct Traits : public SimpleClassHashTraits<WrapperKey> {
- static const bool hasIsEmptyValueFunction = true;
- static bool isEmptyValue(const WrapperKey& key)
- {
- return key.m_wrapper == nullptr;
- }
- };
-
- struct Translator {
- struct ValueAndContext {
- __unsafe_unretained JSContext *m_context;
- JSValueRef m_value;
- };
-
- static unsigned hash(const ValueAndContext& value)
- {
- return DefaultHash<JSValueRef>::Hash::hash(value.m_value);
- }
-
- static bool equal(const WrapperKey& lhs, const ValueAndContext& value)
- {
- return [lhs.m_wrapper JSValueRef] == value.m_value;
- }
-
- static void translate(WrapperKey& result, const ValueAndContext& value, unsigned)
- {
- result = WrapperKey([[[JSValue alloc] initWithValue:value.m_value inContext:value.m_context] autorelease]);
- }
- };
-};
-
@implementation JSWrapperMap {
NSMutableDictionary *m_classMap;
std::unique_ptr<JSC::WeakGCMap<__unsafe_unretained id, JSC::JSObject>> m_cachedJSWrappers;
- HashSet<WrapperKey, WrapperKey::Hash, WrapperKey::Traits> m_cachedObjCWrappers;
+ NSMapTable *m_cachedObjCWrappers;
}
- (instancetype)initWithGlobalContextRef:(JSGlobalContextRef)context
@@ -660,6 +593,10 @@
if (!self)
return nil;
+ NSPointerFunctionsOptions keyOptions = NSPointerFunctionsOpaqueMemory | NSPointerFunctionsOpaquePersonality;
+ NSPointerFunctionsOptions valueOptions = NSPointerFunctionsWeakMemory | NSPointerFunctionsObjectPersonality;
+ m_cachedObjCWrappers = [[NSMapTable alloc] initWithKeyOptions:keyOptions valueOptions:valueOptions capacity:0];
+
m_cachedJSWrappers = std::make_unique<JSC::WeakGCMap<__unsafe_unretained id, JSC::JSObject>>(toJS(context)->vm());
ASSERT(!toJSGlobalObject(context)->wrapperMap());
@@ -670,6 +607,7 @@
- (void)dealloc
{
+ [m_cachedObjCWrappers release];
[m_classMap release];
[super dealloc];
}
@@ -724,16 +662,14 @@
- (JSValue *)objcWrapperForJSValueRef:(JSValueRef)value inContext:context
{
ASSERT(toJSGlobalObject([context JSGlobalContextRef])->wrapperMap() == self);
- WrapperKey::Translator::ValueAndContext valueAndContext { context, value };
- auto addResult = m_cachedObjCWrappers.add<WrapperKey::Translator>(valueAndContext);
- return addResult.iterator->m_wrapper;
+ JSValue *wrapper = (__bridge JSValue *)NSMapGet(m_cachedObjCWrappers, value);
+ if (!wrapper) {
+ wrapper = [[[JSValue alloc] initWithValue:value inContext:context] autorelease];
+ NSMapInsert(m_cachedObjCWrappers, value, (__bridge void*)wrapper);
+ }
+ return wrapper;
}
-- (void)removeWrapper:(JSValue *)wrapper
-{
- m_cachedObjCWrappers.remove(WrapperKey(wrapper));
-}
-
@end
id tryUnwrapObjcObject(JSGlobalContextRef context, JSValueRef value)
Modified: trunk/Source/_javascript_Core/API/tests/testapi.mm (244322 => 244323)
--- trunk/Source/_javascript_Core/API/tests/testapi.mm 2019-04-16 02:03:41 UTC (rev 244322)
+++ trunk/Source/_javascript_Core/API/tests/testapi.mm 2019-04-16 02:12:26 UTC (rev 244323)
@@ -565,28 +565,12 @@
static void testObjectiveCAPIMain()
{
@autoreleasepool {
- JSVirtualMachine *vm = [[JSVirtualMachine alloc] init];
- JSContext *context = [[JSContext alloc] initWithVirtualMachine:vm];
+ JSVirtualMachine* vm = [[JSVirtualMachine alloc] init];
+ JSContext* context = [[JSContext alloc] initWithVirtualMachine:vm];
[context evaluateScript:@"bad"];
}
@autoreleasepool {
- JSVirtualMachine *vm = [[JSVirtualMachine alloc] init];
- JSContext *context = [[JSContext alloc] initWithVirtualMachine:vm];
- JSValue *number1 = [context evaluateScript:@"42092389"];
- JSValue *number2 = [context evaluateScript:@"42092389"];
- checkResult(@"wrapper cache for numbers", number1 == number2 && number1.isNumber && [number1 toInt32] == 42092389);
- }
-
- @autoreleasepool {
- JSVirtualMachine *vm = [[JSVirtualMachine alloc] init];
- JSContext *context = [[JSContext alloc] initWithVirtualMachine:vm];
- JSValue *object1 = [context evaluateScript:@"({})"];
- JSValue *object2 = [context evaluateScript:@"({})"];
- checkResult(@"wrapper cache for objects", object1 != object2);
- }
-
- @autoreleasepool {
JSContext *context = [[JSContext alloc] init];
JSValue *result = [context evaluateScript:@"2 + 2"];
checkResult(@"2 + 2", result.isNumber && [result toInt32] == 4);
Modified: trunk/Source/_javascript_Core/ChangeLog (244322 => 244323)
--- trunk/Source/_javascript_Core/ChangeLog 2019-04-16 02:03:41 UTC (rev 244322)
+++ trunk/Source/_javascript_Core/ChangeLog 2019-04-16 02:12:26 UTC (rev 244323)
@@ -1,3 +1,19 @@
+2019-04-15 Commit Queue <[email protected]>
+
+ Unreviewed, rolling out r243672.
+ https://bugs.webkit.org/show_bug.cgi?id=196952
+
+ [JSValue release] should be thread-safe (Requested by
+ yusukesuzuki on #webkit).
+
+ Reverted changeset:
+
+ "[JSC] JSWrapperMap should not use Objective-C Weak map
+ (NSMapTable with NSPointerFunctionsWeakMemory) for
+ m_cachedObjCWrappers"
+ https://bugs.webkit.org/show_bug.cgi?id=196392
+ https://trac.webkit.org/changeset/243672
+
2019-04-15 Saam barati <[email protected]>
SafeToExecute for GetByOffset/GetGetterByOffset/PutByOffset is using the wrong child for the base
