Tidy up JSScriptRef API
authoroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 19 Dec 2012 23:36:13 +0000 (23:36 +0000)
committeroliver@apple.com <oliver@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 19 Dec 2012 23:36:13 +0000 (23:36 +0000)
https://bugs.webkit.org/show_bug.cgi?id=105470

Reviewed by Anders Carlsson.

People found the API's use of a context confusing, so we'll switch to a JSContextGroup based
API, and drop a number of the unnecessary uses of contexts.

* API/JSScriptRef.cpp:
(OpaqueJSScript::globalData):
(parseScript):
* API/JSScriptRefPrivate.h:
* API/tests/testapi.c:
(main):

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

Source/JavaScriptCore/API/JSScriptRef.cpp
Source/JavaScriptCore/API/JSScriptRefPrivate.h
Source/JavaScriptCore/API/tests/testapi.c
Source/JavaScriptCore/ChangeLog

index 8e5d695..9789860 100644 (file)
@@ -32,6 +32,7 @@
 #include "JSGlobalData.h"
 #include "JSScriptRefPrivate.h"
 #include "OpaqueJSString.h"
+#include "Parser.h"
 #include "SourceCode.h"
 #include "SourceProvider.h"
 
@@ -49,7 +50,7 @@ public:
         return m_source;
     }
 
-    const JSGlobalData* globalData() const { return m_globalData; }
+    JSGlobalData* globalData() const { return m_globalData; }
 
 private:
     OpaqueJSScript(JSGlobalData* globalData, const String& url, int startingLineNumber, const String& source)
@@ -65,60 +66,64 @@ private:
     String m_source;
 };
 
