Attachment 342807 Details for Bug 186579 – Patch

[patch] Patch

bug-186579-20180615153525.patch (text/plain), 13.27 KB, created by Tadeu Zagallo on 2018-06-15 06:35:27 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tadeu Zagallo

Created: 2018-06-15 06:35:27 PDT

Size: 13.27 KB

patch

obsolete

>Subversion Revision: 232634
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index e89b57395009cc09d3fd8a8460af9319cfa14bf1..860be538063233a3781eb3033fa0270e3679f0d4 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,27 @@
>+2018-06-14  Tadeu Zagallo  <tzagallo@apple.com>
>+
>+        Share structure across instances of classes exported through the ObjC API
>+        https://bugs.webkit.org/show_bug.cgi?id=186579
>+        <rdar://problem/40969212>
>+
>+        Reviewed by Saam Barati.
>+
>+        A new structure was being created for each instance of exported ObjC
>+        classes due to setting the prototype in the structure for every object,
>+        since prototype transitions are not cached by the structure. Cache the
>+        Structure in the JSObjcClassInfo to avoid the transition.
>+
>+        * API/JSWrapperMap.mm:
>+        (-[JSObjCClassInfo wrapperForObject:inContext:]):
>+        (-[JSObjCClassInfo structureInContext:]):
>+        * API/tests/JSWrapperMapTests.h: Added.
>+        * API/tests/JSWrapperMapTests.mm: Added.
>+        (+[JSWrapperMapTests testStructureIdentity]):
>+        (runJSWrapperMapTests):
>+        * API/tests/testapi.mm:
>+        (testObjectiveCAPIMain):
>+        * JavaScriptCore.xcodeproj/project.pbxproj:
>+
> 2018-06-07  Chris Dumez  <cdumez@apple.com>
> 
>         Add base class to get WeakPtrFactory member and avoid some boilerplate code
>diff --git a/Source/JavaScriptCore/API/JSWrapperMap.mm b/Source/JavaScriptCore/API/JSWrapperMap.mm
>index 4f5a13b8012a657ae219ae6042aa092e8f37c209..019923ac9610ded144040c04cd73726a2d3d3afe 100644
>--- a/Source/JavaScriptCore/API/JSWrapperMap.mm
>+++ b/Source/JavaScriptCore/API/JSWrapperMap.mm
>@@ -367,6 +367,7 @@ @interface JSObjCClassInfo : NSObject {
>     JSClassRef m_classRef;
>     JSC::Weak<JSC::JSObject> m_prototype;
>     JSC::Weak<JSC::JSObject> m_constructor;
>+    JSC::Weak<JSC::Structure> m_structure;
> }
> 
> - (instancetype)initForClass:(Class)cls;
>@@ -517,10 +518,14 @@ - (JSC::JSObject*)wrapperForObject:(id)object inContext:(JSContext *)context
>         }
>     }
> 
>-    JSC::JSObject* prototype = [self prototypeInContext:context];
>+    JSC::Structure* structure = [self structureInContext:context];
> 
>-    JSC::JSObject* wrapper = makeWrapper([context JSGlobalContextRef], m_classRef, object);
>-    JSObjectSetPrototype([context JSGlobalContextRef], toRef(wrapper), toRef(prototype));
>+    JSC::ExecState* exec = toJS([context JSGlobalContextRef]);
>+    JSC::VM& vm = exec->vm();
>+    JSC::JSLockHolder locker(vm);
>+
>+    JSC::JSCallbackObject<JSC::JSAPIWrapperObject>* wrapper = JSC::JSCallbackObject<JSC::JSAPIWrapperObject>::create(exec, exec->lexicalGlobalObject(), structure, m_classRef, 0);
>+    wrapper->setWrappedObject(object);
>     return wrapper;
> }
> 
>@@ -542,6 +547,20 @@ - (JSC::JSObject*)prototypeInContext:(JSContext *)context
>     return prototype;
> }
> 
>+- (JSC::Structure*)structureInContext:(JSContext *)context
>+{
>+    JSC::Structure* structure = m_structure.get();
>+    if (structure)
>+        return structure;
>+
>+    JSC::ExecState* exec = toJS([context JSGlobalContextRef]);
>+    JSC::JSGlobalObject* globalObject = toJSGlobalObject([context JSGlobalContextRef]);
>+    JSC::JSObject* prototype = [self prototypeInContext:context];
>+    m_structure = JSC::JSCallbackObject<JSC::JSAPIWrapperObject>::createStructure(exec->vm(), globalObject, prototype);
>+
>+    return m_structure.get();
>+}
>+
> @end
> 
> @implementation JSWrapperMap {
>diff --git a/Source/JavaScriptCore/API/tests/JSWrapperMapTests.h b/Source/JavaScriptCore/API/tests/JSWrapperMapTests.h
>new file mode 100644
>index 0000000000000000000000000000000000000000..f4d813ecd0376e2bef683c9584ebce17942e995c
>--- /dev/null
>+++ b/Source/JavaScriptCore/API/tests/JSWrapperMapTests.h
>@@ -0,0 +1,34 @@
>+/*
>+ * Copyright (C) 2018 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.
>+ */
>+
>+#import <Foundation/Foundation.h>
>+#import <JavaScriptCore/JavaScriptCore.h>
>+
>+#if JSC_OBJC_API_ENABLED
>+
>+void runJSWrapperMapTests();
>+
>+#endif // JSC_OBJC_API_ENABLED
>+
>diff --git a/Source/JavaScriptCore/API/tests/JSWrapperMapTests.mm b/Source/JavaScriptCore/API/tests/JSWrapperMapTests.mm
>new file mode 100644
>index 0000000000000000000000000000000000000000..196cf0f0cbf11f945174d6d3a479356954d0c205
>--- /dev/null
>+++ b/Source/JavaScriptCore/API/tests/JSWrapperMapTests.mm
>@@ -0,0 +1,78 @@
>+/*
>+ * Copyright (C) 2018 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.
>+ */
>+
>+#import "config.h"
>+#import "JSWrapperMapTests.h"
>+
>+#import "APICast.h"
>+#import "JSValue.h"
>+
>+#if JSC_OBJC_API_ENABLED
>+
>+extern "C" void checkResult(NSString *description, bool passed);
>+
>+@protocol TestClassJSExport <JSExport>
>+- (instancetype)init;
>+@end
>+
>+@interface TestClass : NSObject <TestClassJSExport>
>+@end
>+
>+@implementation TestClass
>+@end
>+
>+
>+@interface JSWrapperMapTests : NSObject
>++ (void)testStructureIdentity;
>+@end
>+
>+
>+@implementation JSWrapperMapTests
>++ (void)testStructureIdentity
>+{
>+    JSContext* context = [[JSContext alloc] init];
>+    JSGlobalContextRef contextRef = JSGlobalContextRetain(context.JSGlobalContextRef);
>+    JSC::ExecState* exec = toJS(contextRef);
>+
>+    context[@"TestClass"] = [TestClass class];
>+    JSValue* aWrapper = [context evaluateScript:@"new TestClass()"];
>+    JSValue* bWrapper = [context evaluateScript:@"new TestClass()"];
>+    JSC::JSValue aValue = toJS(exec, aWrapper.JSValueRef);
>+    JSC::JSValue bValue = toJS(exec, bWrapper.JSValueRef);
>+    JSC::Structure* aStructure = aValue.structureOrNull();
>+    JSC::Structure* bStructure = bValue.structureOrNull();
>+    checkResult(@"structure should not be null", !!aStructure);
>+    checkResult(@"both wrappers should share the same structure", aStructure == bStructure);
>+}
>+@end
>+
>+void runJSWrapperMapTests()
>+{
>+    @autoreleasepool {
>+        [JSWrapperMapTests testStructureIdentity];
>+    }
>+}
>+
>+#endif // JSC_OBJC_API_ENABLED
>diff --git a/Source/JavaScriptCore/API/tests/testapi.mm b/Source/JavaScriptCore/API/tests/testapi.mm
>index 4119cb7f2d124d15693d14956fd884d57220d301..d7cc2656831bf54412e4d8779af53a349e2a093f 100644
>--- a/Source/JavaScriptCore/API/tests/testapi.mm
>+++ b/Source/JavaScriptCore/API/tests/testapi.mm
>@@ -29,6 +29,7 @@
> #import "DateTests.h"
> #import "JSExportTests.h"
> #import "JSVirtualMachinePrivate.h"
>+#import "JSWrapperMapTests.h"
> #import "Regress141275.h"
> #import "Regress141809.h"
> 
>@@ -1475,6 +1476,7 @@ static void testObjectiveCAPIMain()
>     currentThisInsideBlockGetterTest();
>     runDateTests();
>     runJSExportTests();
>+    runJSWrapperMapTests();
>     runRegress141275();
>     runRegress141809();
> }
>diff --git a/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj b/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
>index 298d61411a286da7cd35f94bc7b1c7d194ad4a34..1530650bddce86d32e8a8b49d6dda5a3540bcf77 100644
>--- a/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
>+++ b/Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj
>@@ -762,6 +762,7 @@
> 		1442566215EDE98D0066A49B /* JSWithScope.h in Headers */ = {isa = PBXBuildFile; fileRef = 1442566015EDE98D0066A49B /* JSWithScope.h */; settings = {ATTRIBUTES = (Private, ); }; };
> 		144836E7132DA7BE005BE785 /* ConservativeRoots.h in Headers */ = {isa = PBXBuildFile; fileRef = 149DAAF212EB559D0083B12B /* ConservativeRoots.h */; settings = {ATTRIBUTES = (Private, ); }; };
> 		145722861437E140005FDE26 /* StrongInlines.h in Headers */ = {isa = PBXBuildFile; fileRef = 145722851437E140005FDE26 /* StrongInlines.h */; settings = {ATTRIBUTES = (Private, ); }; };
>+		1471483020D323D30090E630 /* JSWrapperMapTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 1471482F20D323650090E630 /* JSWrapperMapTests.mm */; };
> 		147341CC1DC02D7200AA29BA /* ExecutableBase.h in Headers */ = {isa = PBXBuildFile; fileRef = 147341CB1DC02D7200AA29BA /* ExecutableBase.h */; settings = {ATTRIBUTES = (Private, ); }; };
> 		147341CE1DC02D7900AA29BA /* ScriptExecutable.h in Headers */ = {isa = PBXBuildFile; fileRef = 147341CD1DC02D7900AA29BA /* ScriptExecutable.h */; settings = {ATTRIBUTES = (Private, ); }; };
> 		147341D01DC02DB400AA29BA /* NativeExecutable.h in Headers */ = {isa = PBXBuildFile; fileRef = 147341CF1DC02DB400AA29BA /* NativeExecutable.h */; settings = {ATTRIBUTES = (Private, ); }; };
>@@ -3083,6 +3084,8 @@
> 		146B14DB12EB5B12001BEC1B /* ConservativeRoots.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ConservativeRoots.cpp; sourceTree = "<group>"; };
> 		146FA5A81378F6B0003627A3 /* HandleTypes.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = HandleTypes.h; sourceTree = "<group>"; };
> 		146FE51111A710430087AE66 /* JITCall32_64.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JITCall32_64.cpp; sourceTree = "<group>"; };
>+		1471482E20D323640090E630 /* JSWrapperMapTests.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = JSWrapperMapTests.h; path = API/tests/JSWrapperMapTests.h; sourceTree = "<group>"; };
>+		1471482F20D323650090E630 /* JSWrapperMapTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = JSWrapperMapTests.mm; path = API/tests/JSWrapperMapTests.mm; sourceTree = "<group>"; };
> 		147341CB1DC02D7200AA29BA /* ExecutableBase.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ExecutableBase.h; sourceTree = "<group>"; };
> 		147341CD1DC02D7900AA29BA /* ScriptExecutable.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ScriptExecutable.h; sourceTree = "<group>"; };
> 		147341CF1DC02DB400AA29BA /* NativeExecutable.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = NativeExecutable.h; sourceTree = "<group>"; };
>@@ -5431,6 +5434,8 @@
> 				0FF47C591EBFE83500F280B7 /* JSObjectGetProxyTargetTest.h */,
> 				5C4E8E941DBEBDA20036F1FC /* JSONParseTest.cpp */,
> 				5C4E8E951DBEBDA20036F1FC /* JSONParseTest.h */,
>+				1471482E20D323640090E630 /* JSWrapperMapTests.h */,
>+				1471482F20D323650090E630 /* JSWrapperMapTests.mm */,
> 				144005170A531CB50005F061 /* minidom */,
> 				FEF49AA91EB947FE00653BDB /* MultithreadedMultiVMExecutionTest.cpp */,
> 				FEF49AAA1EB947FE00653BDB /* MultithreadedMultiVMExecutionTest.h */,
>@@ -10152,6 +10157,7 @@
> 				C2181FC218A948FB0025A235 /* JSExportTests.mm in Sources */,
> 				0FF47C5A1EBFE84600F280B7 /* JSObjectGetProxyTargetTest.cpp in Sources */,
> 				5C4E8E961DBEBE620036F1FC /* JSONParseTest.cpp in Sources */,
>+				1471483020D323D30090E630 /* JSWrapperMapTests.mm in Sources */,
> 				FEF49AAB1EB9484B00653BDB /* MultithreadedMultiVMExecutionTest.cpp in Sources */,
> 				FE7C41961B97FC4B00F4D598 /* PingPongStackOverflowTest.cpp in Sources */,
> 				65570F5A1AA4C3EA009B3C23 /* Regress141275.mm in Sources */,

Actions: View | Formatted Diff | Diff

Attachments on bug 186579: 342610 | 342672 | 342730 | 342733 | 342773 | 342776 | 342807 | 342833 | 342850