Support Runtime.getProperties#accessorPropertiesOnly

Summary:
Chrome DevTools requests object properties as a [combination of two `getProperties` calls](https://github.com/facebookexperimental/rn-chrome-devtools-frontend/blob/35aa264a622e853bb28b4c83a7b5cc3b2a9747bc/front_end/core/sdk/RemoteObject.ts#L170-L173):

1. `accessorPropertiesOnly: true, ownProperties: false`
2. `accessorPropertiesOnly: false, ownProperties: true`

Hermes currently doesn't support the [`accessorPropertiesOnly`](https://cdpstatus.reactnative.dev/devtools-protocol/tot/Runtime#:~:text=accessorPropertiesOnly) parameter and therefore erroneously includes all data properties in the first request. Here, we add support for `accessorPropertiesOnly`.

Reviewed By: mattbfb

Differential Revision: D57614120

fbshipit-source-id: 3412a1c9e1464f454b1c13596a0953aa853fb057

API/hermes/cdp/MessageTypes.cpp

@@ -1,5 +1,5 @@
 // Copyright (c) Meta Platforms, Inc. and affiliates. All Rights Reserved.
-// @generated SignedSource<<8577199f24d290b5b77422ed340660ad>>
+// @generated SignedSource<<de6ca843395a9e62745ae61459f2fe53>>
 
 #include "MessageTypes.h"
 
@@ -1803,15 +1803,17 @@ runtime::GetPropertiesRequest::tryMake(const JSONObject *obj) {
   auto *params = *convertResult;
   TRY_ASSIGN(req->objectId, params, "objectId");
   TRY_ASSIGN(req->ownProperties, params, "ownProperties");
+  TRY_ASSIGN(req->accessorPropertiesOnly, params, "accessorPropertiesOnly");
   TRY_ASSIGN(req->generatePreview, params, "generatePreview");
   return req;
 }
 
 JSONValue *runtime::GetPropertiesRequest::toJsonVal(
     JSONFactory &factory) const {
-  llvh::SmallVector<JSONFactory::Prop, 3> paramsProps;
+  llvh::SmallVector<JSONFactory::Prop, 4> paramsProps;
   put(paramsProps, "objectId", objectId, factory);
   put(paramsProps, "ownProperties", ownProperties, factory);
+  put(paramsProps, "accessorPropertiesOnly", accessorPropertiesOnly, factory);
   put(paramsProps, "generatePreview", generatePreview, factory);
 
   llvh::SmallVector<JSONFactory::Prop, 1> props;

API/hermes/cdp/MessageTypes.h

@@ -1,5 +1,5 @@
 // Copyright (c) Meta Platforms, Inc. and affiliates. All Rights Reserved.
-// @generated SignedSource<<443255d7dd433634cec8a92c7d4f1d4c>>
+// @generated SignedSource<<2d6fd9e085abc8347743709393aff722>>
 
 #pragma once
 
@@ -907,6 +907,7 @@ struct runtime::GetPropertiesRequest : public Request {
 
   runtime::RemoteObjectId objectId{};
   std::optional<bool> ownProperties;
+  std::optional<bool> accessorPropertiesOnly;
   std::optional<bool> generatePreview;
 };
 

API/hermes/cdp/RuntimeDomainAgent.cpp

@@ -549,6 +549,7 @@ void RuntimeDomainAgent::getProperties(
   ObjectSerializationOptions serializationOptions;
   serializationOptions.generatePreview = req.generatePreview.value_or(false);
   bool ownProperties = req.ownProperties.value_or(false);
+  bool accessorPropertiesOnly = req.accessorPropertiesOnly.value_or(false);
 
   std::string objGroup = objTable_->getObjectGroup(req.objectId);
   auto scopePtr = objTable_->getScope(req.objectId);
@@ -580,11 +581,17 @@ void RuntimeDomainAgent::getProperties(
 
     } else if (valuePtr != nullptr) {
       resp.result = makePropsFromValue(
-          *valuePtr, objGroup, ownProperties, serializationOptions);
-      auto internalProps =
-          makeInternalPropsFromValue(*valuePtr, objGroup, serializationOptions);
-      if (internalProps.size()) {
-        resp.internalProperties = std::move(internalProps);
+          *valuePtr,
+          objGroup,
+          ownProperties,
+          accessorPropertiesOnly,
+          serializationOptions);
+      if (!accessorPropertiesOnly) {
+        auto internalProps = makeInternalPropsFromValue(
+            *valuePtr, objGroup, serializationOptions);
+        if (internalProps.size()) {
+          resp.internalProperties = std::move(internalProps);
+        }
       }
     }
   } catch (const jsi::JSError &error) {
@@ -766,6 +773,7 @@ RuntimeDomainAgent::makePropsFromValue(
     const jsi::Value &value,
     const std::string &objectGroup,
     bool onlyOwnProperties,
+    bool accessorPropertiesOnly,
     const ObjectSerializationOptions &serializationOptions) {
   std::vector<m::runtime::PropertyDescriptor> result;
 
@@ -795,6 +803,14 @@ RuntimeDomainAgent::makePropsFromValue(
           m::runtime::PropertyDescriptor desc;
           desc.isOwn = isOwn;
           jsi::Value propNameOrSymbol = propArray.getValueAtIndex(runtime, i);
+          jsi::Object descriptor = helpers_.objectGetOwnPropertyDescriptor
+                                       .call(runtime, obj, propNameOrSymbol)
+                                       .asObject(runtime);
+          if (accessorPropertiesOnly &&
+              !descriptor.hasProperty(runtime, "get") &&
+              !descriptor.hasProperty(runtime, "set")) {
+            continue;
+          }
           if (propNameOrSymbol.isString()) {
             auto propName = propNameOrSymbol.getString(runtime);
             if (
@@ -831,9 +847,6 @@ RuntimeDomainAgent::makePropsFromValue(
             assert(false && "unexpected non-string non-symbol property key");
           }
           try {
-            jsi::Object descriptor = helpers_.objectGetOwnPropertyDescriptor
-                                         .call(runtime, obj, propNameOrSymbol)
-                                         .asObject(runtime);
             desc.enumerable =
                 descriptor.getProperty(runtime, "enumerable").asBool();
             desc.configurable =

API/hermes/cdp/RuntimeDomainAgent.h

@@ -100,6 +100,7 @@ class RuntimeDomainAgent : public DomainAgent {
       const jsi::Value &value,
       const std::string &objectGroup,
       bool onlyOwnProperties,
+      bool accessorPropertiesOnly,
       const ObjectSerializationOptions &serializationOptions);
   std::vector<m::runtime::InternalPropertyDescriptor>
   makeInternalPropsFromValue(

API/hermes/cdp/tools/run_msggen

@@ -19,7 +19,7 @@ yarn build
 
 node bin/index.js \
   --ignore-experimental \
-  --include-experimental=Runtime.getProperties.generatePreview,Runtime.evaluate.generatePreview,Runtime.callFunctionOn.generatePreview,Debugger.evaluateOnCallFrame.generatePreview,Runtime.RemoteObject.preview,Runtime.RemoteObject.customPreview,Runtime.CustomPreview,Runtime.EntryPreview,Runtime.ObjectPreview,Runtime.PropertyPreview,Runtime.getHeapUsage \
+  --include-experimental=Runtime.getProperties.generatePreview,Runtime.getProperties.accessorPropertiesOnly,Runtime.evaluate.generatePreview,Runtime.callFunctionOn.generatePreview,Debugger.evaluateOnCallFrame.generatePreview,Runtime.RemoteObject.preview,Runtime.RemoteObject.customPreview,Runtime.CustomPreview,Runtime.EntryPreview,Runtime.ObjectPreview,Runtime.PropertyPreview,Runtime.getHeapUsage \
   --roots "${MSGTYPES_PATH}" \
   "${HEADER_PATH}" "${CPP_PATH}"
 

unittests/API/CDPAgentTest.cpp

@@ -194,7 +194,8 @@ class CDPAgentTest : public ::testing::Test {
       const std::string &objectId,
       const std::unordered_map<std::string, PropInfo> &infos,
       const std::unordered_map<std::string, PropInfo> &internalInfos = {},
-      bool ownProperties = true);
+      bool ownProperties = true,
+      bool accessorPropertiesOnly = false);
 
   std::unique_ptr<HermesRuntime> runtime_;
   std::unique_ptr<CDPDebugAPI> cdpDebugAPI_;
@@ -508,14 +509,13 @@ m::runtime::GetPropertiesResponse CDPAgentTest::getAndEnsureProps(
     const std::string &objectId,
     const std::unordered_map<std::string, PropInfo> &infos,
     const std::unordered_map<std::string, PropInfo> &internalInfos,
-    bool ownProperties) {
-  sendRequest(
-      "Runtime.getProperties",
-      msgId,
-      [objectId, ownProperties](::hermes::JSONEmitter &json) {
-        json.emitKeyValue("objectId", objectId);
-        json.emitKeyValue("ownProperties", ownProperties);
-      });
+    bool ownProperties,
+    bool accessorPropertiesOnly) {
+  sendRequest("Runtime.getProperties", msgId, [&](::hermes::JSONEmitter &json) {
+    json.emitKeyValue("objectId", objectId);
+    json.emitKeyValue("ownProperties", ownProperties);
+    json.emitKeyValue("accessorPropertiesOnly", accessorPropertiesOnly);
+  });
   return ensureProps(waitForMessage(), infos, internalInfos);
 }
 
@@ -2418,6 +2418,16 @@ TEST_F(CDPAgentTest, RuntimeGetPropertiesExtendedDescriptors) {
   const auto &obj = scopeChildren.at("obj");
   std::string objId = obj.value.value().objectId.value();
 
+  getAndEnsureProps(
+      msgId++,
+      objId,
+      {{"accessor", PropInfo().setConfigurable(true).setEnumerable(false)},
+       {"throwingAccessor",
+        PropInfo().setConfigurable(false).setEnumerable(false)}},
+      {},
+      /* ownProperties */ true,
+      /* accessorPropertiesOnly */ true);
+
   auto objPropsResp = getAndEnsureProps(
       msgId++,
       objId,
@@ -2430,7 +2440,9 @@ TEST_F(CDPAgentTest, RuntimeGetPropertiesExtendedDescriptors) {
        {"accessor", PropInfo().setConfigurable(true).setEnumerable(false)},
        {"throwingAccessor",
         PropInfo().setConfigurable(false).setEnumerable(false)}},
-      {});
+      {},
+      /* ownProperties */ true,
+      /* accessorPropertiesOnly */ false);
 
   /// Helper that invokes a getter function on the specified object.
   auto invokeGetter = [&](std::string objId,