Revert "[DevTools] Perform Session id surgery in Blink."

This reverts commit b0caf8a2.

Reason for revert: broke some protocol clients.

Original change's description:
> [DevTools] Perform Session id surgery in Blink.
> 
> That is, for messages coming from Blink, perform the surgery there.
> For messages that come from the browser, perform the surgery in
> content::DevToolsSession as before.
> 
> Longer term, we may include the session id prior to serializing the
> message. But this requires much deeper changes to the bindings etc.,
> so this just moves the computation closer to where it will be
> eventually.
> 
> Change-Id: I16efafec72e8077d7aaab6458f5253ddb018fd96
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1734142
> Commit-Queue: Johannes Henkel <johannes@chromium.org>
> Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#690168}

TBR=dgozman@chromium.org,dcheng@chromium.org,caseq@chromium.org,johannes@chromium.org

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

Change-Id: I89d2e6a11fd38a0ab0484e26af793ebd230692a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1771549Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Commit-Queue: Andrey Kosyakov <caseq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#690408}

content/browser/devtools/devtools_session.cc

@@ -101,6 +101,7 @@ void DevToolsSession::AttachToAgent(blink::mojom::DevToolsAgent* agent) {
     io_session_.reset();
     return;
   }
+
   // TODO(https://crbug.com/978694): Consider a reset flow since new mojo types
   // checks is_bound strictly.
   if (receiver_.is_bound()) {
@@ -108,11 +109,12 @@ void DevToolsSession::AttachToAgent(blink::mojom::DevToolsAgent* agent) {
     session_.reset();
     io_session_.reset();
   }
-  agent->AttachDevToolsSession(
-      receiver_.BindNewEndpointAndPassRemote(),
-      session_.BindNewEndpointAndPassReceiver(),
-      io_session_.BindNewPipeAndPassReceiver(), session_state_cookie_.Clone(),
-      client_->UsesBinaryProtocol(), child_session_id_);
+
+  agent->AttachDevToolsSession(receiver_.BindNewEndpointAndPassRemote(),
+                               session_.BindNewEndpointAndPassReceiver(),
+                               io_session_.BindNewPipeAndPassReceiver(),
+                               session_state_cookie_.Clone(),
+                               client_->UsesBinaryProtocol());
   session_.set_disconnect_handler(base::BindOnce(
       &DevToolsSession::MojoConnectionDestroyed, base::Unretained(this)));
 
@@ -279,18 +281,10 @@ void DevToolsSession::ResumeSendingMessagesToAgent() {
 // the browser to the client.
 static void SendProtocolResponseOrNotification(
     DevToolsAgentHostClient* client,
-    const std::string& child_session_id,
     DevToolsAgentHostImpl* agent_host,
     std::unique_ptr<protocol::Serializable> message) {
   std::string cbor = message->serialize(/*binary=*/true);
   DCHECK(IsCBORMessage(SpanFrom(cbor)));
-  if (!child_session_id.empty()) {
-    IPEStatus status = AppendString8EntryToCBORMap(
-        SpanFrom(kSessionId), SpanFrom(child_session_id), &cbor);
-    DCHECK(status.ok()) << status.ToASCIIString();
-    if (!status.ok())
-      return;
-  }
   if (client->UsesBinaryProtocol()) {
     client->DispatchProtocolMessage(agent_host, cbor);
     return;
@@ -304,15 +298,13 @@ static void SendProtocolResponseOrNotification(
 void DevToolsSession::sendProtocolResponse(
     int call_id,
     std::unique_ptr<protocol::Serializable> message) {
-  SendProtocolResponseOrNotification(client_, child_session_id_, agent_host_,
-                                     std::move(message));
+  SendProtocolResponseOrNotification(client_, agent_host_, std::move(message));
   // |this| may be deleted at this point.
 }
 
 void DevToolsSession::sendProtocolNotification(
     std::unique_ptr<protocol::Serializable> message) {
-  SendProtocolResponseOrNotification(client_, child_session_id_, agent_host_,
-                                     std::move(message));
+  SendProtocolResponseOrNotification(client_, agent_host_, std::move(message));
   // |this| may be deleted at this point.
 }
 
@@ -400,7 +392,6 @@ DevToolsSession* DevToolsSession::AttachChildSession(
   DCHECK(!root_session_);
   auto session = std::make_unique<DevToolsSession>(client);
   session->root_session_ = this;
-  session->child_session_id_ = session_id;
   DevToolsSession* session_ptr = session.get();
   // If attach did not succeed, |session| is already destroyed.
   if (!agent_host->AttachInternal(std::move(session)))
@@ -418,12 +409,18 @@ void DevToolsSession::SendMessageFromChildSession(const std::string& session_id,
   if (child_sessions_.find(session_id) == child_sessions_.end())
     return;
   DCHECK(IsCBORMessage(SpanFrom(message)));
+  std::string patched(message);
+  IPEStatus status = AppendString8EntryToCBORMap(
+      SpanFrom(kSessionId), SpanFrom(session_id), &patched);
+  LOG_IF(ERROR, !status.ok()) << status.ToASCIIString();
+  if (!status.ok())
+    return;
   if (client_->UsesBinaryProtocol()) {
-    client_->DispatchProtocolMessage(agent_host_, message);
+    client_->DispatchProtocolMessage(agent_host_, patched);
     return;
   }
   std::string json;
-  IPEStatus status = ConvertCBORToJSON(SpanFrom(message), &json);
+  status = ConvertCBORToJSON(SpanFrom(patched), &json);
   LOG_IF(ERROR, !status.ok()) << status.ToASCIIString();
   client_->DispatchProtocolMessage(agent_host_, json);
   // |this| may be deleted at this point.

content/browser/devtools/devtools_session.h

@@ -137,10 +137,6 @@ class DevToolsSession : public protocol::FrontendChannel,
   blink::mojom::DevToolsSessionStatePtr session_state_cookie_;
 
   DevToolsSession* root_session_ = nullptr;
-  // If this session is a child session (root_session_ != nullptr), and we're in
-  // flatten_protocol mode (see protocol/target_handler.cc), then and only then,
-  // |child_session_id_| will contain a non-empty session id.
-  std::string child_session_id_;
   base::flat_map<std::string, DevToolsSession*> child_sessions_;
   base::OnceClosure runtime_resume_;
   DevToolsExternalAgentProxyDelegate* proxy_delegate_ = nullptr;

third_party/blink/public/mojom/devtools/devtools_agent.mojom

@@ -72,17 +72,11 @@ interface DevToolsAgent {
   // |client_expects_binary_response| indicates that responses (and
   // notifications) sent from this session should use binary (CBOR)
   // encoding as opposed to JSON encoding.
-  //
-  // |session_id| is a string which, if provided, uniquely identifies the
-  // session. The renderer must send this back with each response. The current
-  // implementation uses the string serialization of an UnguessableToken, but
-  // that is subject to change.
   AttachDevToolsSession(pending_associated_remote<DevToolsSessionHost> host,
                         pending_associated_receiver<DevToolsSession> session,
                         pending_receiver<DevToolsSession> io_session,
                         DevToolsSessionState? reattach_session_state,
-                        bool client_expects_binary_responses,
-                        string sesson_id);
+                        bool client_expects_binary_responses);
 
   // Requests an element at specific position to be inspected in every
   // attached session (or the next attached one if none yet).

third_party/blink/renderer/core/inspector/devtools_agent.cc

@@ -106,13 +106,12 @@ void DevToolsAgent::AttachDevToolsSession(
         session_receiver,
     mojo::PendingReceiver<mojom::blink::DevToolsSession> io_session_receiver,
     mojom::blink::DevToolsSessionStatePtr reattach_session_state,
-    bool client_expects_binary_responses,
-    const WTF::String& session_id) {
+    bool client_expects_binary_responses) {
   client_->DebuggerTaskStarted();
   DevToolsSession* session = MakeGarbageCollected<DevToolsSession>(
       this, std::move(host), std::move(session_receiver),
       std::move(io_session_receiver), std::move(reattach_session_state),
-      client_expects_binary_responses, session_id);
+      client_expects_binary_responses);
   sessions_.insert(session);
   client_->DebuggerTaskFinished();
 }

third_party/blink/renderer/core/inspector/devtools_agent.h

@@ -79,8 +79,7 @@ class CORE_EXPORT DevToolsAgent
           main_session,
       mojo::PendingReceiver<mojom::blink::DevToolsSession> io_session,
       mojom::blink::DevToolsSessionStatePtr reattach_session_state,
-      bool client_expects_binary_responses,
-      const WTF::String& session_id) override;
+      bool client_expects_binary_responses) override;
   void InspectElement(const WebPoint& point) override;
   void ReportChildWorkers(bool report,
                           bool wait_for_debugger,

third_party/blink/renderer/core/inspector/devtools_session.cc

@@ -28,14 +28,11 @@ namespace blink {
 namespace {
 using ::inspector_protocol_encoding::span;
 using ::inspector_protocol_encoding::SpanFrom;
-using ::inspector_protocol_encoding::cbor::AppendString8EntryToCBORMap;
 using ::inspector_protocol_encoding::cbor::IsCBORMessage;
 using ::inspector_protocol_encoding::json::ConvertCBORToJSON;
 using IPEStatus = ::inspector_protocol_encoding::Status;
 
 const char kV8StateKey[] = "v8";
-static const char kSessionId[] = "sessionId";
-
 bool ShouldInterruptForMethod(const String& method) {
   // Keep in sync with DevToolsSession::ShouldSendOnIO.
   // TODO(dgozman): find a way to share this.
@@ -55,6 +52,20 @@ Vector<uint8_t> UnwrapMessage(const mojom::blink::DevToolsMessagePtr& message) {
   return unwrap_message;
 }
 
+mojom::blink::DevToolsMessagePtr WrapMessage(
+    protocol::ProtocolMessage message) {
+  auto result = mojom::blink::DevToolsMessage::New();
+
+  if (message.json.IsEmpty()) {
+    result->data = message.binary.ReleaseVector();
+  } else {
+    WTF::StringUTF8Adaptor adaptor(message.json);
+    result->data =
+        mojo_base::BigBuffer(base::as_bytes(base::make_span(adaptor)));
+  }
+  return result;
+}
+
 protocol::ProtocolMessage ToProtocolMessage(
     std::unique_ptr<v8_inspector::StringBuffer> buffer) {
   protocol::ProtocolMessage message;
@@ -148,14 +159,12 @@ DevToolsSession::DevToolsSession(
     mojom::blink::DevToolsSessionAssociatedRequest main_request,
     mojom::blink::DevToolsSessionRequest io_request,
     mojom::blink::DevToolsSessionStatePtr reattach_session_state,
-    bool client_expects_binary_responses,
-    const WTF::String& session_id)
+    bool client_expects_binary_responses)
     : agent_(agent),
       binding_(this, std::move(main_request)),
       inspector_backend_dispatcher_(new protocol::UberDispatcher(this)),
       session_state_(std::move(reattach_session_state)),
       client_expects_binary_responses_(client_expects_binary_responses),
-      session_id_(session_id),
       v8_session_state_(kV8StateKey),
       v8_session_state_cbor_(&v8_session_state_,
                              /*default_value=*/{}) {
@@ -314,7 +323,16 @@ void DevToolsSession::SendProtocolResponse(
   // protocol response in any of them.
   if (WebTestSupport::IsRunningWebTest())
     agent_->FlushProtocolNotifications();
-  host_ptr_->DispatchProtocolResponse(FinalizeMessage(message), call_id,
+
+  mojom::blink::DevToolsMessagePtr serialized = WrapMessage(message);
+  if (!client_expects_binary_responses_) {
+    std::vector<uint8_t> json;
+    IPEStatus status = ConvertCBORToJSON(
+        span<uint8_t>(serialized->data.data(), serialized->data.size()), &json);
+    CHECK(status.ok()) << status.ToASCIIString();
+    serialized->data = mojo_base::BigBuffer(json);
+  }
+  host_ptr_->DispatchProtocolResponse(std::move(serialized), call_id,
                                       session_state_.TakeUpdates());
 }
 
@@ -339,16 +357,16 @@ class DevToolsSession::Notification {
       std::unique_ptr<v8_inspector::StringBuffer> notification)
       : v8_notification_(std::move(notification)) {}
 
-  protocol::ProtocolMessage Serialize() {
-    protocol::ProtocolMessage result;
+  mojom::blink::DevToolsMessagePtr Serialize() {
+    protocol::ProtocolMessage serialized;
     if (blink_notification_) {
-      result = blink_notification_->serialize(/*binary=*/true);
+      serialized = blink_notification_->serialize(/*binary=*/true);
       blink_notification_.reset();
     } else if (v8_notification_) {
-      result = ToProtocolMessage(std::move(v8_notification_));
+      serialized = ToProtocolMessage(std::move(v8_notification_));
       v8_notification_.reset();
     }
-    return result;
+    return WrapMessage(std::move(serialized));
   }
 
  private:
@@ -382,9 +400,18 @@ void DevToolsSession::flushProtocolNotifications() {
   if (v8_session_)
     v8_session_state_cbor_.Set(v8_session_->state());
   for (wtf_size_t i = 0; i < notification_queue_.size(); ++i) {
-    host_ptr_->DispatchProtocolNotification(
-        FinalizeMessage(notification_queue_[i]->Serialize()),
-        session_state_.TakeUpdates());
+    mojom::blink::DevToolsMessagePtr serialized =
+        notification_queue_[i]->Serialize();
+    if (!client_expects_binary_responses_) {
+      std::vector<uint8_t> json;
+      IPEStatus status = ConvertCBORToJSON(
+          span<uint8_t>(serialized->data.data(), serialized->data.size()),
+          &json);
+      CHECK(status.ok()) << status.ToASCIIString();
+      serialized->data = mojo_base::BigBuffer(json);
+    }
+    host_ptr_->DispatchProtocolNotification(std::move(serialized),
+                                            session_state_.TakeUpdates());
   }
   notification_queue_.clear();
 }
@@ -394,23 +421,4 @@ void DevToolsSession::Trace(blink::Visitor* visitor) {
   visitor->Trace(agents_);
 }
 
-blink::mojom::blink::DevToolsMessagePtr DevToolsSession::FinalizeMessage(
-    protocol::ProtocolMessage message) {
-  std::vector<uint8_t> message_to_send = message.binary.ReleaseVector();
-  if (!session_id_.IsEmpty()) {
-    IPEStatus status = AppendString8EntryToCBORMap(
-        SpanFrom(kSessionId), SpanFrom(session_id_.Ascii()), &message_to_send);
-    CHECK(status.ok()) << status.ToASCIIString();
-  }
-  if (!client_expects_binary_responses_) {
-    std::vector<uint8_t> json;
-    IPEStatus status = ConvertCBORToJSON(SpanFrom(message_to_send), &json);
-    CHECK(status.ok()) << status.ToASCIIString();
-    message_to_send = std::move(json);
-  }
-  auto mojo_msg = mojom::blink::DevToolsMessage::New();
-  mojo_msg->data = std::move(message_to_send);
-  return mojo_msg;
-}
-
 }  // namespace blink

third_party/blink/renderer/core/inspector/devtools_session.h

@@ -36,8 +36,7 @@ class CORE_EXPORT DevToolsSession
       mojom::blink::DevToolsSessionAssociatedRequest main_request,
       mojom::blink::DevToolsSessionRequest io_request,
       mojom::blink::DevToolsSessionStatePtr reattach_session_state,
-      bool client_expects_binary_responses,
-      const String& session_id);
+      bool client_expects_binary_responses);
   ~DevToolsSession() override;
 
   void ConnectToV8(v8_inspector::V8Inspector*, int context_group_id);
@@ -87,11 +86,6 @@ class CORE_EXPORT DevToolsSession
   void SendProtocolResponse(int call_id,
                             const protocol::ProtocolMessage& message);
 
-  // Inserts the session id (if non-empty) and converts to JSON
-  // if requested by the client.
-  blink::mojom::blink::DevToolsMessagePtr FinalizeMessage(
-      protocol::ProtocolMessage message);
-
   Member<DevToolsAgent> agent_;
   mojo::AssociatedBinding<mojom::blink::DevToolsSession> binding_;
   mojom::blink::DevToolsSessionHostAssociatedPtr host_ptr_;
@@ -103,7 +97,6 @@ class CORE_EXPORT DevToolsSession
   class Notification;
   Vector<std::unique_ptr<Notification>> notification_queue_;
   const bool client_expects_binary_responses_;
-  const String session_id_;
   InspectorAgentState v8_session_state_;
   InspectorAgentState::Bytes v8_session_state_cbor_;