Bug 1477765 - Handle rejected Promises when inspecting an object;r=jimb draft
authorBrian Grinstead <bgrinstead@mozilla.com>
Wed, 25 Jul 2018 13:16:03 -0700
changeset 822716 45d84b1c8ef5a6fc2a9c060dba44b2f56147740b
parent 822648 4c4abe35d80851590954b52eabf828c763104696
push id117458
push userbgrinstead@mozilla.com
push dateWed, 25 Jul 2018 20:16:31 +0000
reviewersjimb
bugs1477765
milestone63.0a1
Bug 1477765 - Handle rejected Promises when inspecting an object;r=jimb This changes the behavior for inspecting rejected Promises such that: (1) We don't consider them safe getters, treating them as we would a thrown exception (2) Manually handle the rejection with `catch` when possible, so that we don't create an error when trying to inspect the promise and leaving it unhandled MozReview-Commit-ID: HZL4BrjCKkA
devtools/server/actors/object.js
devtools/shared/webconsole/test/test_object_actor_native_getters_lenient_this.html
--- a/devtools/server/actors/object.js
+++ b/devtools/server/actors/object.js
@@ -375,35 +375,51 @@ const proto = {
           // The above can throw if the cache becomes stale.
         }
         if (!getter) {
           obj._safeGetters = null;
           continue;
         }
 
         const result = getter.call(this.obj);
-        if (result && !("throw" in result)) {
-          let getterValue = undefined;
-          if ("return" in result) {
-            getterValue = result.return;
-          } else if ("yield" in result) {
-            getterValue = result.yield;
+        if (!result || "throw" in result) {
+          continue;
+        }
+
+        let getterValue = undefined;
+        if ("return" in result) {
+          getterValue = result.return;
+        } else if ("yield" in result) {
+          getterValue = result.yield;
+        }
+
+        // Treat an already-rejected Promise as we would a thrown exception
+        // by not including it as a safe getter value (see Bug 1477765).
+        if (getterValue && (getterValue.class == "Promise" &&
+                            getterValue.promiseState == "rejected")) {
+          // Until we have a good way to handle Promise rejections through the
+          // debugger API (Bug 1478076), call `catch` when it's safe to do so.
+          const raw = getterValue.unsafeDereference();
+          if (DevToolsUtils.isSafeJSObject(raw)) {
+            raw.catch(e=>e);
           }
-          // WebIDL attributes specified with the LenientThis extended attribute
-          // return undefined and should be ignored.
-          if (getterValue !== undefined) {
-            safeGetterValues[name] = {
-              getterValue: this.hooks.createValueGrip(getterValue),
-              getterPrototypeLevel: level,
-              enumerable: desc.enumerable,
-              writable: level == 0 ? desc.writable : true,
-            };
-            if (limit && ++i == limit) {
-              break;
-            }
+          continue;
+        }
+
+        // WebIDL attributes specified with the LenientThis extended attribute
+        // return undefined and should be ignored.
+        if (getterValue !== undefined) {
+          safeGetterValues[name] = {
+            getterValue: this.hooks.createValueGrip(getterValue),
+            getterPrototypeLevel: level,
+            enumerable: desc.enumerable,
+            writable: level == 0 ? desc.writable : true,
+          };
+          if (limit && ++i == limit) {
+            break;
           }
         }
       }
       if (limit && i == limit) {
         break;
       }
 
       obj = obj.proto;
--- a/devtools/shared/webconsole/test/test_object_actor_native_getters_lenient_this.html
+++ b/devtools/shared/webconsole/test/test_object_actor_native_getters_lenient_this.html
@@ -61,21 +61,17 @@ function onConsoleCall(aState, aType, aP
 function onProperties(aState, aResponse)
 {
   let props = aResponse.ownProperties;
   let keys = Object.keys(props);
   info(keys.length + " ownProperties: " + keys);
 
   is(keys.length, 0, "number of properties");
   keys = Object.keys(aResponse.safeGetterValues);
-  // There is one "safe getter" as far as the code in _findSafeGetterValues is
-  // concerned, because it treats any Promise-returning attribute as a "safe
-  // getter".  See bug 1438015.
-  is(keys.length, 1, "number of safe getters");
-  is(keys[0], "documentReadyForIdle", "Unexpected safe getter");
+  is(keys.length, 0, "number of safe getters");
 
   closeDebugger(aState, function() {
     SimpleTest.finish();
   });
 }
 
 addEventListener("load", startTest);
 </script>