js: setProperty operations with triple result state

Property setting callbacks now get a JS::ObjectOpResult on which they
have to call the succeed() method or one if the fail() methods. This is
required in order to implement an awkward part of the ES6 spec where it
is possible for a property setting operation to fail despite not throwing
an exception. This replaces what used to be "strict mode" and
JSStrictPropertyOp.

2 files changed, 36 insertions, 34 deletions

diff --git a/gi/object.cpp b/gi/object.cpp
index f69735ef..82663927 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -426,11 +426,12 @@ object_instance_get_prop(JSContext              *context,
 }
 
 static bool
-set_g_param_from_prop(JSContext      *context,
-                      ObjectInstance *priv,
-                      const char     *name,
-                      bool&           was_set,
-                      JS::HandleValue value_p)
+set_g_param_from_prop(JSContext          *context,
+                      ObjectInstance     *priv,
+                      const char         *name,
+                      bool&               was_set,
+                      JS::HandleValue     value_p,
+                      JS::ObjectOpResult& result)
 {
     GParameter param = { NULL, { 0, }};
     was_set = false;
@@ -443,7 +444,7 @@ set_g_param_from_prop(JSContext      *context,
     case SOME_ERROR_OCCURRED:
         return false;
     case NO_SUCH_G_PROPERTY:
-        return true;
+        return result.succeed();
     case VALUE_WAS_SET:
     default:
         break;
@@ -454,22 +455,22 @@ set_g_param_from_prop(JSContext      *context,
 
     g_value_unset(&param.value);
     was_set = true;
-    return true;
+    return result.succeed();
 }
 
 static bool
 check_set_field_from_prop(JSContext             *cx,
                           ObjectInstance        *priv,
                           const char            *name,
-                          bool                   strict,
-                          JS::MutableHandleValue value_p)
+                          JS::MutableHandleValue value_p,
+                          JS::ObjectOpResult&    result)
 {
     if (priv->info == NULL)
-        return true;
+        return result.succeed();
 
     GIFieldInfo *field = lookup_field_info(priv->info, name);
     if (field == NULL)
-        return true;
+        return result.succeed();
 
     bool retval = true;
 
@@ -478,21 +479,17 @@ check_set_field_from_prop(JSContext             *cx,
     if (g_field_info_get_flags(field) & GI_FIELD_IS_WRITABLE) {
         g_message("Field %s of a GObject is writable, but setting it is not "
                   "implemented", name);
+        result.succeed();
         goto out;
     }
 
-    if (strict) {
-        gjs_throw(cx, "Tried to set read-only field %s in strict mode", name);
-        retval = false;
-        goto out;
-    }
+    result.failReadOnly();  /* still return true; error only in strict mode */
 
     /* We have to update value_p because JS caches it as the property's "stored
      * value" (https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/JSAPI_reference/Stored_value)
      * and so subsequent gets would get the stored value instead of accessing
      * the field */
     value_p.setUndefined();
-
 out:
     g_base_info_unref((GIBaseInfo *) field);
     return retval;
@@ -505,8 +502,8 @@ static bool
 object_instance_set_prop(JSContext              *context,
                          JS::HandleObject        obj,
                          JS::HandleId            id,
-                         bool                    strict,
-                         JS::MutableHandleValue  value_p)
+                         JS::MutableHandleValue  value_p,
+                         JS::ObjectOpResult&     result)
 {
     ObjectInstance *priv;
     char *name;
@@ -514,7 +511,7 @@ object_instance_set_prop(JSContext              *context,
     bool g_param_was_set = false;
 
     if (!gjs_get_string_id(context, id, &name))
-        return true; /* not resolved, but no error */
+        return result.succeed();  /* not resolved, but no error */
 
     priv = priv_from_js(context, obj);
     gjs_debug_jsprop(GJS_DEBUG_GOBJECT,
@@ -523,16 +520,20 @@ object_instance_set_prop(JSContext              *context,
 
     if (priv == NULL) {
         /* see the comment in object_instance_get_prop() on this */
+        result.succeed();
         goto out;
     }
-    if (priv->gobj == NULL) /* prototype, not an instance. */
+    if (priv->gobj == NULL) {
+        /* prototype, not an instance. */
+        result.succeed();
         goto out;
+    }
 
-    ret = set_g_param_from_prop(context, priv, name, g_param_was_set, value_p);
+    ret = set_g_param_from_prop(context, priv, name, g_param_was_set, value_p, result);
     if (g_param_was_set || !ret)
         goto out;
 
-    ret = check_set_field_from_prop(context, priv, name, strict, value_p);
+    ret = check_set_field_from_prop(context, priv, name, value_p, result);
 
     /* note that the prop will also have been set in JS, which I think
      * is OK, since we hook get and set so will always override that
diff --git a/gjs/byteArray.cpp b/gjs/byteArray.cpp
index 2f2baf97..b0130edb 100644
--- a/gjs/byteArray.cpp
+++ b/gjs/byteArray.cpp
@@ -47,8 +47,8 @@ static bool   byte_array_get_prop      (JSContext    *context,
 static bool   byte_array_set_prop      (JSContext    *context,
                                         JS::HandleObject obj,
                                         JS::HandleId id,
-                                        bool                   strict,
-                                        JS::MutableHandleValue value_p);
+                                        JS::MutableHandleValue value_p,
+                                        JS::ObjectOpResult&    result);
 GJS_NATIVE_CONSTRUCTOR_DECLARE(byte_array);
 static void   byte_array_finalize      (JSFreeOp     *fop,
                                         JSObject     *obj);
@@ -61,8 +61,8 @@ struct JSClass gjs_byte_array_class = {
     JSCLASS_BACKGROUND_FINALIZE,
     NULL,  /* addProperty */
     NULL,  /* deleteProperty */
-    (JSPropertyOp)byte_array_get_prop,
-    (JSStrictPropertyOp)byte_array_set_prop,
+    byte_array_get_prop,
+    byte_array_set_prop,
     NULL,  /* enumerate */
     NULL,  /* resolve */
     NULL,  /* convert */
@@ -267,7 +267,8 @@ byte_array_set_index(JSContext         *context,
                      JS::HandleObject obj,
                      ByteArrayInstance *priv,
                      gsize              idx,
-                     JS::MutableHandleValue value_p)
+                     JS::MutableHandleValue value_p,
+                     JS::ObjectOpResult&    result)
 {
     guint8 v;
 
@@ -288,7 +289,7 @@ byte_array_set_index(JSContext         *context,
     /* Stop JS from storing a copy of the value */
     value_p.setUndefined();
 
-    return true;
+    return result.succeed();
 }
 
 /* a hook on setting a property; set value_p to override property value to
@@ -298,15 +299,15 @@ static bool
 byte_array_set_prop(JSContext *context,
                     JS::HandleObject obj,
                     JS::HandleId id,
-                    bool strict,
-                    JS::MutableHandleValue value_p)
+                    JS::MutableHandleValue value_p,
+                    JS::ObjectOpResult&    result)
 {
     ByteArrayInstance *priv;
 
     priv = priv_from_js(context, obj);
 
     if (priv == NULL)
-        return true; /* prototype, not an instance. */
+        return result.succeed(); /* prototype, not an instance. */
 
     JS::RootedValue id_value(context);
     if (!JS_IdToValue(context, id, &id_value))
@@ -318,12 +319,12 @@ byte_array_set_prop(JSContext *context,
         if (!gjs_value_to_gsize(context, id_value, &idx))
             return false;
 
-        return byte_array_set_index(context, obj, priv, idx, value_p);
+        return byte_array_set_index(context, obj, priv, idx, value_p, result);
     }
 
     /* We don't special-case anything else for now */
 
-    return true;
+    return result.succeed();
 }
 
 static GByteArray *