Stop using NSMapTable in places where we were only using it to be GC safe
authorweinig@apple.com <weinig@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Feb 2016 04:07:11 +0000 (04:07 +0000)
committerweinig@apple.com <weinig@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 16 Feb 2016 04:07:11 +0000 (04:07 +0000)
<rdar://problem/24063723>
https://bugs.webkit.org/show_bug.cgi?id=154264

Reviewed by Dan Bernstein.

Switch from NSMapTable to HashMap.

* WebCore.xcodeproj/project.pbxproj:
* bindings/objc/DOMInternal.h:
* bindings/objc/DOMInternal.mm:
* bindings/objc/WebScriptObject.mm:
* bridge/objc/objc_instance.mm:
* platform/spi/cocoa/NSPointerFunctionsSPI.h: Removed. No longer used.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@196614 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/WebCore.xcodeproj/project.pbxproj
Source/WebCore/bindings/objc/DOMInternal.h
Source/WebCore/bindings/objc/DOMInternal.mm
Source/WebCore/bindings/objc/WebScriptObject.mm
Source/WebCore/bridge/objc/objc_instance.mm
Source/WebCore/platform/spi/cocoa/NSPointerFunctionsSPI.h [deleted file]

index b7b39fc..4a3ca6f 100644 (file)
@@ -1,3 +1,20 @@
+2016-02-15  Sam Weinig  <sam@webkit.org>
+
+        Stop using NSMapTable in places where we were only using it to be GC safe
+        <rdar://problem/24063723>
+        https://bugs.webkit.org/show_bug.cgi?id=154264
+
+        Reviewed by Dan Bernstein.
+
+        Switch from NSMapTable to HashMap.
+
+        * WebCore.xcodeproj/project.pbxproj:
+        * bindings/objc/DOMInternal.h:
+        * bindings/objc/DOMInternal.mm:
+        * bindings/objc/WebScriptObject.mm:
+        * bridge/objc/objc_instance.mm:
+        * platform/spi/cocoa/NSPointerFunctionsSPI.h: Removed. No longer used.
+
 2016-02-15  Myles C. Maxfield  <mmaxfield@apple.com>
 
         [Font Loading] Implement FontFace JavaScript object
index f10bfc7..3591dfb 100644 (file)
                CE1252411A16B1B600864480 /* MediaPlayerSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE1252401A16B1B600864480 /* MediaPlayerSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
                CE1252431A16C01A00864480 /* CoreUISPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE1252421A16C01A00864480 /* CoreUISPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
                CE1252451A16C22500864480 /* DynamicLinkerSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE1252441A16C22500864480 /* DynamicLinkerSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
-               CE1252471A16C2C200864480 /* NSPointerFunctionsSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE1252461A16C2C200864480 /* NSPointerFunctionsSPI.h */; };
                CE1252491A16C3BC00864480 /* MobileGestaltSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE1252481A16C3BC00864480 /* MobileGestaltSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
                CE12524D1A1A77DE00864480 /* IOPMLibSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE12524C1A1A77DE00864480 /* IOPMLibSPI.h */; };
                CE12524F1A1A78D200864480 /* MachVMSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE12524E1A1A78D200864480 /* MachVMSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
                CE1252401A16B1B600864480 /* MediaPlayerSPI.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MediaPlayerSPI.h; sourceTree = "<group>"; };
                CE1252421A16C01A00864480 /* CoreUISPI.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CoreUISPI.h; sourceTree = "<group>"; };
                CE1252441A16C22500864480 /* DynamicLinkerSPI.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = DynamicLinkerSPI.h; sourceTree = "<group>"; };
-               CE1252461A16C2C200864480 /* NSPointerFunctionsSPI.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = NSPointerFunctionsSPI.h; sourceTree = "<group>"; };
                CE1252481A16C3BC00864480 /* MobileGestaltSPI.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MobileGestaltSPI.h; sourceTree = "<group>"; };
                CE12524C1A1A77DE00864480 /* IOPMLibSPI.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = IOPMLibSPI.h; sourceTree = "<group>"; };
                CE12524E1A1A78D200864480 /* MachVMSPI.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = MachVMSPI.h; sourceTree = "<group>"; };
                                31DF63561AF187DD0078FD91 /* NSColorSPI.h */,
                                2DDB97F319F9AECA002025D8 /* NSExtensionSPI.h */,
                                CE12523A1A16711000864480 /* NSFileManagerSPI.h */,
