[gjs: 2/3] byteArray: Add compatibility toString property

[Philip Chimento <pchimento src gnome org>]

commit 8f0dff8df5c9c18e84c34ab544be1ee4f47eb3f2
Author: Philip Chimento <philip chimento gmail com>
Date:   Sat Aug 18 20:59:47 2018 -0400

    byteArray: Add compatibility toString property
    
    This overrides, on each Uint8Array returned from an introspected function
    or from ByteArray.fromString() or ByteArray.fromGBytes(), the toString()
    property with a compatibility shim that preserves the old behaviour of
    ByteArray.prototype.toString() while logging a compatibility warning
    asking people to fix their code.
    
    This ByteArray.toString() -> Uint8Array.toString() change has had more
    fallout in application code than I expected, so it seems better to
    preserve backwards compatibility. (The old behaviour was to decode the
    byte array into a string with default encoding UTF-8, and the default
    behaviour of Uint8Array is to return a string with the decimal digits of
    each byte joined with commas. So the effect was that strings like
    "97,98,99,100" would show up in UIs where previously "abcd" would have
    been printed.
    
    This is only on specific instances, so Uint8Array.prototype.toString()
    remains untouched.

 gjs/byteArray.cpp                   | 56 ++++++++++++++++++++++++++-----------
 gjs/deprecation.cpp                 |  9 ++++++
 gjs/deprecation.h                   |  1 +
 installed-tests/js/testByteArray.js | 24 ++++++++++++++++
 4 files changed, 74 insertions(+), 16 deletions(-)
---
diff --git a/gjs/byteArray.cpp b/gjs/byteArray.cpp
index 96368482..5dacfe48 100644
--- a/gjs/byteArray.cpp
+++ b/gjs/byteArray.cpp
@@ -25,6 +25,7 @@
 
 #include "byteArray.h"
 #include "gi/boxed.h"
+#include "gjs/deprecation.h"
 #include "jsapi-util-args.h"
 #include "jsapi-wrapper.h"
 
@@ -45,22 +46,11 @@ static void bytes_unref_arraybuffer(void* contents, void* user_data) {
 }
 
 /* implement toString() with an optional encoding arg */
-static bool
-to_string_func(JSContext *context,
-               unsigned   argc,
-               JS::Value *vp)
-{
-    JS::CallArgs argv = JS::CallArgsFromVp(argc, vp);
-    GjsAutoJSChar encoding;
-    JS::RootedObject byte_array(context);
+static bool to_string_impl(JSContext* context, JS::HandleObject byte_array,
+                           const char* encoding, JS::MutableHandleValue rval) {
     bool encoding_is_utf8;
     uint8_t* data;
 
-    if (!gjs_parse_call_args(context, "toString", argv, "o|s",
-                             "byteArray", &byte_array,
-                             "encoding", &encoding))
-        return false;
-
     if (encoding) {
         /* maybe we should be smarter about utf8 synonyms here.
          * doesn't matter much though. encoding_is_utf8 is
@@ -80,7 +70,7 @@ to_string_func(JSContext *context,
          * libmozjs hardwired utf8-to-utf16
          */
         return gjs_string_from_utf8_n(context, reinterpret_cast<char*>(data),
-                                      len, argv.rval());
+                                      len, rval);
     } else {
         bool ok = false;
         gsize bytes_written;
@@ -109,7 +99,7 @@ to_string_func(JSContext *context,
         s = JS_NewUCStringCopyN(context, u16_out, bytes_written / 2);
         if (s != NULL) {
             ok = true;
-            argv.rval().setString(s);
+            rval.setString(s);
         }
 
         g_free(u16_str);
@@ -118,6 +108,35 @@ to_string_func(JSContext *context,
     }
 }
 
+static bool to_string_func(JSContext* cx, unsigned argc, JS::Value* vp) {
+    JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
+    GjsAutoJSChar encoding;
+    JS::RootedObject byte_array(cx);
+
+    if (!gjs_parse_call_args(cx, "toString", args, "o|s", "byteArray",
+                             &byte_array, "encoding", &encoding))
+        return false;
+
+    return to_string_impl(cx, byte_array, encoding, args.rval());
+}
+
+/* Workaround to keep existing code compatible. This function is tacked onto
+ * any Uint8Array instances created in situations where previously a ByteArray
+ * would have been created. It logs a compatibility warning. */
+static bool instance_to_string_func(JSContext* cx, unsigned argc,
+                                    JS::Value* vp) {
+    GJS_GET_THIS(cx, argc, vp, args, this_obj);
+    GjsAutoJSChar encoding;
+
+    _gjs_warn_deprecated_once_per_callsite(
+        cx, GjsDeprecationMessageId::ByteArrayInstanceToString);
+
+    if (!gjs_parse_call_args(cx, "toString", args, "|s", "encoding", &encoding))
+        return false;
+
+    return to_string_impl(cx, this_obj, encoding, args.rval());
+}
+
 static bool
 to_gbytes_func(JSContext *context,
                unsigned   argc,
@@ -227,6 +246,7 @@ from_string_func(JSContext *context,
     if (!array_buffer)
         return false;
     obj = JS_NewUint8ArrayWithBuffer(context, array_buffer, 0, -1);
+    JS_DefineFunction(context, obj, "toString", instance_to_string_func, 1, 0);
     argv.rval().setObject(*obj);
     return true;
 }
@@ -264,6 +284,7 @@ from_gbytes_func(JSContext *context,
         context, JS_NewUint8ArrayWithBuffer(context, array_buffer, 0, -1));
     if (!obj)
         return false;
+    JS_DefineFunction(context, obj, "toString", instance_to_string_func, 1, 0);
 
     argv.rval().setObject(*obj);
     return true;
@@ -275,7 +296,10 @@ JSObject* gjs_byte_array_from_data(JSContext* cx, size_t nbytes, void* data) {
     if (!array_buffer)
         return nullptr;
 
-    return JS_NewUint8ArrayWithBuffer(cx, array_buffer, 0, -1);
+    JS::RootedObject array(cx,
+                           JS_NewUint8ArrayWithBuffer(cx, array_buffer, 0, -1));
+    JS_DefineFunction(cx, array, "toString", instance_to_string_func, 1, 0);
+    return array;
 }
 
 JSObject* gjs_byte_array_from_byte_array(JSContext* cx, GByteArray* array) {
diff --git a/gjs/deprecation.cpp b/gjs/deprecation.cpp
index 9faf0c02..de3c0080 100644
--- a/gjs/deprecation.cpp
+++ b/gjs/deprecation.cpp
@@ -32,6 +32,15 @@
 const char* messages[] = {
     // None:
     "(invalid message)",
+
+    // ByteArrayInstanceToString:
+    "Some code called array.toString() on a Uint8Array instance. Previously "
+    "this would have interpreted the bytes of the array as a string, but that "
+    "is nonstandard. In the future this will return the bytes as "
+    "comma-separated digits. For the time being, the old behavior has been "
+    "preserved, but please fix your code anyway to explicitly call ByteArray"
+    ".toString(array).\n"
+    "(Note that array.toString() may have been called implicitly.)",
 };
 
 struct DeprecationEntry {
diff --git a/gjs/deprecation.h b/gjs/deprecation.h
index 7353388f..8ed98e3d 100644
--- a/gjs/deprecation.h
+++ b/gjs/deprecation.h
@@ -28,6 +28,7 @@
 
 enum GjsDeprecationMessageId {
     None,
+    ByteArrayInstanceToString,
 };
 
 void _gjs_warn_deprecated_once_per_callsite(JSContext* cx,
diff --git a/installed-tests/js/testByteArray.js b/installed-tests/js/testByteArray.js
index 2bdcbf1b..ec502ed6 100644
--- a/installed-tests/js/testByteArray.js
+++ b/installed-tests/js/testByteArray.js
@@ -1,4 +1,5 @@
 const ByteArray = imports.byteArray;
+const {GIMarshallingTests, GLib} = imports.gi;
 
 describe('Byte array', function () {
     it('can be created from a string', function () {
@@ -35,4 +36,27 @@ describe('Byte array', function () {
         expect(s.length).toEqual(4);
         expect(s).toEqual('abcd');
     });
+
+    describe('legacy toString() behavior', function () {
+        beforeEach(function () {
+            GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_WARNING,
+                'Some code called array.toString()*');
+        });
+
+        it('is preserved when created from a string', function () {
+            let a = ByteArray.fromString('⅜');
+            expect(a.toString()).toEqual('⅜');
+        });
+
+        it('is preserved when marshalled from GI', function () {
+            let a = GIMarshallingTests.bytearray_full_return();
+            expect(() => a.toString()).toThrowError(TypeError,
+                /malformed UTF-8 character sequence/);
+        });
+
+        afterEach(function () {
+            GLib.test_assert_expected_messages_internal('Gjs',
+                'testByteArray.js', 0, 'testToStringCompatibility');
+        });
+    });
 });