+static bool parseScript(JSGlobalData* globalData, const SourceCode& source, ParserError& error)
+{
+    return JSC::parse<JSC::ProgramNode>(globalData, source, 0, Identifier(), JSParseNormal, JSParseProgramCode, error);
+}
+
 extern "C" {
 
-JSScriptRef JSScriptCreateReferencingImmortalASCIIText(JSContextRef context, JSStringRef url, int startingLineNumber, const char* source, size_t length, JSValueRef* exception)
+JSScriptRef JSScriptCreateReferencingImmortalASCIIText(JSContextGroupRef contextGroup, JSStringRef url, int startingLineNumber, const char* source, size_t length, JSStringRef* errorMessage, int* errorLine)
 {
-    ExecState* exec = toJS(context);
-    APIEntryShim entryShim(exec);
-    JSGlobalData* globalData = &exec->globalData();
+    JSGlobalData* globalData = toJS(contextGroup);
+    APIEntryShim entryShim(globalData);
     for (size_t i = 0; i < length; i++) {
         if (!isASCII(source[i]))
             return 0;
     }
 
     RefPtr<OpaqueJSScript> result = OpaqueJSScript::create(globalData, url->string(), startingLineNumber, String(StringImpl::createFromLiteral(source, length)));
-    JSValue exceptionValue;
-    if (!checkSyntax(exec, SourceCode(result), &exceptionValue)) {
-        if (exception)
-            *exception = toRef(exec, exceptionValue);
+
+    ParserError error;
+    if (!parseScript(globalData, SourceCode(result), error)) {
+        if (errorMessage)
+            *errorMessage = OpaqueJSString::create(error.m_message).leakRef();
+        if (errorLine)
+            *errorLine = error.m_line;
         return 0;
     }
 
     return result.release().leakRef();
 }
 
-JSScriptRef JSScriptCreateFromString(JSContextRef context, JSStringRef url, int startingLineNumber, JSStringRef source, JSValueRef* exception)
+JSScriptRef JSScriptCreateFromString(JSContextGroupRef contextGroup, JSStringRef url, int startingLineNumber, JSStringRef source, JSStringRef* errorMessage, int* errorLine)
 {
-    ExecState* exec = toJS(context);
-    APIEntryShim entryShim(exec);
-    JSGlobalData* globalData = &exec->globalData();
+    JSGlobalData* globalData = toJS(contextGroup);
+    APIEntryShim entryShim(globalData);
 
     RefPtr<OpaqueJSScript> result = OpaqueJSScript::create(globalData, url->string(), startingLineNumber, source->string());
-    JSValue exceptionValue;
-    if (!checkSyntax(exec, SourceCode(result), &exceptionValue)) {
-        if (exception)
-            *exception = toRef(exec, exceptionValue);
+
+    ParserError error;
+    if (!parseScript(globalData, SourceCode(result), error)) {
+        if (errorMessage)
+            *errorMessage = OpaqueJSString::create(error.m_message).leakRef();
+        if (errorLine)
+            *errorLine = error.m_line;
         return 0;
     }
+
     return result.release().leakRef();
 }
 
-void JSScriptRetain(JSContextRef context, JSScriptRef script)
+void JSScriptRetain(JSScriptRef script)
 {
-    ExecState* exec = toJS(context);
-    APIEntryShim entryShim(exec);
-    if (script->globalData() != &exec->globalData())
-        CRASH();
+    APIEntryShim entryShim(script->globalData());
     script->ref();
 }
 
-void JSScriptRelease(JSContextRef context, JSScriptRef script)
+void JSScriptRelease(JSScriptRef script)
 {
-    ExecState* exec = toJS(context);
-    APIEntryShim entryShim(exec);
-    if (script->globalData() != &exec->globalData())
-        CRASH();
+    APIEntryShim entryShim(script->globalData());
     script->deref();
 }
 
index a106385..073798d 100644 (file)
@@ -40,45 +40,45 @@ typedef struct OpaqueJSScript* JSScriptRef;
 /*!
  @function
  @abstract Creates a script reference from an ascii string, without copying or taking ownership of the string
- @param ctx The context to use.
+ @param contextGroup The context group the script is to be used in.
  @param url The source url to be reported in errors and exceptions.
  @param startingLineNumber An integer value specifying the script's starting line number in the file located at sourceURL. This is only used when reporting exceptions.
  @param source The source string.  This is required to be pure ASCII and to never be deallocated.
  @param length The length of the source string.
- @param exception A pointer to a JSValueRef in which to store an exception if the source is not valid. Pass NULL if you do not care to store an exception.
+ @param errorMessage A pointer to a JSStringRef in which to store the parse error message if the source is not valid. Pass NULL if you do not care to store an error message.
+ @param errorLine A pointer to an int in which to store the line number of a parser error. Pass NULL if you do not care to store an error line.
  @result A JSScriptRef for the provided source, or NULL if any non-ASCII character is found in source or if the source is not a valid JavaScript program. Ownership follows the Create Rule.
  @discussion Use this function to create a reusable script reference with a constant
  buffer as the backing string.  The source string must outlive the global context.
  */
-JS_EXPORT JSScriptRef JSScriptCreateReferencingImmortalASCIIText(JSContextRef ctx, JSStringRef url, int startingLineNumber, const char* source, size_t length, JSValueRef* exception);
+JS_EXPORT JSScriptRef JSScriptCreateReferencingImmortalASCIIText(JSContextGroupRef contextGroup, JSStringRef url, int startingLineNumber, const char* source, size_t length, JSStringRef* errorMessage, int* errorLine);
 
 /*!
  @function
  @abstract Creates a script reference from a string
- @param ctx The context to use.
+ @param contextGroup The context group the script is to be used in.
  @param url The source url to be reported in errors and exceptions.
  @param startingLineNumber An integer value specifying the script's starting line number in the file located at sourceURL. This is only used when reporting exceptions.
  @param source The source string.
- @param exception A pointer to a JSValueRef in which to store an exception if the source is not valid. Pass NULL if you do not care to store an exception.
+ @param errorMessage A pointer to a JSStringRef in which to store the parse error message if the source is not valid. Pass NULL if you do not care to store an error message.
+ @param errorLine A pointer to an int in which to store the line number of a parser error. Pass NULL if you do not care to store an error line.
  @result A JSScriptRef for the provided source, or NULL is the source is not a valid JavaScript program.  Ownership follows the Create Rule.
  */
-JS_EXPORT JSScriptRef JSScriptCreateFromString(JSContextRef ctx, JSStringRef url, int startingLineNumber, JSStringRef source, JSValueRef* exception);
+JS_EXPORT JSScriptRef JSScriptCreateFromString(JSContextGroupRef contextGroup, JSStringRef url, int startingLineNumber, JSStringRef source, JSStringRef* errorMessage, int* errorLine);
 
 /*!
  @function
  @abstract Retains a JavaScript script.
- @param ctx The context that the script is defined in.
  @param script The script to retain.
  */
-JS_EXPORT void JSScriptRetain(JSContextRef ctx, JSScriptRef script);
+JS_EXPORT void JSScriptRetain(JSScriptRef script);
 
 /*!
  @function
  @abstract Releases a JavaScript script.
- @param ctx The context that the script is defined in.
  @param script The script to release.
  */
-JS_EXPORT void JSScriptRelease(JSContextRef ctx, JSScriptRef script);
+JS_EXPORT void JSScriptRelease(JSScriptRef script);
 
 /*!
  @function
index 11c5045..25b945a 100644 (file)
@@ -1047,6 +1047,7 @@ int main(int argc, char* argv[])
     
     // Test garbage collection with a fresh context
     context = JSGlobalContextCreateInGroup(NULL, NULL);
+    JSContextGroupRef contextGroup = JSContextGetGroup(context);
     TestInitializeFinalize = true;
     testInitializeFinalize();
     JSGlobalContextRelease(context);
@@ -1404,8 +1405,8 @@ int main(int argc, char* argv[])
     JSStringRef badSyntax = JSStringCreateWithUTF8CString(badSyntaxConstant);
     ASSERT(JSCheckScriptSyntax(context, goodSyntax, NULL, 0, NULL));
     ASSERT(!JSCheckScriptSyntax(context, badSyntax, NULL, 0, NULL));
-    ASSERT(!JSScriptCreateFromString(context, 0, 0, badSyntax, 0));
-    ASSERT(!JSScriptCreateReferencingImmortalASCIIText(context, 0, 0, badSyntaxConstant, strlen(badSyntaxConstant), 0));
+    ASSERT(!JSScriptCreateFromString(contextGroup, 0, 0, badSyntax, 0, 0));
+    ASSERT(!JSScriptCreateReferencingImmortalASCIIText(contextGroup, 0, 0, badSyntaxConstant, strlen(badSyntaxConstant), 0, 0));
 
     JSValueRef result;
     JSValueRef v;
@@ -1602,12 +1603,12 @@ int main(int argc, char* argv[])
     ASSERT(JSValueIsEqual(context, v, o, NULL));
     JSStringRelease(script);
 
-    JSScriptRef scriptObject = JSScriptCreateReferencingImmortalASCIIText(context, 0, 0, thisScript, strlen(thisScript), 0);
+    JSScriptRef scriptObject = JSScriptCreateReferencingImmortalASCIIText(contextGroup, 0, 0, thisScript, strlen(thisScript), 0, 0);
     v = JSScriptEvaluate(context, scriptObject, NULL, NULL);
     ASSERT(JSValueIsEqual(context, v, globalObject, NULL));
     v = JSScriptEvaluate(context, scriptObject, o, NULL);
     ASSERT(JSValueIsEqual(context, v, o, NULL));
-    JSScriptRelease(context, scriptObject);
+    JSScriptRelease(scriptObject);
 
     script = JSStringCreateWithUTF8CString("eval(this);");
     v = JSEvaluateScript(context, script, NULL, NULL, 1, NULL);
@@ -1630,10 +1631,16 @@ int main(int argc, char* argv[])
     } else {
         JSStringRef url = JSStringCreateWithUTF8CString(scriptPath);
         JSStringRef script = JSStringCreateWithUTF8CString(scriptUTF8);
-        JSScriptRef scriptObject = JSScriptCreateFromString(context, url, 1, script, &exception);
+        JSStringRef errorMessage = 0;
+        int errorLine = 0;
+        JSScriptRef scriptObject = JSScriptCreateFromString(contextGroup, url, 1, script, &errorMessage, &errorLine);
         ASSERT((!scriptObject) != (!exception));
         if (exception) {
-            printf("FAIL: Test script did not parse\n");
+            printf("FAIL: Test script did not parse\n\t%s:%d\n\t", scriptPath, errorLine);
+            CFStringRef errorCF = JSStringCopyCFString(kCFAllocatorDefault, errorMessage);
+            CFShow(errorCF);
+            CFRelease(errorCF);
+            JSStringRelease(errorMessage);
             failed = 1;
         }
 
@@ -1650,7 +1657,7 @@ int main(int argc, char* argv[])
             JSStringRelease(exceptionIString);
             failed = 1;
         }
-        JSScriptRelease(context, scriptObject);
+        JSScriptRelease(scriptObject);
         free(scriptUTF8);
     }
 
index d6b832d..651cac2 100644 (file)
@@ -1,3 +1,20 @@
+2012-12-19  Oliver Hunt  <oliver@apple.com>
+
+        Tidy up JSScriptRef API
+        https://bugs.webkit.org/show_bug.cgi?id=105470
+
+        Reviewed by Anders Carlsson.
+
+        People found the API's use of a context confusing, so we'll switch to a JSContextGroup based
+        API, and drop a number of the unnecessary uses of contexts.
+
+        * API/JSScriptRef.cpp:
+        (OpaqueJSScript::globalData):
+        (parseScript):
+        * API/JSScriptRefPrivate.h:
+        * API/tests/testapi.c:
+        (main):
+
 2012-12-19  Alexis Menard  <alexis@webkit.org>
 
         Implement CSS parsing for CSS transitions unprefixed.