-                               CE1252461A16C2C200864480 /* NSPointerFunctionsSPI.h */,
                                CE1252521A1BEC0600864480 /* NSStringSPI.h */,
                                31B313DA1B69871600F2AABC /* NSURLConnectionSPI.h */,
                                CE1252541A1BEC0E00864480 /* NSURLDownloadSPI.h */,
                                1C6466281A12C4200094603C /* NSFontSPI.h in Headers */,
                                9321D5901A390704008052BE /* NSImmediateActionGestureRecognizerSPI.h in Headers */,
                                937F4CCE1A2D4B0100BB39F5 /* NSMenuSPI.h in Headers */,
-                               CE1252471A16C2C200864480 /* NSPointerFunctionsSPI.h in Headers */,
                                93F1E1EC1A40FDDC00348D13 /* NSPopoverSPI.h in Headers */,
                                93500F3213FDE3BE0099EC24 /* NSScrollerImpDetails.h in Headers */,
                                F40EA8AB1B867E4400CE5581 /* NSScrollingInputFilterSPI.h in Headers */,
index 7741b89..22a2d05 100644 (file)
@@ -58,9 +58,6 @@ WEBCORE_EXPORT @interface DOMNodeFilter : DOMObject <DOMNodeFilter>
 
 // Helper functions for DOM wrappers and gluing to Objective-C
 
-// Create an NSMapTable mapping from pointers to ObjC objects held with zeroing weak references.
-NSMapTable* createWrapperCache();
-
 id createDOMWrapper(JSC::JSObject*, PassRefPtr<JSC::Bindings::RootObject> origin, PassRefPtr<JSC::Bindings::RootObject> current);
 
 NSObject* getDOMWrapper(DOMObjectInternal*);
index 2a49bf7..7c3045e 100644 (file)
 #import "DOMNodeInternal.h"
 #import "Frame.h"
 #import "JSNode.h"
-#import "NSPointerFunctionsSPI.h"
 #import "ScriptController.h"
 #import "WebScriptObjectPrivate.h"
 #import "runtime_root.h"
+#import <wtf/HashMap.h>
 #import <wtf/Lock.h>
 #import <wtf/NeverDestroyed.h>
-#import <wtf/spi/cocoa/NSMapTableSPI.h>
 
 #if PLATFORM(IOS)
 #define NEEDS_WRAPPER_CACHE_LOCK 1
 //------------------------------------------------------------------------------------------
 // Wrapping WebCore implementation objects
 
-static NSMapTable* DOMWrapperCache;
-    
 #ifdef NEEDS_WRAPPER_CACHE_LOCK
 static StaticLock wrapperCacheLock;
 #endif
 
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
-
-NSMapTable* createWrapperCache()
+static HashMap<DOMObjectInternal*, NSObject *>& wrapperCache()
 {
-    // NSMapTable with zeroing weak pointers is the recommended way to build caches like this under garbage collection.
-    NSPointerFunctionsOptions keyOptions = NSPointerFunctionsOpaqueMemory | NSPointerFunctionsOpaquePersonality;
-    NSPointerFunctionsOptions valueOptions = NSPointerFunctionsZeroingWeakMemory | NSPointerFunctionsObjectPersonality;
-    return [[NSMapTable alloc] initWithKeyOptions:keyOptions valueOptions:valueOptions capacity:0];
+    static NeverDestroyed<HashMap<DOMObjectInternal*, NSObject *>> map;
+    return map;
 }
 
-#pragma clang diagnostic pop
-
 NSObject* getDOMWrapper(DOMObjectInternal* impl)
 {
 #ifdef NEEDS_WRAPPER_CACHE_LOCK
     std::lock_guard<StaticLock> lock(wrapperCacheLock);
 #endif
-    if (!DOMWrapperCache)
-        return nil;
-    return static_cast<NSObject*>(NSMapGet(DOMWrapperCache, impl));
+    return wrapperCache().get(impl);
 }
 
 void addDOMWrapper(NSObject* wrapper, DOMObjectInternal* impl)
@@ -78,9 +66,7 @@ void addDOMWrapper(NSObject* wrapper, DOMObjectInternal* impl)
 #ifdef NEEDS_WRAPPER_CACHE_LOCK
     std::lock_guard<StaticLock> lock(wrapperCacheLock);
 #endif
