Use OpaqueJSString rather than JSRetainPtr inside WebKit
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 17 Sep 2018 16:54:55 +0000 (16:54 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 17 Sep 2018 16:54:55 +0000 (16:54 +0000)
https://bugs.webkit.org/show_bug.cgi?id=189652

Reviewed by Saam Barati.

Source/JavaScriptCore:

* API/JSCallbackObjectFunctions.h: Removed an uneeded include of
JSStringRef.h.

* API/JSContext.mm:
(-[JSContext evaluateScript:withSourceURL:]): Use OpaqueJSString::create rather
than JSStringCreateWithCFString, simplifying the code and also obviating the
need for explicit JSStringRelease.
(-[JSContext setName:]): Ditto.

* API/JSStringRef.cpp:
(JSStringIsEqualToUTF8CString): Use adoptRef rather than explicit JSStringRelease.
It seems that additional optimization is possible, obviating the need to allocate
an OpaqueJSString, but that's true almost everywhere else in this patch, too.

* API/JSValue.mm:
(+[JSValue valueWithNewRegularExpressionFromPattern:flags:inContext:]): Use
OpaqueJSString::create and adoptRef as appropriate.
(+[JSValue valueWithNewErrorFromMessage:inContext:]): Ditto.
(+[JSValue valueWithNewSymbolFromDescription:inContext:]): Ditto.
(performPropertyOperation): Ditto.
(-[JSValue invokeMethod:withArguments:]): Ditto.
(valueToObjectWithoutCopy): Ditto.
(containerValueToObject): Ditto.
(valueToString): Ditto.
(objectToValueWithoutCopy): Ditto.
(objectToValue): Ditto.

Source/WebCore:

* Modules/plugins/QuickTimePluginReplacement.mm:
(WebCore::jsValueWithDictionaryInContext): Use OpaqueJSString::create.
(WebCore::jsValueWithAVMetadataItemInContext): Use adoptCF.

* platform/mac/SerializedPlatformRepresentationMac.mm:
(WebCore::jsValueWithDictionaryInContext): Use OpaqueJSString::create.

Source/WebKit:

* Shared/API/c/WKString.cpp: Removed unneeded include of JSStringRef.h.

* WebProcess/Automation/WebAutomationSessionProxy.cpp: Removed unneeded
include of JSRetainPtr.
(WebKit::toJSString): Deleted.
(WebKit::toJSValue): Use OpaqueJSString::create.
(WebKit::callPropertyFunction): Ditto.
(WebKit::evaluate): Use adoptRef.
(WebKit::evaluateJavaScriptCallback): Ditto.
(WebKit::WebAutomationSessionProxy::scriptObjectForFrame):
Use OpaqueJSString::create.
(WebKit::WebAutomationSessionProxy::evaluateJavaScriptFunction): Use
String rather than JSStringRef.

* WebProcess/Plugins/PDF/PDFPlugin.mm: Removed unneeded includes.
(WebKit::PDFPlugin::runScriptsInPDFDocument): Use OpaqueJSString::create.

Source/WebKitLegacy/ios:

* WebView/WebPDFViewIOS.mm:
(-[WebPDFView finishedLoadingWithDataSource:]): Use OpaqueJSString::create.
* WebView/WebPDFViewPlaceholder.mm:
(-[WebPDFViewPlaceholder _evaluateJSForDocument:]): Ditto.

Source/WebKitLegacy/mac:

* WebView/WebPDFRepresentation.mm:
(-[WebPDFRepresentation finishedLoadingWithDataSource:]): Use OpaqueJSString::create.

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

17 files changed:
Source/JavaScriptCore/API/JSCallbackObjectFunctions.h
Source/JavaScriptCore/API/JSContext.mm
Source/JavaScriptCore/API/JSStringRef.cpp
Source/JavaScriptCore/API/JSValue.mm
Source/JavaScriptCore/ChangeLog
Source/WebCore/ChangeLog
Source/WebCore/Modules/plugins/QuickTimePluginReplacement.mm
Source/WebCore/platform/mac/SerializedPlatformRepresentationMac.mm
Source/WebKit/ChangeLog
Source/WebKit/Shared/API/c/WKString.cpp
Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp
Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm
Source/WebKitLegacy/ios/ChangeLog
Source/WebKitLegacy/ios/WebView/WebPDFViewIOS.mm
Source/WebKitLegacy/ios/WebView/WebPDFViewPlaceholder.mm
Source/WebKitLegacy/mac/ChangeLog
Source/WebKitLegacy/mac/WebView/WebPDFRepresentation.mm

index 0b2f04e..15b8608 100644 (file)
@@ -34,7 +34,6 @@
 #include "JSLock.h"
 #include "JSObjectRef.h"
 #include "JSString.h"
-#include "JSStringRef.h"
 #include "OpaqueJSString.h"
 #include "PropertyNameArray.h"
 #include <wtf/Vector.h>
index f4e88c1..b88ceaa 100644 (file)
 - (JSValue *)evaluateScript:(NSString *)script withSourceURL:(NSURL *)sourceURL
 {
     JSValueRef exceptionValue = nullptr;
-    JSStringRef scriptJS = JSStringCreateWithCFString((__bridge CFStringRef)script);
-    JSStringRef sourceURLJS = sourceURL ? JSStringCreateWithCFString((__bridge CFStringRef)[sourceURL absoluteString]) : nullptr;
-    JSValueRef result = JSEvaluateScript(m_context, scriptJS, nullptr, sourceURLJS, 0, &exceptionValue);
-    if (sourceURLJS)
-        JSStringRelease(sourceURLJS);
-    JSStringRelease(scriptJS);
+    auto scriptJS = OpaqueJSString::create(script);
+    auto sourceURLJS = OpaqueJSString::create([sourceURL absoluteString]);
+    JSValueRef result = JSEvaluateScript(m_context, scriptJS.get(), nullptr, sourceURLJS.get(), 0, &exceptionValue);
 
     if (exceptionValue)
         return [self valueFromNotifyException:exceptionValue];
 
 - (void)setName:(NSString *)name
 {
-    JSStringRef nameJS = name ? JSStringCreateWithCFString((__bridge CFStringRef)name) : nullptr;
-    JSGlobalContextSetName(m_context, nameJS);
-    if (nameJS)
-        JSStringRelease(nameJS);
+    JSGlobalContextSetName(m_context, OpaqueJSString::create(name).get());
 }
 
 - (BOOL)_remoteInspectionEnabled
index 9095404..1683aae 100644 (file)
@@ -125,9 +125,5 @@ bool JSStringIsEqual(JSStringRef a, JSStringRef b)
 
 bool JSStringIsEqualToUTF8CString(JSStringRef a, const char* b)
 {
-    JSStringRef bBuf = JSStringCreateWithUTF8CString(b);
-    bool result = JSStringIsEqual(a, bBuf);
-    JSStringRelease(bBuf);
-    
-    return result;
+    return JSStringIsEqual(a, adoptRef(JSStringCreateWithUTF8CString(b)).get());
 }
index 9e49ae2..dc11239 100644 (file)
@@ -124,21 +124,16 @@ NSString * const JSPropertyDescriptorSetKey = @"set";
 
 + (JSValue *)valueWithNewRegularExpressionFromPattern:(NSString *)pattern flags:(NSString *)flags inContext:(JSContext *)context
 {
-    JSStringRef patternString = JSStringCreateWithCFString((__bridge CFStringRef)pattern);
-    JSStringRef flagsString = JSStringCreateWithCFString((__bridge CFStringRef)flags);
-    JSValueRef arguments[2] = { JSValueMakeString([context JSGlobalContextRef], patternString), JSValueMakeString([context JSGlobalContextRef], flagsString) };
-    JSStringRelease(patternString);
-    JSStringRelease(flagsString);
-
+    auto patternString = OpaqueJSString::create(pattern);
+    auto flagsString = OpaqueJSString::create(flags);
+    JSValueRef arguments[2] = { JSValueMakeString([context JSGlobalContextRef], patternString.get()), JSValueMakeString([context JSGlobalContextRef], flagsString.get()) };
     return [JSValue valueWithJSValueRef:JSObjectMakeRegExp([context JSGlobalContextRef], 2, arguments, 0) inContext:context];
 }
 
 + (JSValue *)valueWithNewErrorFromMessage:(NSString *)message inContext:(JSContext *)context
 {
-    JSStringRef string = JSStringCreateWithCFString((__bridge CFStringRef)message);
-    JSValueRef argument = JSValueMakeString([context JSGlobalContextRef], string);
-    JSStringRelease(string);
-
+    auto string = OpaqueJSString::create(message);
+    JSValueRef argument = JSValueMakeString([context JSGlobalContextRef], string.get());
     return [JSValue valueWithJSValueRef:JSObjectMakeError([context JSGlobalContextRef], 1, &argument, 0) inContext:context];
 }
 
@@ -154,13 +149,10 @@ NSString * const JSPropertyDescriptorSetKey = @"set";
 
 + (JSValue *)valueWithNewSymbolFromDescription:(NSString *)description inContext:(JSContext *)context
 {
-    JSStringRef string = JSStringCreateWithCFString((__bridge CFStringRef)description);
-    auto value = [JSValue valueWithJSValueRef:JSValueMakeSymbol([context JSGlobalContextRef], string) inContext:context];
-    JSStringRelease(string);
-    return value;
+    auto string = OpaqueJSString::create(description);
+    return [JSValue valueWithJSValueRef:JSValueMakeSymbol([context JSGlobalContextRef], string.get()) inContext:context];
 }
 
-
 - (id)toObject
 {
     return valueToObject(_context, m_value);
@@ -256,9 +248,8 @@ inline Expected<Result, JSValueRef> performPropertyOperation(NSStringFunction st
     Result result;
     // If it's a NSString already, reduce indirection and just pass the NSString.
     if ([propertyKey isKindOfClass:[NSString class]]) {
-        JSStringRef name = JSStringCreateWithCFString((__bridge CFStringRef)propertyKey);
-        result = stringFunction([context JSGlobalContextRef], object, name, arguments..., &exception);
-        JSStringRelease(name);
+        auto name = OpaqueJSString::create((NSString *)propertyKey);
+        result = stringFunction([context JSGlobalContextRef], object, name.get(), arguments..., &exception);
     } else
         result = jsFunction([context JSGlobalContextRef], object, [[JSValue valueWithObject:propertyKey inContext:context] JSValueRef], arguments..., &exception);
     return Expected<Result, JSValueRef>(result);
@@ -485,9 +476,8 @@ inline Expected<Result, JSValueRef> performPropertyOperation(NSStringFunction st
     if (exception)
         return [_context valueFromNotifyException:exception];
 
-    JSStringRef name = JSStringCreateWithCFString((__bridge CFStringRef)method);
-    JSValueRef function = JSObjectGetProperty([_context JSGlobalContextRef], thisObject, name, &exception);
-    JSStringRelease(name);
+    auto name = OpaqueJSString::create(method);
+    JSValueRef function = JSObjectGetProperty([_context JSGlobalContextRef], thisObject, name.get(), &exception);
     if (exception)
         return [_context valueFromNotifyException:exception];
 
@@ -698,9 +688,8 @@ static JSContainerConvertor::Task valueToObjectWithoutCopy(JSGlobalContextRef co
             primitive = [NSNumber numberWithDouble:JSValueToNumber(context, value, 0)];
         } else if (JSValueIsString(context, value)) {
             // Would be nice to unique strings, too.
-            JSStringRef jsstring = JSValueToStringCopy(context, value, 0);
-            primitive = CFBridgingRelease(JSStringCopyCFString(kCFAllocatorDefault, jsstring));
-            JSStringRelease(jsstring);
+            auto jsstring = adoptRef(JSValueToStringCopy(context, value, 0));
+            primitive = CFBridgingRelease(JSStringCopyCFString(kCFAllocatorDefault, jsstring.get()));
         } else if (JSValueIsNull(context, value))
             primitive = [NSNull null];
         else {
@@ -741,9 +730,8 @@ static id containerValueToObject(JSGlobalContextRef context, JSContainerConverto
             ASSERT([current.objc isKindOfClass:[NSMutableArray class]]);
             NSMutableArray *array = (NSMutableArray *)current.objc;
         
-            JSStringRef lengthString = JSStringCreateWithUTF8CString("length");
-            unsigned length = JSC::toUInt32(JSValueToNumber(context, JSObjectGetProperty(context, js, lengthString, 0), 0));
-            JSStringRelease(lengthString);
+            auto lengthString = OpaqueJSString::create("length"_s);
+            unsigned length = JSC::toUInt32(JSValueToNumber(context, JSObjectGetProperty(context, js, lengthString.get(), 0), 0));
 
             for (unsigned i = 0; i < length; ++i) {
                 id objc = convertor.convert(JSObjectGetPropertyAtIndex(context, js, i, 0));
@@ -803,15 +791,13 @@ id valueToString(JSGlobalContextRef context, JSValueRef value, JSValueRef* excep
             return wrapped;
     }
 
-    JSStringRef jsstring = JSValueToStringCopy(context, value, exception);
+    auto jsstring = adoptRef(JSValueToStringCopy(context, value, exception));
     if (*exception) {
         ASSERT(!jsstring);
         return nil;
     }
 
-    NSString *string = CFBridgingRelease(JSStringCopyCFString(kCFAllocatorDefault, jsstring));
-    JSStringRelease(jsstring);
-    return string;
+    return CFBridgingRelease(JSStringCopyCFString(kCFAllocatorDefault, jsstring.get()));
 }
 
 id valueToDate(JSGlobalContextRef context, JSValueRef value, JSValueRef* exception)
@@ -954,10 +940,8 @@ static ObjcContainerConvertor::Task objectToValueWithoutCopy(JSContext *context,
             return { object, ((JSValue *)object)->m_value, ContainerNone };
 
         if ([object isKindOfClass:[NSString class]]) {
-            JSStringRef string = JSStringCreateWithCFString((__bridge CFStringRef)object);
-            JSValueRef js = JSValueMakeString(contextRef, string);
-            JSStringRelease(string);
-            return { object, js, ContainerNone };
+            auto string = OpaqueJSString::create((NSString *)object);
+            return { object, JSValueMakeString(contextRef, string.get()), ContainerNone };
         }
 
         if ([object isKindOfClass:[NSNumber class]]) {
@@ -1013,13 +997,11 @@ JSValueRef objectToValue(JSContext *context, id object)
             NSDictionary *dictionary = (NSDictionary *)current.objc;
             for (id key in [dictionary keyEnumerator]) {
                 if ([key isKindOfClass:[NSString class]]) {
-                    JSStringRef propertyName = JSStringCreateWithCFString((__bridge CFStringRef)key);
-                    JSObjectSetProperty(contextRef, js, propertyName, convertor.convert([dictionary objectForKey:key]), 0, 0);
-                    JSStringRelease(propertyName);
+                    auto propertyName = OpaqueJSString::create((NSString *)key);
+                    JSObjectSetProperty(contextRef, js, propertyName.get(), convertor.convert([dictionary objectForKey:key]), 0, 0);
                 }
             }
         }
-        
     } while (!convertor.isWorkListEmpty());
 
     return task.js;
index bafeb6e..5d7b9ae 100644 (file)
@@ -1,3 +1,37 @@
+2018-09-17  Darin Adler  <darin@apple.com>
+
+        Use OpaqueJSString rather than JSRetainPtr inside WebKit
+        https://bugs.webkit.org/show_bug.cgi?id=189652
+
+        Reviewed by Saam Barati.
+
+        * API/JSCallbackObjectFunctions.h: Removed an uneeded include of
+        JSStringRef.h.
+
+        * API/JSContext.mm:
+        (-[JSContext evaluateScript:withSourceURL:]): Use OpaqueJSString::create rather
+        than JSStringCreateWithCFString, simplifying the code and also obviating the
+        need for explicit JSStringRelease.
+        (-[JSContext setName:]): Ditto.
+
+        * API/JSStringRef.cpp:
+        (JSStringIsEqualToUTF8CString): Use adoptRef rather than explicit JSStringRelease.
+        It seems that additional optimization is possible, obviating the need to allocate
+        an OpaqueJSString, but that's true almost everywhere else in this patch, too.
+
+        * API/JSValue.mm:
+        (+[JSValue valueWithNewRegularExpressionFromPattern:flags:inContext:]): Use
+        OpaqueJSString::create and adoptRef as appropriate.
+        (+[JSValue valueWithNewErrorFromMessage:inContext:]): Ditto.
+        (+[JSValue valueWithNewSymbolFromDescription:inContext:]): Ditto.
+        (performPropertyOperation): Ditto.
+        (-[JSValue invokeMethod:withArguments:]): Ditto.
+        (valueToObjectWithoutCopy): Ditto.
+        (containerValueToObject): Ditto.
+        (valueToString): Ditto.
+        (objectToValueWithoutCopy): Ditto.
+        (objectToValue): Ditto.
+
 2018-09-08  Darin Adler  <darin@apple.com>
 
         Streamline JSRetainPtr, fix leaks of JSString and JSGlobalContext
index 82ada9d..fb93aa4 100644 (file)
@@ -1,3 +1,17 @@
+2018-09-17  Darin Adler  <darin@apple.com>
+
+        Use OpaqueJSString rather than JSRetainPtr inside WebKit
+        https://bugs.webkit.org/show_bug.cgi?id=189652
+
+        Reviewed by Saam Barati.
+
+        * Modules/plugins/QuickTimePluginReplacement.mm:
+        (WebCore::jsValueWithDictionaryInContext): Use OpaqueJSString::create.
+        (WebCore::jsValueWithAVMetadataItemInContext): Use adoptCF.
+
+        * platform/mac/SerializedPlatformRepresentationMac.mm:
+        (WebCore::jsValueWithDictionaryInContext): Use OpaqueJSString::create.
+
 2018-09-08  Darin Adler  <darin@apple.com>
 
         Streamline JSRetainPtr, fix leaks of JSString and JSGlobalContext
index ca34d81..79ec891 100644 (file)
@@ -255,6 +255,7 @@ void QuickTimePluginReplacement::postEvent(const String& eventName)
 }
 
 #if PLATFORM(IOS)
+
 static JSValue *jsValueWithDataInContext(NSData *data, const String& mimeType, JSContext *context)
 {
     Vector<char> base64Data;
@@ -291,7 +292,6 @@ static JSValue *jsValueWithArrayInContext(NSArray *array, JSContext *context)
     return result;
 }
 
-
 static JSValue *jsValueWithDictionaryInContext(NSDictionary *dictionary, JSContext *context)
 {
     JSValueRef exception = 0;
@@ -308,9 +308,8 @@ static JSValue *jsValueWithDictionaryInContext(NSDictionary *dictionary, JSConte
         if (!value)
             continue;
 
-        JSStringRef name = JSStringCreateWithCFString((__bridge CFStringRef)key);
-        JSObjectSetProperty([context JSGlobalContextRef], resultObject, name, [value JSValueRef], 0, &exception);
-        JSStringRelease(name);
+        auto name = OpaqueJSString::create(key);
+        JSObjectSetProperty([context JSGlobalContextRef], resultObject, name.get(), [value JSValueRef], 0, &exception);
         if (exception)
             continue;
     }
@@ -350,12 +349,8 @@ static JSValue *jsValueWithAVMetadataItemInContext(AVMetadataItemType *item, JSC
         [dictionary setObject:item.locale forKey:@"locale"];
 
     if (CMTIME_IS_VALID(item.time)) {
-        CFDictionaryRef timeDict = PAL::CMTimeCopyAsDictionary(item.time, kCFAllocatorDefault);
-
-        if (timeDict) {
-            [dictionary setObject:(id)timeDict forKey:@"timestamp"];
-            CFRelease(timeDict);
-        }
+        if (auto timeDictionary = adoptCF(PAL::CMTimeCopyAsDictionary(item.time, kCFAllocatorDefault)))
+            [dictionary setObject:(__bridge NSDictionary *)timeDictionary.get() forKey:@"timestamp"];
     }
     
     if (item.value) {
@@ -365,13 +360,14 @@ static JSValue *jsValueWithAVMetadataItemInContext(AVMetadataItemType *item, JSC
             Vector<char> base64Data;
             base64Encode([value bytes], [value length], base64Data);
             String data64 = "data:" + String(mimeType) + ";base64," + base64Data;
-            [dictionary setObject:(id)data64.createCFString().get() forKey:@"value"];
+            [dictionary setObject:(__bridge NSString *)data64.createCFString().get() forKey:@"value"];
         } else
             [dictionary setObject:value forKey:@"value"];
     }
 
     return jsValueWithDictionaryInContext(dictionary, context);
 }
+
 #endif
 
 JSC::JSValue JSQuickTimePluginReplacement::timedMetaData(JSC::ExecState& state) const
@@ -391,7 +387,7 @@ JSC::JSValue JSQuickTimePluginReplacement::timedMetaData(JSC::ExecState& state)
 
     JSContext *jsContext = frame->script().javaScriptContext();
     JSValue *metaDataValue = jsValueWithValueInContext(metaData, jsContext);
-    
+
     return toJS(&state, [metaDataValue JSValueRef]);
 #else
     UNUSED_PARAM(state);
index ded7ec0..dc0d41c 100644 (file)
@@ -190,9 +190,8 @@ static JSValue *jsValueWithDictionaryInContext(NSDictionary *dictionary, JSConte
         if (!value)
             continue;
 
-        JSStringRef name = JSStringCreateWithCFString((__bridge CFStringRef)key);
-        JSObjectSetProperty([context JSGlobalContextRef], resultObject, name, [value JSValueRef], 0, &exception);
-        JSStringRelease(name);
+        auto name = OpaqueJSString::create(key);
+        JSObjectSetProperty([context JSGlobalContextRef], resultObject, name.get(), [value JSValueRef], 0, &exception);
         if (exception)
             continue;
     }
index 9eecaf4..8c9cfe5 100644 (file)
@@ -1,3 +1,27 @@
+2018-09-17  Darin Adler  <darin@apple.com>
+
+        Use OpaqueJSString rather than JSRetainPtr inside WebKit
+        https://bugs.webkit.org/show_bug.cgi?id=189652
+
+        Reviewed by Saam Barati.
+
+        * Shared/API/c/WKString.cpp: Removed unneeded include of JSStringRef.h.
+
+        * WebProcess/Automation/WebAutomationSessionProxy.cpp: Removed unneeded
+        include of JSRetainPtr.
+        (WebKit::toJSString): Deleted.
+        (WebKit::toJSValue): Use OpaqueJSString::create.
+        (WebKit::callPropertyFunction): Ditto.
+        (WebKit::evaluate): Use adoptRef.
+        (WebKit::evaluateJavaScriptCallback): Ditto.
+        (WebKit::WebAutomationSessionProxy::scriptObjectForFrame):
+        Use OpaqueJSString::create.
+        (WebKit::WebAutomationSessionProxy::evaluateJavaScriptFunction): Use
+        String rather than JSStringRef.
+
+        * WebProcess/Plugins/PDF/PDFPlugin.mm: Removed unneeded includes.
+        (WebKit::PDFPlugin::runScriptsInPDFDocument): Use OpaqueJSString::create.
+
 2018-09-17  Michael Catanzaro  <mcatanzaro@igalia.com>
 
         Unreviewed, fix incorrect WPE build fix made in r236009
index 6503670..78de61e 100644 (file)
@@ -29,7 +29,6 @@
 
 #include "WKAPICast.h"
 #include <JavaScriptCore/InitializeThreading.h>
-#include <JavaScriptCore/JSStringRef.h>
 #include <JavaScriptCore/OpaqueJSString.h>
 
 using namespace WebKit;
index fa6df2a..29fb01e 100644 (file)
@@ -38,7 +38,6 @@
 #include "WebProcess.h"
 #include <JavaScriptCore/APICast.h>
 #include <JavaScriptCore/JSObject.h>
-#include <JavaScriptCore/JSRetainPtr.h>
 #include <JavaScriptCore/JSStringRefPrivate.h>
 #include <JavaScriptCore/OpaqueJSString.h>
 #include <WebCore/CookieJar.h>
@@ -87,14 +86,9 @@ static JSObjectRef toJSArray(JSContextRef context, const Vector<T>& data, JSValu
     return array;
 }
 
-static inline JSRetainPtr<JSStringRef> toJSString(const String& string)
-{
-    return adopt(OpaqueJSString::create(string).leakRef());
-}
-
 static inline JSValueRef toJSValue(JSContextRef context, const String& string)
 {
-    return JSValueMakeString(context, toJSString(string).get());
+    return JSValueMakeString(context, OpaqueJSString::create(string).get());
 }
 
 static inline JSValueRef callPropertyFunction(JSContextRef context, JSObjectRef object, const String& propertyName, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
@@ -102,7 +96,7 @@ static inline JSValueRef callPropertyFunction(JSContextRef context, JSObjectRef
     ASSERT_ARG(object, object);
     ASSERT_ARG(object, JSValueIsObject(context, object));
 
-    JSObjectRef function = const_cast<JSObjectRef>(JSObjectGetProperty(context, object, toJSString(propertyName).get(), exception));
+    JSObjectRef function = const_cast<JSObjectRef>(JSObjectGetProperty(context, object, OpaqueJSString::create(propertyName).get(), exception));
     ASSERT(JSObjectIsFunction(context, function));
 
     return JSObjectCallAsFunction(context, function, object, argumentCount, arguments, exception);
@@ -127,7 +121,7 @@ static JSValueRef evaluate(JSContextRef context, JSObjectRef function, JSObjectR
     if (argumentCount != 1)
         return JSValueMakeUndefined(context);
 
-    auto script = adopt(JSValueToStringCopy(context, arguments[0], exception));
+    auto script = adoptRef(JSValueToStringCopy(context, arguments[0], exception));
     return JSEvaluateScript(context, script.get(), nullptr, nullptr, 0, exception);
 }
 
@@ -150,7 +144,7 @@ static JSValueRef evaluateJavaScriptCallback(JSContextRef context, JSObjectRef f
 
     uint64_t frameID = JSValueToNumber(context, arguments[0], exception);
     uint64_t callbackID = JSValueToNumber(context, arguments[1], exception);
-    auto result = adopt(JSValueToStringCopy(context, arguments[2], exception));
+    auto result = adoptRef(JSValueToStringCopy(context, arguments[2], exception));
 
     bool resultIsErrorName = JSValueToBoolean(context, arguments[3]);
 
@@ -183,7 +177,7 @@ JSObjectRef WebAutomationSessionProxy::scriptObjectForFrame(WebFrame& frame)
 
     String script = StringImpl::createWithoutCopying(WebAutomationSessionProxyScriptSource, sizeof(WebAutomationSessionProxyScriptSource));
 
-    JSObjectRef scriptObjectFunction = const_cast<JSObjectRef>(JSEvaluateScript(context, toJSString(script).get(), nullptr, nullptr, 0, &exception));
+    JSObjectRef scriptObjectFunction = const_cast<JSObjectRef>(JSEvaluateScript(context, OpaqueJSString::create(script).get(), nullptr, nullptr, 0, &exception));
     ASSERT(JSValueIsObject(context, scriptObjectFunction));
 
     JSValueRef arguments[] = { sessionIdentifier, evaluateFunction, createUUIDFunction };
@@ -284,25 +278,25 @@ void WebAutomationSessionProxy::evaluateJavaScriptFunction(uint64_t pageID, uint
 
     String errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::JavaScriptError);
 
-    JSRetainPtr<JSStringRef> exceptionMessage;
+    String exceptionMessage;
     if (JSValueIsObject(context, exception)) {
-        JSValueRef nameValue = JSObjectGetProperty(context, const_cast<JSObjectRef>(exception), toJSString("name"_s).get(), nullptr);
-        auto exceptionName = adopt(JSValueToStringCopy(context, nameValue, nullptr));
-        if (exceptionName->string() == "NodeNotFound")
+        JSValueRef nameValue = JSObjectGetProperty(context, const_cast<JSObjectRef>(exception), OpaqueJSString::create("name"_s).get(), nullptr);
+        auto exceptionName = adoptRef(JSValueToStringCopy(context, nameValue, nullptr))->string();
+        if (exceptionName == "NodeNotFound")
             errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::NodeNotFound);
-        else if (exceptionName->string() == "InvalidElementState")
+        else if (exceptionName == "InvalidElementState")
             errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::InvalidElementState);
-        else if (exceptionName->string() == "InvalidParameter")
+        else if (exceptionName == "InvalidParameter")
             errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::InvalidParameter);
-        else if (exceptionName->string() == "InvalidSelector")
+        else if (exceptionName == "InvalidSelector")
             errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::InvalidSelector);
 
-        JSValueRef messageValue = JSObjectGetProperty(context, const_cast<JSObjectRef>(exception), toJSString("message"_s).get(), nullptr);
-        exceptionMessage = adopt(JSValueToStringCopy(context, messageValue, nullptr));
+        JSValueRef messageValue = JSObjectGetProperty(context, const_cast<JSObjectRef>(exception), OpaqueJSString::create("message"_s).get(), nullptr);
+        exceptionMessage = adoptRef(JSValueToStringCopy(context, messageValue, nullptr))->string();
     } else
-        exceptionMessage = adopt(JSValueToStringCopy(context, exception, nullptr));
+        exceptionMessage = adoptRef(JSValueToStringCopy(context, exception, nullptr))->string();
 
-    didEvaluateJavaScriptFunction(frameID, callbackID, exceptionMessage->string(), errorType);
+    didEvaluateJavaScriptFunction(frameID, callbackID, exceptionMessage, errorType);
 }
 
 void WebAutomationSessionProxy::didEvaluateJavaScriptFunction(uint64_t frameID, uint64_t callbackID, const String& result, const String& errorType)
index da04107..71b8cf8 100644 (file)
@@ -48,8 +48,6 @@
 #import "WebProcess.h"
 #import <JavaScriptCore/JSContextRef.h>
 #import <JavaScriptCore/JSObjectRef.h>
-#import <JavaScriptCore/JSStringRef.h>
-#import <JavaScriptCore/JSStringRefCF.h>
 #import <PDFKit/PDFKit.h>
 #import <QuartzCore/QuartzCore.h>
 #import <WebCore/AXObjectCache.h>
@@ -1096,19 +1094,13 @@ void PDFPlugin::runScriptsInPDFDocument()
     Vector<RetainPtr<CFStringRef>> scripts;
     getAllScriptsInPDFDocument([m_pdfDocument documentRef], scripts);
 
-    size_t scriptCount = scripts.size();
-    if (!scriptCount)
+    if (scripts.isEmpty())
         return;
 
     JSGlobalContextRef ctx = JSGlobalContextCreate(0);
     JSObjectRef jsPDFDoc = makeJSPDFDoc(ctx);
-
-    for (size_t i = 0; i < scriptCount; ++i) {
-        JSStringRef script = JSStringCreateWithCFString(scripts[i].get());
-        JSEvaluateScript(ctx, script, jsPDFDoc, 0, 0, 0);
-        JSStringRelease(script);
-    }
-    
+    for (auto& script : scripts)
+        JSEvaluateScript(ctx, OpaqueJSString::create(script.get()).get(), jsPDFDoc, nullptr, 0, nullptr);
     JSGlobalContextRelease(ctx);
 }
 
index cbce570..28b52ae 100644 (file)
@@ -1,3 +1,15 @@
+2018-09-17  Darin Adler  <darin@apple.com>
+
+        Use OpaqueJSString rather than JSRetainPtr inside WebKit
+        https://bugs.webkit.org/show_bug.cgi?id=189652
+
+        Reviewed by Saam Barati.
+
+        * WebView/WebPDFViewIOS.mm:
+        (-[WebPDFView finishedLoadingWithDataSource:]): Use OpaqueJSString::create.
+        * WebView/WebPDFViewPlaceholder.mm:
+        (-[WebPDFViewPlaceholder _evaluateJSForDocument:]): Ditto.
+
 2018-08-29  Olivia Barnett  <obarnett@apple.com>
 
         Implement the Web Share API
index ec1a8ec..1841b98 100644 (file)
@@ -34,8 +34,7 @@
 #import "WebPDFDocumentExtras.h"
 #import "WebPDFViewPlaceholder.h"
 #import <JavaScriptCore/JSContextRef.h>
-#import <JavaScriptCore/JSStringRef.h>
-#import <JavaScriptCore/JSStringRefCF.h>
+#import <JavaScriptCore/OpaqueJSString.h>
 #import <WebCore/Color.h>
 #import <WebCore/Frame.h>
 #import <WebCore/FrameLoader.h>
@@ -321,19 +320,13 @@ static CGColorRef createCGColorWithDeviceWhite(CGFloat white, CGFloat alpha)
 
     NSArray *scripts = allScriptsInPDFDocument(_PDFDocument);
 
-    NSUInteger scriptCount = [scripts count];
-    if (!scriptCount)
+    if (![scripts count])
         return;
 
     JSGlobalContextRef ctx = JSGlobalContextCreate(0);
     JSObjectRef jsPDFDoc = makeJSPDFDoc(ctx, dataSource);
-
-    for (NSUInteger i = 0; i < scriptCount; ++i) {
-        JSStringRef script = JSStringCreateWithCFString((CFStringRef)[scripts objectAtIndex:i]);
-        JSEvaluateScript(ctx, script, jsPDFDoc, 0, 0, 0);
-        JSStringRelease(script);
-    }
-
+    for (NSString *script in scripts)
+        JSEvaluateScript(ctx, OpaqueJSString::create(script).get(), jsPDFDoc, nullptr, 0, nullptr);
     JSGlobalContextRelease(ctx);
 
     [self setNeedsDisplay:YES];
index bf68f3a..93b1566 100644 (file)
@@ -30,8 +30,6 @@
 #import "WebFrameInternal.h"
 #import "WebPDFViewIOS.h"
 #import <JavaScriptCore/JSContextRef.h>
-#import <JavaScriptCore/JSStringRef.h>
-#import <JavaScriptCore/JSStringRefCF.h>
 #import <WebCore/DataTransfer.h>
 #import <WebCore/EventHandler.h>
 #import <WebCore/EventNames.h>
@@ -309,18 +307,11 @@ static const float PAGE_HEIGHT_INSET = 4.0f * 2.0f;
 
     NSArray *scripts = allScriptsInPDFDocument(pdfDocument);
 
-    NSUInteger scriptCount = [scripts count];
-    if (scriptCount) {
-
+    if ([scripts count]) {
         JSGlobalContextRef ctx = JSGlobalContextCreate(0);
         JSObjectRef jsPDFDoc = makeJSPDFDoc(ctx, _dataSource);
-
-        for (NSUInteger i = 0; i < scriptCount; ++i) {
-            JSStringRef script = JSStringCreateWithCFString((CFStringRef)[scripts objectAtIndex:i]);
-            JSEvaluateScript(ctx, script, jsPDFDoc, 0, 0, 0);
-            JSStringRelease(script);
-        }
-
+        for (NSString *script in scripts)
+            JSEvaluateScript(ctx, OpaqueJSString::create(script).get(), jsPDFDoc, nullptr, 0, nullptr);
         JSGlobalContextRelease(ctx);
     }
 }
index 7c96424..28da8e8 100644 (file)
@@ -1,3 +1,13 @@
+2018-09-17  Darin Adler  <darin@apple.com>
+
+        Use OpaqueJSString rather than JSRetainPtr inside WebKit
+        https://bugs.webkit.org/show_bug.cgi?id=189652
+
+        Reviewed by Saam Barati.
+
+        * WebView/WebPDFRepresentation.mm:
+        (-[WebPDFRepresentation finishedLoadingWithDataSource:]): Use OpaqueJSString::create.
+
 2018-09-14  Jer Noble  <jer.noble@apple.com>
 
         Turn SourceBufferChangeTypeEnabled on by default
index 79db38a..aadfed2 100644 (file)
@@ -37,8 +37,7 @@
 #import "WebPDFView.h"
 #import "WebTypesInternal.h"
 #import <JavaScriptCore/JSContextRef.h>
-#import <JavaScriptCore/JSStringRef.h>
-#import <JavaScriptCore/JSStringRefCF.h>
+#import <JavaScriptCore/OpaqueJSString.h>
 #import <wtf/Assertions.h>
 #import <wtf/ObjcRuntimeExtras.h>
 #import <wtf/RetainPtr.h>
     [doc release];
     doc = nil;
 
-    NSUInteger scriptCount = [scripts count];
-    if (!scriptCount)
+    if (![scripts count])
         return;
 
     JSGlobalContextRef ctx = JSGlobalContextCreate(0);
     JSObjectRef jsPDFDoc = makeJSPDFDoc(ctx, dataSource);
-
-    for (NSUInteger i = 0; i < scriptCount; ++i) {
-        JSStringRef script = JSStringCreateWithCFString((CFStringRef)[scripts objectAtIndex:i]);
-        JSEvaluateScript(ctx, script, jsPDFDoc, 0, 0, 0);
-        JSStringRelease(script);
-    }
-
+    for (NSString *script in scripts)
+        JSEvaluateScript(ctx, OpaqueJSString::create(script).get(), jsPDFDoc, nullptr, 0, nullptr);
     JSGlobalContextRelease(ctx);
 }