Remove the old state cookie.

This removes the string parameters from the interfaces.
It also removes the json dictionaries that were representing
  the state.
v8 session continues to be encoded as JSON
  (see inspector_session.{h,cc}).

For reference, the cl that introduced the new way to manage
the agent state was this one (and advantage is that the updates
sent back to the browser process are deltas, not the entire
state):
https://chromium-review.googlesource.com/c/chromium/src/+/1149201
This change can get rid of the old way since all agents have
been migrated to the new way.

Bug: 851762
Change-Id: Ie245c1c073eb006d4db19d819c2882e6a36f5b2b
Reviewed-on: https://chromium-review.googlesource.com/1162931Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Johannes Henkel <johannes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581103}

content/browser/devtools/devtools_session.cc

@@ -81,10 +81,9 @@ void DevToolsSession::AttachToAgent(
     const blink::mojom::DevToolsAgentAssociatedPtr& agent) {
   blink::mojom::DevToolsSessionHostAssociatedPtrInfo host_ptr_info;
   binding_.Bind(mojo::MakeRequest(&host_ptr_info));
-  agent->AttachDevToolsSession(std::move(host_ptr_info),
-                               mojo::MakeRequest(&session_ptr_),
-                               mojo::MakeRequest(&io_session_ptr_),
-                               state_cookie_, session_state_cookie_.Clone());
+  agent->AttachDevToolsSession(
+      std::move(host_ptr_info), mojo::MakeRequest(&session_ptr_),
+      mojo::MakeRequest(&io_session_ptr_), session_state_cookie_.Clone());
   session_ptr_.set_connection_error_handler(base::BindOnce(
       &DevToolsSession::MojoConnectionDestroyed, base::Unretained(this)));
 
@@ -103,9 +102,7 @@ void DevToolsSession::AttachToAgent(
     waiting_for_response_messages_.clear();
   }
 
-  // Set cookie to an empty string to reattach next time instead of attaching.
-  if (!state_cookie_.has_value())
-    state_cookie_ = std::string();
+  // Set cookie to an empty struct to reattach next time instead of attaching.
   if (!session_state_cookie_)
     session_state_cookie_ = blink::mojom::DevToolsSessionState::New();
 }