-    if (!DOMWrapperCache)
-        DOMWrapperCache = createWrapperCache();
-    NSMapInsert(DOMWrapperCache, impl, wrapper);
+    wrapperCache().set(impl, wrapper);
 }
 
 void removeDOMWrapper(DOMObjectInternal* impl)
@@ -88,9 +74,7 @@ void removeDOMWrapper(DOMObjectInternal* impl)
 #ifdef NEEDS_WRAPPER_CACHE_LOCK
     std::lock_guard<StaticLock> lock(wrapperCacheLock);
 #endif
-    if (!DOMWrapperCache)
-        return;
-    NSMapRemove(DOMWrapperCache, impl);
+    wrapperCache().remove(impl);
 }
 
 //------------------------------------------------------------------------------------------
index aa87848..0466329 100644 (file)
 #import <JavaScriptCore/JSContextInternal.h>
 #import <JavaScriptCore/JSValueInternal.h>
 #import <interpreter/CallFrame.h>
+#import <runtime/Completion.h>
 #import <runtime/InitializeThreading.h>
 #import <runtime/JSGlobalObject.h>
 #import <runtime/JSLock.h>
-#import <runtime/Completion.h>
-#import <runtime/Completion.h>
+#import <wtf/HashMap.h>
 #import <wtf/Lock.h>
+#import <wtf/NeverDestroyed.h>
 #import <wtf/Threading.h>
-#import <wtf/spi/cocoa/NSMapTableSPI.h>
 #import <wtf/text/WTFString.h>
 
 using namespace JSC::Bindings;
@@ -71,17 +71,20 @@ using JSC::makeSource;
 
 namespace WebCore {
 
-static NSMapTable* JSWrapperCache;
 static StaticLock spinLock;
 
+static HashMap<JSObject*, NSObject *>& wrapperCache()
+{
+    static NeverDestroyed<HashMap<JSObject*, NSObject *>> map;
+    return map;
+}
+
 NSObject* getJSWrapper(JSObject* impl)
 {
     ASSERT(isMainThread());
     LockHolder holder(&spinLock);
 
-    if (!JSWrapperCache)
-        return nil;
-    NSObject* wrapper = static_cast<NSObject*>(NSMapGet(JSWrapperCache, impl));
+    NSObject* wrapper = wrapperCache().get(impl);
     return wrapper ? [[wrapper retain] autorelease] : nil;
 }
 
@@ -90,28 +93,22 @@ void addJSWrapper(NSObject* wrapper, JSObject* impl)
     ASSERT(isMainThread());
     LockHolder holder(&spinLock);
 
-    if (!JSWrapperCache)
-        JSWrapperCache = createWrapperCache();
-    NSMapInsert(JSWrapperCache, impl, wrapper);
+    wrapperCache().set(impl, wrapper);
 }
 
 void removeJSWrapper(JSObject* impl)
 {
     LockHolder holder(&spinLock);
 
-    if (!JSWrapperCache)
-        return;
-    NSMapRemove(JSWrapperCache, impl);
+    wrapperCache().remove(impl);
 }
 
 static void removeJSWrapperIfRetainCountOne(NSObject* wrapper, JSObject* impl)
 {
     LockHolder holder(&spinLock);
 
-    if (!JSWrapperCache)
-        return;
     if ([wrapper retainCount] == 1)
-        NSMapRemove(JSWrapperCache, impl);
+        wrapperCache().remove(impl);
 }
 
 id createJSWrapper(JSC::JSObject* object, PassRefPtr<JSC::Bindings::RootObject> origin, PassRefPtr<JSC::Bindings::RootObject> root)
index 7ade521..bca2190 100644 (file)
@@ -27,7 +27,6 @@
 #import "objc_instance.h"
 
 #import "JSDOMBinding.h"
-#import "NSPointerFunctionsSPI.h"
 #import "ObjCRuntimeObject.h"
 #import "WebScriptObject.h"
 #import "WebScriptObjectProtocol.h"
@@ -37,7 +36,8 @@
 #import <runtime/JSLock.h>
 #import <runtime/ObjectPrototype.h>
 #import <wtf/Assertions.h>
