[gjs] js: Deprecate GJS rooted value arrays

[Philip Chimento <pchimento src gnome org>]

commit bceeb3428fa81e7d2e2df25429203a0254c60265
Author: Philip Chimento <philip chimento gmail com>
Date:   Sun Oct 2 01:11:53 2016 -0700

    js: Deprecate GJS rooted value arrays
    
    There is now JS API for this, JS::AutoValueVector, which achieves the
    same thing more safely through RAII, and in addition avoids the alignment
    problems that might result from casting the char * pointer inside GArray
    to another type.
    
    The GjsRootedArray type was not actually used anymore inside GJS, but
    gjs_root_value_locations() and friends still were, so these are replaced
    with JS::AutoValueVector.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=742249

 gi/function.cpp          |   11 ++++-------
 gi/value.cpp             |   18 +++++-------------
 gjs/jsapi-util-array.cpp |   19 +++++++++++++++++--
 gjs/jsapi-util.h         |   27 ++++++++++++++++++---------
 test/gjs-tests.cpp       |   46 ----------------------------------------------
 5 files changed, 44 insertions(+), 77 deletions(-)
---
diff --git a/gi/function.cpp b/gi/function.cpp
index 123eb55..70c4808 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -704,7 +704,7 @@ gjs_invoke_c_function(JSContext       *context,
     bool is_method;
     GITypeInfo return_info;
     GITypeTag return_tag;
-    JS::Value *return_values = NULL;
+    JS::AutoValueVector return_values(context);
     guint8 next_rval = 0; /* index into return_values */
     GSList *iter;
 
@@ -1009,9 +1009,8 @@ gjs_invoke_c_function(JSContext       *context,
 
     /* Only process return values if the function didn't throw */
     if (function->js_out_argc > 0 && !did_throw_gerror) {
-        return_values = g_newa(JS::Value, function->js_out_argc);
-        gjs_set_values(context, return_values, function->js_out_argc, JS::UndefinedValue());
-        gjs_root_value_locations(context, return_values, function->js_out_argc);
+        for (size_t i = 0; i < function->js_out_argc; i++)
+            return_values.append(JS::UndefinedValue());
 
         if (return_tag != GI_TYPE_TAG_VOID) {
             GITransfer transfer = g_callable_info_get_caller_owns((GICallableInfo*) function->info);
@@ -1268,7 +1267,7 @@ release:
                 JSObject *array;
                 array = JS_NewArrayObject(context,
                                           function->js_out_argc,
-                                          return_values);
+                                          &return_values[0]);
                 if (array == NULL) {
                     failed = true;
                 } else {
@@ -1277,8 +1276,6 @@ release:
             }
         }
 
-        gjs_unroot_value_locations(context, return_values, function->js_out_argc);
-
         if (r_value) {
             *r_value = return_gargument;
         }
diff --git a/gi/value.cpp b/gi/value.cpp
index fc8e50d..1c5cc34 100644
--- a/gi/value.cpp
+++ b/gi/value.cpp
@@ -122,7 +122,6 @@ closure_marshal(GClosure        *closure,
     JSRuntime *runtime;
     JSObject *obj;
     int argc;
-    JS::Value *argv;
     JS::Value rval;
     int i;
     GSignalQuery signal_query = { 0, };
@@ -173,15 +172,10 @@ closure_marshal(GClosure        *closure,
 
     argc = n_param_values;
     rval = JS::UndefinedValue();
-    if (argc > 0) {
-        argv = g_newa(JS::Value, n_param_values);
+    JS::AutoValueVector argv(context);
+    for (i = 0; i < argc; i++)
+        argv.append(JS::UndefinedValue());
 
-        gjs_set_values(context, argv, argc, JS::UndefinedValue());
-        gjs_root_value_locations(context, argv, argc);
-    } else {
-        /* squash a compiler warning */
-        argv = NULL;
-    }
     JS_AddValueRoot(context, &rval);
 
     if (marshal_data) {
@@ -284,7 +278,7 @@ closure_marshal(GClosure        *closure,
         if (type_info_for[i])
             g_base_info_unref((GIBaseInfo *)type_info_for[i]);
 
-    gjs_closure_invoke(closure, argv_index, argv, &rval);
+    gjs_closure_invoke(closure, argv_index, &argv[0], &rval);
 
     if (return_value != NULL) {
         if (rval.isUndefined()) {
@@ -301,8 +295,6 @@ closure_marshal(GClosure        *closure,
     }
 
  cleanup:
-    if (argc > 0)
-        gjs_unroot_value_locations(context, argv, argc);
     JS_RemoveValueRoot(context, &rval);
     JS_EndRequest(context);
 }
diff --git a/gjs/jsapi-util-array.cpp b/gjs/jsapi-util-array.cpp
index d3e5787..4bae378 100644
--- a/gjs/jsapi-util-array.cpp
+++ b/gjs/jsapi-util-array.cpp
@@ -39,6 +39,8 @@
  * Creates an opaque data type that holds JS::Values and keeps
  * their location (NOT their value) GC-rooted.
  *
+ * Deprecated: Use JS::AutoValueVector instead.
+ *
  * Returns: an opaque object prepared to hold GC root locations.
  **/
 GjsRootedArray*
@@ -84,6 +86,7 @@ remove_root_jsval(JSContext *context,
  * Appends @value to @array, calling JS_AddValueRoot on the location where it's
  * stored.
  *
+ * Deprecated: Use JS::AutoValueVector instead.
  **/
 void
 gjs_rooted_array_append(JSContext        *context,
@@ -112,6 +115,9 @@ gjs_rooted_array_append(JSContext        *context,
  * @context: a #JSContext
  * @array: an array
  * @i: element to return
+ *
+ * Deprecated: Use JS::AutoValueVector instead.
+ *
  * Returns: value of an element
  */
 JS::Value
@@ -136,9 +142,11 @@ gjs_rooted_array_get(JSContext        *context,
 
 /**
  * gjs_rooted_array_get_data:
- *
  * @context: a #JSContext
  * @array: an array
+ *
+ * Deprecated: Use JS::AutoValueVector instead.
+ *
  * Returns: the rooted JS::Value locations in the array
  */
 JS::Value *
@@ -157,9 +165,11 @@ gjs_rooted_array_get_data(JSContext      *context,
 
 /**
  * gjs_rooted_array_get_length:
- *
  * @context: a #JSContext
  * @array: an array
+ *
+ * Deprecated: Use JS::AutoValueVector instead.
+ *
  * Returns: number of JS::Value in the rooted array
  */
 int
@@ -185,6 +195,7 @@ gjs_rooted_array_get_length (JSContext        *context,
  *
  * Calls JS_AddValueRoot() on each address in @locations.
  *
+ * Deprecated: Use JS::AutoValueArray or JS::AutoValueVector instead.
  **/
 void
 gjs_root_value_locations(JSContext        *context,
@@ -213,6 +224,7 @@ gjs_root_value_locations(JSContext        *context,
  *
  * Calls JS_RemoveValueRoot() on each address in @locations.
  *
+ * Deprecated: Use JS::AutoValueArray or JS::AutoValueVector instead.
  **/
 void
 gjs_unroot_value_locations(JSContext *context,
@@ -241,6 +253,7 @@ gjs_unroot_value_locations(JSContext *context,
  *
  * Assigns initializer to each member of the given array.
  *
+ * Deprecated: Use JS::AutoValueArray or JS::AutoValueVector instead.
  **/
 void
 gjs_set_values(JSContext        *context,
@@ -269,6 +282,8 @@ gjs_set_values(JSContext        *context,
  * true the internal memory block allocated for the JS::Value array will
  * be freed and unrooted also.
  *
+ * Deprecated: Use JS::AutoValueVector instead.
+ *
  * Returns: the JS::Value array if it was not freed
  **/
 JS::Value *
diff --git a/gjs/jsapi-util.h b/gjs/jsapi-util.h
index 5b6b3ef..811074f 100644
--- a/gjs/jsapi-util.h
+++ b/gjs/jsapi-util.h
@@ -422,30 +422,39 @@ bool        gjs_parse_call_args              (JSContext    *context,
                                               JS::CallArgs &args,
                                               ...);
 
-GjsRootedArray*   gjs_rooted_array_new        (void);
+GjsRootedArray*   gjs_rooted_array_new        (void)
+G_GNUC_DEPRECATED_FOR(JS::AutoValueVector);
 void              gjs_rooted_array_append     (JSContext        *context,
                                                GjsRootedArray *array,
-                                               JS::Value       value);
+                                               JS::Value       value)
+G_GNUC_DEPRECATED_FOR(JS::AutoValueVector::append);
 JS::Value         gjs_rooted_array_get        (JSContext      *context,
                                                GjsRootedArray *array,
-                                               int               i);
+                                               int             i)
+G_GNUC_DEPRECATED_FOR(JS::AutoValueVector::operator[]);
 JS::Value        *gjs_rooted_array_get_data   (JSContext        *context,
-                                               GjsRootedArray *array);
+                                               GjsRootedArray *array)
+G_GNUC_DEPRECATED_FOR(JS::AutoValueVector::operator[]);
 int               gjs_rooted_array_get_length (JSContext        *context,
-                                               GjsRootedArray *array);
+                                               GjsRootedArray *array)
+G_GNUC_DEPRECATED_FOR(JS::AutoValueVector::length);
 JS::Value        *gjs_rooted_array_free       (JSContext        *context,
                                                GjsRootedArray *array,
-                                               bool              free_segment);
+                                               bool              free_segment)
+G_GNUC_DEPRECATED_FOR(JS::AutoValueVector);
 void              gjs_set_values              (JSContext        *context,
                                                JS::Value        *locations,
                                                int               n_locations,
-                                               JS::Value         initializer);
+                                               JS::Value         initializer)
+G_GNUC_DEPRECATED_FOR(JS::AutoValueVector);
 void              gjs_root_value_locations    (JSContext        *context,
                                                JS::Value        *locations,
-                                               int               n_locations);
+                                               int               n_locations)
+G_GNUC_DEPRECATED_FOR(JS::AutoValueVector);
 void              gjs_unroot_value_locations  (JSContext        *context,
                                                JS::Value        *locations,
-                                               int               n_locations);
+                                               int               n_locations)
+G_GNUC_DEPRECATED_FOR(JS::AutoValueVector);
 
 /* Functions intended for more "internal" use */
 
diff --git a/test/gjs-tests.cpp b/test/gjs-tests.cpp
index 40e0ec5..0a17c8d 100644
--- a/test/gjs-tests.cpp
+++ b/test/gjs-tests.cpp
@@ -89,51 +89,6 @@ gjstest_test_func_gjs_context_construct_eval(void)
     g_object_unref (context);
 }
 
-#define N_ELEMS 15
-
-static void
-gjstest_test_func_gjs_jsapi_util_array(void)
-{
-    GjsUnitTestFixture fixture;
-    JSContext *context;
-    JSObject *global;
-    GjsRootedArray *array;
-    int i;
-    JS::Value value;
-
-    _gjs_unit_test_fixture_begin(&fixture);
-    context = fixture.context;
-
-    array = gjs_rooted_array_new();
-
-    global = gjs_get_global_object(context);
-    JSCompartment *oldCompartment = JS_EnterCompartment(context, global);
-
-    for (i = 0; i < N_ELEMS; i++) {
-        value = JS::StringValue(JS_NewStringCopyZ(context, "abcdefghijk"));
-        gjs_rooted_array_append(context, array, value);
-    }
-
-    JS_GC(JS_GetRuntime(context));
-
-    for (i = 0; i < N_ELEMS; i++) {
-        char *ascii;
-
-        value = gjs_rooted_array_get(context, array, i);
-        g_assert(value.isString());
-        gjs_string_to_utf8(context, value, &ascii);
-        g_assert(strcmp(ascii, "abcdefghijk") == 0);
-        g_free(ascii);
-    }
-
-    gjs_rooted_array_free(context, array, true);
-
-    JS_LeaveCompartment(context, oldCompartment);
-    _gjs_unit_test_fixture_finish(&fixture);
-}
-
-#undef N_ELEMS
-
 static void
 gjstest_test_func_gjs_jsapi_util_string_js_string_utf8(void)
 {
@@ -323,7 +278,6 @@ main(int    argc,
 
     g_test_add_func("/gjs/context/construct/destroy", gjstest_test_func_gjs_context_construct_destroy);
     g_test_add_func("/gjs/context/construct/eval", gjstest_test_func_gjs_context_construct_eval);
-    g_test_add_func("/gjs/jsapi/util/array", gjstest_test_func_gjs_jsapi_util_array);
     g_test_add_func("/gjs/jsapi/util/error/throw", gjstest_test_func_gjs_jsapi_util_error_throw);
     g_test_add_func("/gjs/jsapi/util/string/js/string/utf8", 
gjstest_test_func_gjs_jsapi_util_string_js_string_utf8);
     g_test_add_func("/gjs/jsutil/strip_shebang/no_shebang", 
gjstest_test_strip_shebang_no_advance_for_no_shebang);