@@ -221,10 +218,7 @@ void DevToolsSession::flushProtocolNotifications() {
 void DevToolsSession::DispatchProtocolResponse(
     const std::string& message,
     int call_id,
-    const base::Optional<std::string>& state,
     blink::mojom::DevToolsSessionStatePtr updates) {
-  if (state.has_value())
-    state_cookie_ = state.value();
   ApplySessionStateUpdates(std::move(updates));
   waiting_for_response_messages_.erase(call_id);
   client_->DispatchProtocolMessage(agent_host_, message);
@@ -233,10 +227,7 @@ void DevToolsSession::DispatchProtocolResponse(
 
 void DevToolsSession::DispatchProtocolNotification(
     const std::string& message,
-    const base::Optional<std::string>& state,
     blink::mojom::DevToolsSessionStatePtr updates) {
-  if (state.has_value())
-    state_cookie_ = state.value();
   ApplySessionStateUpdates(std::move(updates));
   client_->DispatchProtocolMessage(agent_host_, message);
   // |this| may be deleted at this point.

content/browser/devtools/devtools_session.h

@@ -88,11 +88,9 @@ class DevToolsSession : public protocol::FrontendChannel,
   void DispatchProtocolResponse(
       const std::string& message,
       int call_id,
-      const base::Optional<std::string>& state,
       blink::mojom::DevToolsSessionStatePtr updates) override;
   void DispatchProtocolNotification(
       const std::string& message,
-      const base::Optional<std::string>& state,
       blink::mojom::DevToolsSessionStatePtr updates) override;
 
   // Merges the |updates| received from the renderer into session_state_cookie_.
@@ -128,10 +126,9 @@ class DevToolsSession : public protocol::FrontendChannel,
   };
   std::map<int, WaitingMessage> waiting_for_response_messages_;
 
-  // |state_cookie_| always corresponds to a state before
+  // |session_state_cookie_| always corresponds to a state before
   // any of the waiting for response messages have been handled.
-  // Note that |state_cookie_| is not present only before first attach.
-  base::Optional<std::string> state_cookie_;
+  // |session_state_cookie_| is nullptr before first attach.
   blink::mojom::DevToolsSessionStatePtr session_state_cookie_;
 
   base::WeakPtrFactory<DevToolsSession> weak_factory_;

third_party/blink/public/web/devtools_agent.mojom

@@ -38,8 +38,8 @@ interface DevToolsAgent {
   // e.g. requesting a pause. There is no ordering guarantee relative to
   // regular |session|.
   //
-  // If |reattach_state| is present, restores the state of the session to
-  // previously saved one (see DevToolsSessionHost). This is useful when
+  // If |reattach_session_state| is present, restores the state of the session
+  // to previously saved one (see DevToolsSessionHost). This is useful when
   // transferring a session from one agent to another while preserving the
   // state. For example, cross-process navigation in a frame creates a new
   // DevToolsAgent (in a different process), but we can preserve the state of
@@ -65,7 +65,6 @@ interface DevToolsAgent {
   AttachDevToolsSession(associated DevToolsSessionHost host,
                         associated DevToolsSession& session,
                         DevToolsSession& io_session,
-                        string? reattach_state,
                         DevToolsSessionState? reattach_session_state);
 
   // Requests an element at specific position to be inspected in every
@@ -97,16 +96,15 @@ interface DevToolsSession {
 interface DevToolsSessionHost {
   // Dispatches protocol command response to a remote debugging client.
   // |call_id| is a command id as defined in protocol.
-  // |state| is a state for future reattach (see DevToolsAgent),
-  // may be missing if the state did not change since last time.
+  // |updates| are the session state deltas for future reattach (see
+  // DevToolsAgent), may be missing if the state did not change since
+  // last time.
   DispatchProtocolResponse(mojo_base.mojom.BigString message,
                            int32 call_id,
-                           string? state,
                            DevToolsSessionState? updates);
 
   // Dispatches protocol notification to a remote debugging client.
   DispatchProtocolNotification(mojo_base.mojom.BigString message,
-                               string? state,
                                DevToolsSessionState? updates);
 };
 

third_party/blink/renderer/core/exported/web_dev_tools_agent_impl.cc

@@ -207,7 +207,6 @@ class WebDevToolsAgentImpl::Session : public GarbageCollectedFinalized<Session>,
           mojom::blink::DevToolsSessionHostAssociatedPtrInfo host_ptr_info,
           mojom::blink::DevToolsSessionAssociatedRequest main_request,
           mojom::blink::DevToolsSessionRequest io_request,
-          const String& reattach_state,
           mojom::blink::DevToolsSessionStatePtr reattach_session_state);
   ~Session() override;
 
@@ -233,19 +232,16 @@ class WebDevToolsAgentImpl::Session : public GarbageCollectedFinalized<Session>,
       int session_id,
       int call_id,
       const String& response,
-      const String& state,
       mojom::blink::DevToolsSessionStatePtr updates) override;
   void SendProtocolNotification(
       int session_id,
       const String& message,
-      const String& state,
       mojom::blink::DevToolsSessionStatePtr updates) override;
 
   void DispatchProtocolCommandInternal(int call_id,
                                        const String& method,
                                        const String& message);
   void InitializeInspectorSession(
-      const String& reattach_state,
       mojom::blink::DevToolsSessionStatePtr reattach_session_state);
 
   Member<WebDevToolsAgentImpl> agent_;
@@ -317,7 +313,6 @@ WebDevToolsAgentImpl::Session::Session(
     mojom::blink::DevToolsSessionHostAssociatedPtrInfo host_ptr_info,
     mojom::blink::DevToolsSessionAssociatedRequest request,
     mojom::blink::DevToolsSessionRequest io_request,
-    const String& reattach_state,
     mojom::blink::DevToolsSessionStatePtr reattach_session_state)
     : agent_(agent),
       frame_(agent->web_local_frame_impl_),
@@ -331,7 +326,7 @@ WebDevToolsAgentImpl::Session::Session(
   host_ptr_.set_connection_error_handler(WTF::Bind(
       &WebDevToolsAgentImpl::Session::Detach, WrapWeakPersistent(this)));
 
-  InitializeInspectorSession(reattach_state, std::move(reattach_session_state));
+  InitializeInspectorSession(std::move(reattach_session_state));
 }
 
 WebDevToolsAgentImpl::Session::~Session() {
@@ -363,7 +358,6 @@ void WebDevToolsAgentImpl::Session::SendProtocolResponse(
     int session_id,
     int call_id,
     const String& response,
-    const String& state,
     mojom::blink::DevToolsSessionStatePtr updates) {
   if (detached_)
     return;
@@ -372,16 +366,14 @@ void WebDevToolsAgentImpl::Session::SendProtocolResponse(
   // protocol response in any of them.
   if (LayoutTestSupport::IsRunningLayoutTest())
     agent_->FlushProtocolNotifications();
-  host_ptr_->DispatchProtocolResponse(response, call_id, state,
-                                      std::move(updates));
+  host_ptr_->DispatchProtocolResponse(response, call_id, std::move(updates));
 }
 
 void WebDevToolsAgentImpl::Session::SendProtocolNotification(
     int session_id,
     const String& message,
-    const String& state,
     mojom::blink::DevToolsSessionStatePtr updates) {
-  host_ptr_->DispatchProtocolNotification(message, state, std::move(updates));
+  host_ptr_->DispatchProtocolNotification(message, std::move(updates));
 }
 
 void WebDevToolsAgentImpl::Session::DispatchProtocolCommand(
@@ -404,18 +396,19 @@ void WebDevToolsAgentImpl::Session::DispatchProtocolCommand(
 }
 
 void WebDevToolsAgentImpl::Session::InitializeInspectorSession(
-    const String& reattach_state,
     mojom::blink::DevToolsSessionStatePtr reattach_session_state) {
   ClientMessageLoopAdapter::EnsureMainThreadDebuggerCreated();
   MainThreadDebugger* main_thread_debugger = MainThreadDebugger::Instance();
   v8::Isolate* isolate = V8PerIsolateData::MainThreadIsolate();
   InspectedFrames* inspected_frames = agent_->inspected_frames_.Get();
 
+  bool should_reattach = !reattach_session_state.is_null();
+
   inspector_session_ = new InspectorSession(
       this, agent_->probe_sink_.Get(), 0,
       main_thread_debugger->GetV8Inspector(),
       main_thread_debugger->ContextGroupId(inspected_frames->Root()),
-      reattach_state, std::move(reattach_session_state));
+      std::move(reattach_session_state));
 
   InspectorDOMAgent* dom_agent = new InspectorDOMAgent(
       isolate, inspected_frames, inspector_session_->V8Session());
@@ -489,7 +482,7 @@ void WebDevToolsAgentImpl::Session::InitializeInspectorSession(
       inspector_session_, agent_->include_view_agents_, dom_agent,
       inspected_frames, frame_->ViewImpl()->GetPage());
 
-  if (!reattach_state.IsNull()) {
+  if (should_reattach) {
     inspector_session_->Restore();
     if (agent_->worker_client_)
       agent_->worker_client_->ResumeStartup();
@@ -564,14 +557,12 @@ void WebDevToolsAgentImpl::AttachDevToolsSession(
     mojom::blink::DevToolsSessionHostAssociatedPtrInfo host,
     mojom::blink::DevToolsSessionAssociatedRequest session_request,
     mojom::blink::DevToolsSessionRequest io_session_request,
-    const String& reattach_state,
     mojom::blink::DevToolsSessionStatePtr reattach_session_state) {
   if (!sessions_.size())
     Platform::Current()->CurrentThread()->AddTaskObserver(this);
-  Session* session =
-      new Session(this, std::move(host), std::move(session_request),
-                  std::move(io_session_request), reattach_state,
-                  std::move(reattach_session_state));
+  Session* session = new Session(
+      this, std::move(host), std::move(session_request),
+      std::move(io_session_request), std::move(reattach_session_state));
   sessions_.insert(session);
   if (node_to_inspect_) {
     session->overlay_agent()->Inspect(node_to_inspect_);

third_party/blink/renderer/core/exported/web_dev_tools_agent_impl.h

@@ -100,7 +100,6 @@ class CORE_EXPORT WebDevToolsAgentImpl final
       mojom::blink::DevToolsSessionHostAssociatedPtrInfo,
       mojom::blink::DevToolsSessionAssociatedRequest main_session,
       mojom::blink::DevToolsSessionRequest io_session,
-      const String& reattach_state,
       mojom::blink::DevToolsSessionStatePtr reattach_session_state) override;
   void InspectElement(const WebPoint& point_in_local_root) override;
 

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

@@ -56,7 +56,6 @@ class CORE_EXPORT InspectorAgent
 
   virtual void Init(CoreProbeSink*,
                     protocol::UberDispatcher*,
-                    protocol::DictionaryValue*,
                     InspectorSessionState*) = 0;
   virtual void Dispose() = 0;
 };
@@ -69,20 +68,12 @@ class InspectorBaseAgent : public InspectorAgent,
 
   void Init(CoreProbeSink* instrumenting_agents,
             protocol::UberDispatcher* dispatcher,
-            protocol::DictionaryValue* state,
             InspectorSessionState* session_state) override {
     instrumenting_agents_ = instrumenting_agents;
     frontend_.reset(
         new typename DomainMetainfo::FrontendClass(dispatcher->channel()));
     DomainMetainfo::DispatcherClass::wire(dispatcher, this);
 
-    state_ = state->getObject(DomainMetainfo::domainName);
-    if (!state_) {
-      std::unique_ptr<protocol::DictionaryValue> new_state =
-          protocol::DictionaryValue::create();
-      state_ = new_state.get();
-      state->setObject(DomainMetainfo::domainName, std::move(new_state));
-    }
     agent_state_.InitFrom(session_state);
   }
 
@@ -91,7 +82,6 @@ class InspectorBaseAgent : public InspectorAgent,
   void Dispose() override {
     disable();
     frontend_.reset();
-    state_ = nullptr;
     instrumenting_agents_ = nullptr;
   }
 
@@ -107,7 +97,6 @@ class InspectorBaseAgent : public InspectorAgent,
     return frontend_.get();
   }
   Member<CoreProbeSink> instrumenting_agents_;
-  protocol::DictionaryValue* state_;
   InspectorAgentState agent_state_;
 
  private:

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

@@ -25,7 +25,6 @@ InspectorSession::InspectorSession(
     int session_id,
     v8_inspector::V8Inspector* inspector,
     int context_group_id,
-    const String& reattach_state,
     mojom::blink::DevToolsSessionStatePtr reattach_session_state)
     : client_(client),
       v8_session_(nullptr),
@@ -33,21 +32,13 @@ InspectorSession::InspectorSession(
       disposed_(false),
       instrumenting_agents_(instrumenting_agents),
       inspector_backend_dispatcher_(new protocol::UberDispatcher(this)),
-      session_state_(std::move(reattach_session_state)) {
-  String v8_state;
-  if (!reattach_state.IsNull()) {
-    std::unique_ptr<protocol::Value> state =
-        protocol::StringUtil::parseJSON(reattach_state);
-    if (state)
-      state_ = protocol::DictionaryValue::cast(std::move(state));
-    if (!state_)
-      state_ = protocol::DictionaryValue::create();
-    state_->getString(kV8StateKey, &v8_state);
-  } else {
-    state_ = protocol::DictionaryValue::create();
-  }
-  v8_session_ = inspector->connect(context_group_id, this,
-                                   ToV8InspectorStringView(v8_state));
+      session_state_(std::move(reattach_session_state)),
+      v8_session_state_(kV8StateKey),
+      v8_session_state_json_(&v8_session_state_, /*default_value=*/String()) {
+  v8_session_state_.InitFrom(&session_state_);
+  v8_session_ =
+      inspector->connect(context_group_id, this,
+                         ToV8InspectorStringView(v8_session_state_json_.Get()));
 }
 
 InspectorSession::~InspectorSession() {
@@ -57,7 +48,7 @@ InspectorSession::~InspectorSession() {
 void InspectorSession::Append(InspectorAgent* agent) {
   agents_.push_back(agent);
   agent->Init(instrumenting_agents_.Get(), inspector_backend_dispatcher_.get(),
-              state_.get(), &session_state_);
+              &session_state_);
 }
 
 void InspectorSession::Restore() {
@@ -140,21 +131,11 @@ void InspectorSession::SendProtocolResponse(int call_id,
   if (disposed_)
     return;
   flushProtocolNotifications();
-  client_->SendProtocolResponse(session_id_, call_id, message, GetStateToSend(),
+  v8_session_state_json_.Set(ToCoreString(v8_session_->stateJSON()));
+  client_->SendProtocolResponse(session_id_, call_id, message,
                                 session_state_.TakeUpdates());
 }
 
-String InspectorSession::GetStateToSend() {
-  if (v8_session_)
-    state_->setString(kV8StateKey, ToCoreString(v8_session_->stateJSON()));
-  String state_to_send = state_->serialize();
-  if (state_to_send == last_sent_state_)
-    state_to_send = String();
-  else
-    last_sent_state_ = state_to_send;
-  return state_to_send;
-}
-
 class InspectorSession::Notification {
  public:
   static std::unique_ptr<Notification> CreateForBlink(
@@ -216,13 +197,11 @@ void InspectorSession::flushProtocolNotifications() {
     agents_[i]->FlushPendingProtocolNotifications();
   if (!notification_queue_.size())
     return;
-  String state_to_send = GetStateToSend();
+  v8_session_state_json_.Set(ToCoreString(v8_session_->stateJSON()));
   for (size_t i = 0; i < notification_queue_.size(); ++i) {
-    client_->SendProtocolNotification(
-        session_id_, notification_queue_[i]->Serialize(), state_to_send,
-        session_state_.TakeUpdates());
-    // Only send state once in this series of serialized updates.
-    state_to_send = String();
+    client_->SendProtocolNotification(session_id_,
+                                      notification_queue_[i]->Serialize(),
+                                      session_state_.TakeUpdates());
   }
   notification_queue_.clear();
 }

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

@@ -33,12 +33,10 @@ class CORE_EXPORT InspectorSession
         int session_id,
         int call_id,
         const String& response,
-        const String& state,
         mojom::blink::DevToolsSessionStatePtr updates) = 0;
     virtual void SendProtocolNotification(
         int session_id,
         const String& message,
-        const String& state,
         mojom::blink::DevToolsSessionStatePtr updates) = 0;
     virtual ~Client() = default;
   };
@@ -49,7 +47,6 @@ class CORE_EXPORT InspectorSession
       int session_id,
       v8_inspector::V8Inspector*,
       int context_group_id,
-      const String& reattach_state,
       mojom::blink::DevToolsSessionStatePtr reattach_session_state);
   ~InspectorSession() override;
   // TODO(dgozman): remove session id once WokrerInspectorController
@@ -89,20 +86,18 @@ class CORE_EXPORT InspectorSession
                    const String& method,
                    const String& message) override;
 
-  String GetStateToSend();
-
   Client* client_;
   std::unique_ptr<v8_inspector::V8InspectorSession> v8_session_;
   int session_id_;
   bool disposed_;
   Member<CoreProbeSink> instrumenting_agents_;
   std::unique_ptr<protocol::UberDispatcher> inspector_backend_dispatcher_;
-  std::unique_ptr<protocol::DictionaryValue> state_;
   InspectorSessionState session_state_;
   HeapVector<Member<InspectorAgent>> agents_;
   class Notification;
   Vector<std::unique_ptr<Notification>> notification_queue_;
-  String last_sent_state_;
+  InspectorAgentState v8_session_state_;
+  InspectorAgentState::String v8_session_state_json_;
 
   DISALLOW_COPY_AND_ASSIGN(InspectorSession);
 };

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

@@ -74,7 +74,7 @@ void WorkerInspectorController::ConnectFrontend(int session_id) {
 
   InspectorSession* session = new InspectorSession(
       this, probe_sink_.Get(), session_id, debugger_->GetV8Inspector(),
-      debugger_->ContextGroupId(thread_), String(), nullptr);
+      debugger_->ContextGroupId(thread_), nullptr);
   session->Append(new InspectorLogAgent(thread_->GetConsoleMessageStorage(),
                                         nullptr, session->V8Session()));
   if (thread_->GlobalScope()->IsWorkerGlobalScope()) {
@@ -129,7 +129,6 @@ void WorkerInspectorController::SendProtocolResponse(
     int session_id,
     int call_id,
     const String& response,
-    const String& state,
     mojom::blink::DevToolsSessionStatePtr updates) {
   // Make tests more predictable by flushing all sessions before sending
   // protocol response in any of them.
@@ -143,7 +142,6 @@ void WorkerInspectorController::SendProtocolResponse(
 void WorkerInspectorController::SendProtocolNotification(
     int session_id,
     const String& message,
-    const String& state,
     mojom::blink::DevToolsSessionStatePtr updates) {
   thread_->GetWorkerReportingProxy().PostMessageToPageInspector(session_id,
                                                                 message);

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

@@ -71,12 +71,10 @@ class WorkerInspectorController final
       int session_id,
       int call_id,
       const String& response,
-      const String& state,
       mojom::blink::DevToolsSessionStatePtr updates) override;
   void SendProtocolNotification(
       int session_id,
       const String& message,
-      const String& state,
       mojom::blink::DevToolsSessionStatePtr updates) override;
 
   // WebThread::TaskObserver implementation.