-#import <wtf/spi/cocoa/NSMapTableSPI.h>
+#import <wtf/HashMap.h>
+#import <wtf/NeverDestroyed.h>
 
 #ifdef NDEBUG
 #define OBJC_LOG(formatAndArgs...) ((void)0)
@@ -53,21 +53,13 @@ using namespace JSC;
 
 static NSString *s_exception;
 static JSGlobalObject* s_exceptionEnvironment; // No need to protect this value, since we just use it for a pointer comparison.
-static NSMapTable *s_instanceWrapperCache;
 
-#pragma clang diagnostic push
-#pragma clang diagnostic ignored "-Wdeprecated-declarations"
-
-static NSMapTable *createInstanceWrapperCache()
+static HashMap<id, ObjcInstance*>& wrapperCache()
 {
-    // NSMapTable with zeroing weak pointers is the recommended way to build caches like this under garbage collection.
-    NSPointerFunctionsOptions keyOptions = NSPointerFunctionsZeroingWeakMemory | NSPointerFunctionsOpaquePersonality;
-    NSPointerFunctionsOptions valueOptions = NSPointerFunctionsOpaqueMemory | NSPointerFunctionsOpaquePersonality;
-    return [[NSMapTable alloc] initWithKeyOptions:keyOptions valueOptions:valueOptions capacity:0];
+    static NeverDestroyed<HashMap<id, ObjcInstance*>> map;
+    return map;
 }
 
-#pragma clang diagnostic pop
-
 RuntimeObject* ObjcInstance::newRuntimeObject(ExecState* exec)
 {
     // FIXME: deprecatedGetDOMStructure uses the prototype off of the wrong global object.
@@ -111,13 +103,14 @@ ObjcInstance::ObjcInstance(id instance, RefPtr<RootObject>&& rootObject)
 
 RefPtr<ObjcInstance> ObjcInstance::create(id instance, RefPtr<RootObject>&& rootObject)
 {
-    if (!s_instanceWrapperCache)
-        s_instanceWrapperCache = createInstanceWrapperCache();
-    if (void* existingWrapper = NSMapGet(s_instanceWrapperCache, instance))
-        return static_cast<ObjcInstance*>(existingWrapper);
-    RefPtr<ObjcInstance> wrapper = adoptRef(new ObjcInstance(instance, WTFMove(rootObject)));
-    NSMapInsert(s_instanceWrapperCache, instance, wrapper.get());
-    return wrapper;
+    auto result = wrapperCache().add(instance, nullptr);
+    if (result.isNewEntry) {
+        RefPtr<ObjcInstance> wrapper = adoptRef(new ObjcInstance(instance, WTFMove(rootObject)));
+        result.iterator->value = wrapper.get();
+        return wrapper;
+    }
+
+    return result.iterator->value;
 }
 
 ObjcInstance::~ObjcInstance() 
@@ -125,9 +118,8 @@ ObjcInstance::~ObjcInstance()
     // Both -finalizeForWebScript and -dealloc/-finalize of _instance may require autorelease pools.
     NSAutoreleasePool* pool = [[NSAutoreleasePool alloc] init];
 
-    ASSERT(s_instanceWrapperCache);
     ASSERT(_instance);
-    NSMapRemove(s_instanceWrapperCache, _instance.get());
+    wrapperCache().remove(_instance.get());
 
     if ([_instance.get() respondsToSelector:@selector(finalizeForWebScript)])
         [_instance.get() performSelector:@selector(finalizeForWebScript)];
diff --git a/Source/WebCore/platform/spi/cocoa/NSPointerFunctionsSPI.h b/Source/WebCore/platform/spi/cocoa/NSPointerFunctionsSPI.h
deleted file mode 100644 (file)
index 47fbd29..0000000
+++ /dev/null
@@ -1,41 +0,0 @@
-/*
- * Copyright (C) 2014 Apple Inc. All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
- * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
- * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
- * THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#ifndef NSPointerFunctionsSPI_h
-#define NSPointerFunctionsSPI_h
-
-#if PLATFORM(MAC) || (PLATFORM(IOS) && USE(APPLE_INTERNAL_SDK))
-
-#include <Foundation/NSPointerFunctions.h>
-
-#else
-
-enum {
-    NSPointerFunctionsZeroingWeakMemory = 1UL << 0,
-};
-
-#endif
-
-#endif // NSPointerFunctionsSPI_h