Revert "Reland "[ VM / Service ] Omit private fields from service responses by default""

This reverts commit 91a496e5db.

Reason for revert: breaks hot reloads internally

Original change's description:
> Reland "[ VM / Service ] Omit private fields from service responses by default"
>
> This reverts commit 7d39d2dd51.
>
> TEST=N/A
>
> Change-Id: I2119c841719c77be5380857ce209532ed036bd0e
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/226322
> Reviewed-by: Siva Annamalai <asiva@google.com>
> Commit-Queue: Ben Konyi <bkonyi@google.com>

# Not skipping CQ checks because original CL landed > 1 day ago.

Change-Id: I6e751343244a4788a1f080ea1aef5fdd18417109
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/227503
Reviewed-by: Alexander Thomas <athom@google.com>
Commit-Queue: Ivan Inozemtsev <iinozemtsev@google.com>
This commit is contained in:
Ivan Inozemtsev 2022-01-11 14:06:04 +00:00 committed by Commit Bot
parent 99a902ff36
commit 28e958febb
11 changed files with 21 additions and 170 deletions

View file

@ -97,13 +97,7 @@ class DartDevelopmentServiceClient {
void _registerJsonRpcMethods() {
_clientPeer.registerMethod('streamListen', (parameters) async {
final streamId = parameters['streamId'].asString;
final includePrivates =
parameters['_includePrivateMembers'].asBoolOr(false);
await dds.streamManager.streamListen(
this,
streamId,
includePrivates: includePrivates,
);
await dds.streamManager.streamListen(this, streamId);
return RPCResponses.success;
});

View file

@ -170,19 +170,16 @@ class StreamManager {
/// `streamListen` request for `stream` to the VM service.
Future<void> streamListen(
DartDevelopmentServiceClient? client,
String stream, {
bool? includePrivates,
}) async {
String stream,
) async {
await _streamSubscriptionMutex.runGuarded(
() async {
assert(stream.isNotEmpty);
bool streamNewlySubscribed = false;
if (!streamListeners.containsKey(stream)) {
// Initialize the list of clients for the new stream before we do
// anything else to ensure multiple clients registering for the same
// stream in quick succession doesn't result in multiple streamListen
// requests being sent to the VM service.
streamNewlySubscribed = true;
streamListeners[stream] = <DartDevelopmentServiceClient>[];
if ((stream == kDebugStream && client == null) ||
stream != kDebugStream) {
@ -191,18 +188,12 @@ class StreamManager {
final result =
await dds.vmServiceClient.sendRequest('streamListen', {
'streamId': stream,
if (includePrivates != null)
'_includePrivateMembers': includePrivates,
});
assert(result['type'] == 'Success');
}
}
if (streamListeners[stream]!.contains(client)) {
throw kStreamAlreadySubscribedException;
} else if (!streamNewlySubscribed && includePrivates != null) {
await dds.vmServiceClient.sendRequest(
'_setStreamIncludePrivateMembers',
{'streamId': stream, 'includePrivateMembers': includePrivates});
}
if (client != null) {
streamListeners[stream]!.add(client);

View file

@ -128,13 +128,10 @@ DEFINE_NATIVE_ENTRY(VMService_OnServerAddressChange, 0, 1) {
return Object::null();
}
DEFINE_NATIVE_ENTRY(VMService_ListenStream, 0, 2) {
DEFINE_NATIVE_ENTRY(VMService_ListenStream, 0, 1) {
#ifndef PRODUCT
GET_NON_NULL_NATIVE_ARGUMENT(String, stream_id, arguments->NativeArgAt(0));
GET_NON_NULL_NATIVE_ARGUMENT(Bool, include_privates,
arguments->NativeArgAt(1));
bool result =
Service::ListenStream(stream_id.ToCString(), include_privates.value());
bool result = Service::ListenStream(stream_id.ToCString());
return Bool::Get(result).ptr();
#else
return Object::null();

View file

@ -149,11 +149,7 @@ abstract class CommonWebSocketVM extends VM {
return new Future.error(exception);
}
String serial = (_requestSerial++).toString();
var request = new _WebSocketRequest(method, <String, dynamic>{
...params,
// Include internal response data.
'_includePrivateMembers': true,
});
var request = new _WebSocketRequest(method, params);
if ((_webSocket != null) && _webSocket!.isOpen) {
// Already connected, send request immediately.
_sendRequest(serial, request);

View file

@ -149,11 +149,7 @@ abstract class CommonWebSocketVM extends VM {
return new Future.error(exception);
}
String serial = (_requestSerial++).toString();
var request = new _WebSocketRequest(method, <String, dynamic>{
...params,
// Include internal response data.
'_includePrivateMembers': true,
});
var request = new _WebSocketRequest(method, params);
if ((_webSocket != null) && _webSocket.isOpen) {
// Already connected, send request immediately.
_sendRequest(serial, request);

View file

@ -361,7 +361,7 @@ namespace dart {
V(VMService_OnStart, 0) \
V(VMService_OnExit, 0) \
V(VMService_OnServerAddressChange, 1) \
V(VMService_ListenStream, 2) \
V(VMService_ListenStream, 1) \
V(VMService_CancelStream, 1) \
V(VMService_RequestAssets, 0) \
V(VMService_DecodeAssets, 1) \

View file

@ -37,9 +37,7 @@ JSONStream::JSONStream(intptr_t buf_size)
param_values_(NULL),
num_params_(0),
offset_(0),
count_(-1),
include_private_members_(true),
ignore_object_depth_(0) {
count_(-1) {
ObjectIdRing* ring = NULL;
Isolate* isolate = Isolate::Current();
if (isolate != NULL) {
@ -92,10 +90,6 @@ void JSONStream::Setup(Zone* zone,
"request %s\n",
Dart::UptimeMillis(), main_port, isolate_name, method_);
}
const char* kIncludePrivateMembersKey = "_includePrivateMembers";
if (HasParam(kIncludePrivateMembersKey)) {
include_private_members_ = ParamIs(kIncludePrivateMembersKey, "true");
}
buffer()->Printf("{\"jsonrpc\":\"2.0\", \"result\":");
}
@ -374,61 +368,49 @@ void JSONStream::PrintServiceId(const Object& o) {
PrintProperty("id", id_zone_->GetServiceId(o));
}
#define PRIVATE_NAME_CHECK() \
if (!IsAllowableKey(name) || ignore_object_depth_ > 0) return
void JSONStream::PrintProperty(const char* name, const ServiceEvent* event) {
PRIVATE_NAME_CHECK();
PrintPropertyName(name);
PrintValue(event);
}
void JSONStream::PrintProperty(const char* name, Breakpoint* bpt) {
PRIVATE_NAME_CHECK();
PrintPropertyName(name);
PrintValue(bpt);
}
void JSONStream::PrintProperty(const char* name, TokenPosition tp) {
PRIVATE_NAME_CHECK();
PrintPropertyName(name);
PrintValue(tp);
}
void JSONStream::PrintProperty(const char* name, Metric* metric) {
PRIVATE_NAME_CHECK();
PrintPropertyName(name);
PrintValue(metric);
}
void JSONStream::PrintProperty(const char* name, MessageQueue* queue) {
PRIVATE_NAME_CHECK();
PrintPropertyName(name);
PrintValue(queue);
}
void JSONStream::PrintProperty(const char* name, Isolate* isolate) {
PRIVATE_NAME_CHECK();
PrintPropertyName(name);
PrintValue(isolate);
}
void JSONStream::PrintProperty(const char* name,
const TimelineEvent* timeline_event) {
PRIVATE_NAME_CHECK();
PrintPropertyName(name);
PrintValue(timeline_event);
}
void JSONStream::PrintProperty(const char* name,
const TimelineEventBlock* timeline_event_block) {
PRIVATE_NAME_CHECK();
PrintPropertyName(name);
PrintValue(timeline_event_block);
}
void JSONStream::PrintfProperty(const char* name, const char* format, ...) {
PRIVATE_NAME_CHECK();
va_list args;
va_start(args, format);
writer_.VPrintfProperty(name, format, args);
@ -480,13 +462,11 @@ void JSONStream::SetParams(const char** param_keys,
}
void JSONStream::PrintProperty(const char* name, const Object& o, bool ref) {
PRIVATE_NAME_CHECK();
PrintPropertyName(name);
PrintValue(o, ref);
}
void JSONStream::PrintPropertyVM(const char* name, bool ref) {
PRIVATE_NAME_CHECK();
PrintPropertyName(name);
PrintValueVM(ref);
}

View file

@ -9,7 +9,6 @@
#include "platform/allocation.h"
#include "platform/text_buffer.h"
#include "vm/json_writer.h"
#include "vm/os.h"
#include "vm/service.h"
#include "vm/token_position.h"
@ -103,18 +102,6 @@ class JSONStream : ValueObject {
void set_reply_port(Dart_Port port);
bool include_private_members() const { return include_private_members_; }
void set_include_private_members(bool include_private_members) {
include_private_members_ = include_private_members;
}
bool IsAllowableKey(const char* key) {
if (include_private_members_) {
return true;
}
return *key != '_';
}
void SetParams(const char** param_keys,
const char** param_values,
intptr_t num_params);
@ -180,41 +167,15 @@ class JSONStream : ValueObject {
void PostNullReply(Dart_Port port);
void OpenObject(const char* property_name = NULL) {
if (ignore_object_depth_ > 0 ||
(property_name != nullptr && !IsAllowableKey(property_name))) {
ignore_object_depth_++;
return;
}
writer_.OpenObject(property_name);
}
void CloseObject() {
if (ignore_object_depth_ > 0) {
ignore_object_depth_--;
return;
}
writer_.CloseObject();
}
void UncloseObject() {
// This should be updated to handle unclosing a private object if we need
// to handle that case, which we don't currently.
writer_.UncloseObject();
}
void CloseObject() { writer_.CloseObject(); }
void UncloseObject() { writer_.UncloseObject(); }
void OpenArray(const char* property_name = NULL) {
if (ignore_object_depth_ > 0 ||
(property_name != nullptr && !IsAllowableKey(property_name))) {
ignore_object_depth_++;
return;
}
writer_.OpenArray(property_name);
}
void CloseArray() {
if (ignore_object_depth_ > 0) {
ignore_object_depth_--;
return;
}
writer_.CloseArray();
}
void CloseArray() { writer_.CloseArray(); }
void PrintValueNull() { writer_.PrintValueNull(); }
void PrintValueBool(bool b) { writer_.PrintValueBool(b); }
@ -250,67 +211,46 @@ class JSONStream : ValueObject {
void PrintServiceId(const Object& o);
#define PRIVATE_NAME_CHECK() \
if (!IsAllowableKey(name) || ignore_object_depth_ > 0) { \
return; \
}
void PrintPropertyBool(const char* name, bool b) {
PRIVATE_NAME_CHECK();
writer_.PrintPropertyBool(name, b);
}
void PrintProperty(const char* name, intptr_t i) {
PRIVATE_NAME_CHECK();
writer_.PrintProperty(name, i);
}
void PrintProperty64(const char* name, int64_t i) {
PRIVATE_NAME_CHECK();
writer_.PrintProperty64(name, i);
}
void PrintPropertyTimeMillis(const char* name, int64_t millis) {
PRIVATE_NAME_CHECK();
writer_.PrintProperty64(name, millis);
}
void PrintPropertyTimeMicros(const char* name, int64_t micros) {
PRIVATE_NAME_CHECK();
writer_.PrintProperty64(name, micros);
}
void PrintProperty(const char* name, double d) {
PRIVATE_NAME_CHECK();
writer_.PrintProperty(name, d);
}
void PrintPropertyBase64(const char* name,
const uint8_t* bytes,
intptr_t length) {
PRIVATE_NAME_CHECK();
writer_.PrintPropertyBase64(name, bytes, length);
}
void PrintProperty(const char* name, const char* s) {
PRIVATE_NAME_CHECK();
writer_.PrintProperty(name, s);
}
bool PrintPropertyStr(const char* name,
const String& s,
intptr_t offset,
intptr_t count) {
if (!IsAllowableKey(name)) {
return false;
}
return writer_.PrintPropertyStr(name, s, offset, count);
}
void PrintPropertyNoEscape(const char* name, const char* s) {
PRIVATE_NAME_CHECK();
writer_.PrintPropertyNoEscape(name, s);
}
void PrintfProperty(const char* name, const char* format, ...)
PRINTF_ATTRIBUTE(3, 4);
void VPrintfProperty(const char* name, const char* format, va_list args) {
PRIVATE_NAME_CHECK();
writer_.VPrintfProperty(name, format, args);
}
#undef PRIVATE_NAME_CHECK
void PrintProperty(const char* name, const Object& o, bool ref = true);
void PrintProperty(const char* name, const ServiceEvent* event);
@ -345,8 +285,7 @@ class JSONStream : ValueObject {
intptr_t offset_;
intptr_t count_;
int64_t setup_time_micros_;
bool include_private_members_;
intptr_t ignore_object_depth_;
friend class JSONObject;
friend class JSONArray;
friend class TimelineEvent;

View file

@ -417,8 +417,7 @@ static StreamInfo* const streams_[] = {
&Service::timeline_stream, &Service::profiler_stream,
};
bool Service::ListenStream(const char* stream_id,
bool include_private_members) {
bool Service::ListenStream(const char* stream_id) {
if (FLAG_trace_service) {
OS::PrintErr("vm-service: starting stream '%s'\n", stream_id);
}
@ -426,7 +425,6 @@ bool Service::ListenStream(const char* stream_id,
for (intptr_t i = 0; i < num_streams; i++) {
if (strcmp(stream_id, streams_[i]->id()) == 0) {
streams_[i]->set_enabled(true);
streams_[i]->set_include_private_members(include_private_members);
return true;
}
}
@ -1217,7 +1215,7 @@ static void ReportPauseOnConsole(ServiceEvent* event) {
}
void Service::HandleEvent(ServiceEvent* event, bool enter_safepoint) {
if (event->stream_info() != nullptr && !event->stream_info()->enabled()) {
if (event->stream_info() != NULL && !event->stream_info()->enabled()) {
if (FLAG_warn_on_pause_with_no_debugger && event->IsPause()) {
// If we are about to pause a running program which has no
// debugger connected, tell the user about it.
@ -1225,7 +1223,7 @@ void Service::HandleEvent(ServiceEvent* event, bool enter_safepoint) {
}
// Ignore events when no one is listening to the event stream.
return;
} else if (event->stream_info() != nullptr &&
} else if (event->stream_info() != NULL &&
FLAG_warn_on_pause_with_no_debugger && event->IsPause()) {
ReportPauseOnConsole(event);
}
@ -1233,12 +1231,8 @@ void Service::HandleEvent(ServiceEvent* event, bool enter_safepoint) {
return;
}
JSONStream js;
if (event->stream_info() != nullptr) {
js.set_include_private_members(
event->stream_info()->include_private_members());
}
const char* stream_id = event->stream_id();
ASSERT(stream_id != nullptr);
ASSERT(stream_id != NULL);
{
JSONObject jsobj(&js);
jsobj.AddProperty("jsonrpc", "2.0");
@ -1554,31 +1548,6 @@ static void PrintSentinel(JSONStream* js, SentinelType sentinel_type) {
}
}
static const MethodParameter* const
set_stream_include_private_members_params[] = {
NO_ISOLATE_PARAMETER,
new BoolParameter("includePrivateMembers", true),
nullptr,
};
static void SetStreamIncludePrivateMembers(Thread* thread, JSONStream* js) {
const char* stream_id = js->LookupParam("streamId");
if (stream_id == nullptr) {
PrintMissingParamError(js, "streamId");
return;
}
bool include_private_members =
BoolParameter::Parse(js->LookupParam("includePrivateMembers"), false);
intptr_t num_streams = sizeof(streams_) / sizeof(streams_[0]);
for (intptr_t i = 0; i < num_streams; i++) {
if (strcmp(stream_id, streams_[i]->id()) == 0) {
streams_[i]->set_include_private_members(include_private_members);
break;
}
}
PrintSuccess(js);
}
static void ActOnIsolateGroup(JSONStream* js,
std::function<void(IsolateGroup*)> visitor) {
const String& prefix =
@ -5710,8 +5679,6 @@ static const ServiceMethodDescriptor service_methods_[] = {
set_library_debuggable_params },
{ "setName", SetName,
set_name_params },
{ "_setStreamIncludePrivateMembers", SetStreamIncludePrivateMembers,
set_stream_include_private_members_params },
{ "setTraceClassAllocation", SetTraceClassAllocation,
set_trace_class_allocation_params },
{ "setVMName", SetVMName,

View file

@ -68,23 +68,16 @@ class RingServiceIdZone : public ServiceIdZone {
class StreamInfo {
public:
explicit StreamInfo(const char* id)
: id_(id), enabled_(false), include_private_members_(false) {}
explicit StreamInfo(const char* id) : id_(id), enabled_(false) {}
const char* id() const { return id_; }
void set_enabled(bool value) { enabled_ = value; }
bool enabled() const { return enabled_; }
void set_include_private_members(bool value) {
include_private_members_ = value;
}
bool include_private_members() const { return include_private_members_; }
private:
const char* id_;
bool enabled_;
bool include_private_members_;
};
class Service : public AllStatic {
@ -181,7 +174,7 @@ class Service : public AllStatic {
static StreamInfo timeline_stream;
static StreamInfo profiler_stream;
static bool ListenStream(const char* stream_id, bool include_privates);
static bool ListenStream(const char* stream_id);
static void CancelStream(const char* stream_id);
static ObjectPtr RequestAssets();

View file

@ -549,9 +549,7 @@ class VMService extends MessageRouter {
return encodeRpcError(message, kStreamAlreadySubscribed);
}
if (!_isAnyClientSubscribed(streamId)) {
final includePrivates = message.params['_includePrivateMembers'] == true;
if (!serviceStreams.contains(streamId) &&
!_vmListenStream(streamId, includePrivates)) {
if (!serviceStreams.contains(streamId) && !_vmListenStream(streamId)) {
return encodeRpcError(message, kInvalidParams,
details: "streamListen: invalid 'streamId' parameter: ${streamId}");
}
@ -833,7 +831,7 @@ external void _onServerAddressChange(String? address);
/// Subscribe to a service stream.
@pragma("vm:external-name", "VMService_ListenStream")
external bool _vmListenStream(String streamId, bool include_privates);
external bool _vmListenStream(String streamId);
/// Cancel a subscription to a service stream.
@pragma("vm:external-name", "VMService_CancelStream")