[DevTools] Rework RFDTAH navigation tracking with browser side navigation

content/browser/devtools/render_frame_devtools_agent_host.cc

Index: content/browser/devtools/render_frame_devtools_agent_host.cc
diff --git a/content/browser/devtools/render_frame_devtools_agent_host.cc b/content/browser/devtools/render_frame_devtools_agent_host.cc
index 7554bf685f6cfac64c914509db0b9e3786dce1d6..fbff6645f6b26a9839761a371b6456e2b811499a 100644
--- a/content/browser/devtools/render_frame_devtools_agent_host.cc
+++ b/content/browser/devtools/render_frame_devtools_agent_host.cc
@@ -123,9 +123,9 @@ class RenderFrameDevToolsAgentHost::FrameHostHolder {
   // <session_id, message>
   std::vector<std::pair<int, std::string>> pending_messages_;
   // <call_id> -> PendingMessage
-  std::map<int, PendingMessage> sent_messages_;
+  std::map<int, Message> sent_messages_;
   // These are sent messages for which we got a reply while suspended.
-  std::map<int, PendingMessage> sent_messages_whose_reply_came_while_suspended_;
+  std::map<int, Message> sent_messages_whose_reply_came_while_suspended_;
 };
 
 RenderFrameDevToolsAgentHost::FrameHostHolder::FrameHostHolder(
@@ -185,11 +185,13 @@ void RenderFrameDevToolsAgentHost::FrameHostHolder::Detach(int session_id) {
 }
 
 void RenderFrameDevToolsAgentHost::FrameHostHolder::GrantPolicy() {
+  agent_->GrantPolicy(host_);
   ChildProcessSecurityPolicyImpl::GetInstance()->GrantReadRawCookies(

[dgozman, 2017/06/09 22:37:05]

This should be removed.

       host_->GetProcess()->GetID());
 }
 
 void RenderFrameDevToolsAgentHost::FrameHostHolder::RevokePolicy() {
+  agent_->RevokePolicy(host_);
   bool process_has_agents = false;

[dgozman, 2017/06/09 22:37:05]

And this (it got moved to RevokePolicy(host)).

   RenderProcessHost* process_host = host_->GetProcess();
   for (RenderFrameDevToolsAgentHost* agent : g_instances.Get()) {
@@ -211,6 +213,7 @@ void RenderFrameDevToolsAgentHost::FrameHostHolder::RevokePolicy() {
         process_host->GetID());
   }
 }
+
 void RenderFrameDevToolsAgentHost::FrameHostHolder::DispatchProtocolMessage(
     int session_id,
     int call_id,
@@ -218,7 +221,7 @@ void RenderFrameDevToolsAgentHost::FrameHostHolder::DispatchProtocolMessage(
     const std::string& message) {
   host_->Send(new DevToolsAgentMsg_DispatchOnInspectorBackend(
       host_->GetRoutingID(), session_id, call_id, method, message));
-  sent_messages_[call_id] = { session_id, method, message };
+  sent_messages_[call_id] = { session_id, call_id, method, message };

[caseq, 2017/06/10 00:02:46]

emplace()? ;-)

[dgozman, 2017/06/10 01:36:34]

That's more code :(

 }
 
 void RenderFrameDevToolsAgentHost::FrameHostHolder::InspectElement(
@@ -238,7 +241,7 @@ void RenderFrameDevToolsAgentHost::FrameHostHolder::SendMessageToClient(
     int session_id,
     const std::string& message) {
   int id = chunk_processor_.last_call_id();
-  PendingMessage sent_message = sent_messages_[id];
+  Message sent_message = sent_messages_[id];
   sent_messages_.erase(id);
   if (suspended_) {
     sent_messages_whose_reply_came_while_suspended_[id] = sent_message;
@@ -342,16 +345,6 @@ void RenderFrameDevToolsAgentHost::OnBeforeNavigation(
     agent_host->AboutToNavigateRenderFrame(current, pending);
 }
 
-// static
-void RenderFrameDevToolsAgentHost::OnBeforeNavigation(
-    NavigationHandle* navigation_handle) {
-  FrameTreeNode* frame_tree_node =
-      static_cast<NavigationHandleImpl*>(navigation_handle)->frame_tree_node();
-  RenderFrameDevToolsAgentHost* agent_host = FindAgentHost(frame_tree_node);
-  if (agent_host)
-    agent_host->AboutToNavigate(navigation_handle);
-}
-
 // static
 void RenderFrameDevToolsAgentHost::OnFailedNavigation(
     RenderFrameHost* host,
@@ -360,8 +353,10 @@ void RenderFrameDevToolsAgentHost::OnFailedNavigation(
     net::Error error_code) {
   RenderFrameDevToolsAgentHost* agent_host =
       FindAgentHost(static_cast<RenderFrameHostImpl*>(host)->frame_tree_node());
-  if (agent_host)
-    agent_host->OnFailedNavigation(common_params, begin_params, error_code);
+  if (!agent_host)
+    return;
+  for (auto* network : protocol::NetworkHandler::ForAgentHost(agent_host))
+    network->NavigationFailed(common_params, begin_params, error_code);
 }
 
 // static
@@ -432,11 +427,18 @@ RenderFrameDevToolsAgentHost::RenderFrameDevToolsAgentHost(
       frame_trace_recorder_(nullptr),
       handlers_frame_host_(nullptr),
       current_frame_crashed_(false),
-      pending_handle_(nullptr),
+      chunk_processor_(base::Bind(
+           &RenderFrameDevToolsAgentHost::SendMessageFromProcessor,
+           base::Unretained(this))),
       frame_tree_node_(frame_tree_node) {
-  if (frame_tree_node->current_frame_host()) {
-    SetPending(frame_tree_node->current_frame_host());
-    CommitPending();
+  if (IsBrowserSideNavigationEnabled()) {
+    frame_host_ = frame_tree_node->current_frame_host();
+    render_frame_alive_ = frame_host_ && frame_host_->IsRenderFrameLive();
+  } else {
+    if (frame_tree_node->current_frame_host()) {
+      SetPending(frame_tree_node->current_frame_host());
+      CommitPending();
+    }
   }
   WebContentsObserver::Observe(
       WebContentsImpl::FromFrameTreeNode(frame_tree_node));
@@ -453,6 +455,7 @@ RenderFrameDevToolsAgentHost::RenderFrameDevToolsAgentHost(
 }
 
 void RenderFrameDevToolsAgentHost::SetPending(RenderFrameHostImpl* host) {
+  DCHECK(!IsBrowserSideNavigationEnabled());
   DCHECK(!pending_);
   current_frame_crashed_ = false;
   pending_.reset(new FrameHostHolder(this, host));
@@ -468,6 +471,7 @@ void RenderFrameDevToolsAgentHost::SetPending(RenderFrameHostImpl* host) {
 }
 
 void RenderFrameDevToolsAgentHost::CommitPending() {
+  DCHECK(!IsBrowserSideNavigationEnabled());
   DCHECK(pending_);
   current_frame_crashed_ = false;
 
@@ -483,6 +487,7 @@ void RenderFrameDevToolsAgentHost::CommitPending() {
 }
 
 void RenderFrameDevToolsAgentHost::DiscardPending() {
+  DCHECK(!IsBrowserSideNavigationEnabled());
   DCHECK(pending_);
   DCHECK(current_);
   pending_.reset();
@@ -501,7 +506,10 @@ WebContents* RenderFrameDevToolsAgentHost::GetWebContents() {
 
 void RenderFrameDevToolsAgentHost::AttachSession(DevToolsSession* session) {
   session->SetFallThroughForNotFound(true);
-  session->SetRenderFrameHost(handlers_frame_host_);
+  if (IsBrowserSideNavigationEnabled())
+    session->SetRenderFrameHost(frame_host_);
+  else
+    session->SetRenderFrameHost(handlers_frame_host_);
   if (frame_tree_node_ && !frame_tree_node_->parent()) {
     session->AddHandler(base::WrapUnique(new protocol::PageHandler()));
     session->AddHandler(base::WrapUnique(new protocol::SecurityHandler()));
@@ -522,18 +530,31 @@ void RenderFrameDevToolsAgentHost::AttachSession(DevToolsSession* session) {
       frame_tree_node_ ? frame_tree_node_->frame_tree_node_id() : 0,
       GetIOContext())));
 
-  if (current_)
-    current_->Attach(session);
-  if (pending_)
-    pending_->Attach(session);
+  if (IsBrowserSideNavigationEnabled()) {
+    if (frame_host_) {
+      frame_host_->Send(new DevToolsAgentMsg_Attach(
+          frame_host_->GetRoutingID(), GetId(), session->session_id()));
+    }
+  } else {
+    if (current_)
+      current_->Attach(session);
+    if (pending_)
+      pending_->Attach(session);
+  }
   OnClientAttached();
 }
 
 void RenderFrameDevToolsAgentHost::DetachSession(int session_id) {
-  if (current_)
-    current_->Detach(session_id);
-  if (pending_)
-    pending_->Detach(session_id);
+  if (IsBrowserSideNavigationEnabled()) {
+    if (frame_host_) {
+      frame_host_->Send(new DevToolsAgentMsg_Detach(frame_host_->GetRoutingID(), session_id));
+    }
+  } else {
+    if (current_)
+      current_->Detach(session_id);
+    if (pending_)
+      pending_->Detach(session_id);
+  }
   OnClientDetached();
 }
 
@@ -542,25 +563,28 @@ bool RenderFrameDevToolsAgentHost::DispatchProtocolMessage(
     const std::string& message) {
   int call_id = 0;
   std::string method;
+  int session_id = session->session_id();
   if (session->Dispatch(message, &call_id, &method) !=
       protocol::Response::kFallThrough) {
     return true;
   }
 
-  if (!navigating_handles_.empty()) {
-    DCHECK(IsBrowserSideNavigationEnabled());
-    in_navigation_protocol_message_buffer_[call_id] =
-        { session->session_id(), method, message };
-    return true;
-  }
-
-  if (current_) {
-    current_->DispatchProtocolMessage(
-        session->session_id(), call_id, method, message);
-  }
-  if (pending_) {
-    pending_->DispatchProtocolMessage(
-        session->session_id(), call_id, method, message);
+  if (IsBrowserSideNavigationEnabled()) {

[caseq, 2017/06/10 00:02:46]

nit: where possible, this is better written as

if (!IsBrowserSideNavigationEnabled()) {
   ...
   return;
}

to minimize the diff when cleaning this up later.

[dgozman, 2017/06/10 01:36:35]

Acknowledged.

+    if (suspended_count_) {
+      suspended_messages_[call_id] = {session_id, call_id, method, message};
+      return true;
+    }
+    if (frame_host_) {
+      frame_host_->Send(new DevToolsAgentMsg_DispatchOnInspectorBackend(
+          frame_host_->GetRoutingID(), session_id, call_id, method, message));
+    }
+    waiting_for_response_messages_[call_id] =
+        {session_id, call_id, method, message};
+  } else {
+    if (current_)
+      current_->DispatchProtocolMessage(session_id, call_id, method, message);
+    if (pending_)
+      pending_->DispatchProtocolMessage(session_id, call_id, method, message);
   }
   return true;
 }
@@ -569,10 +593,17 @@ void RenderFrameDevToolsAgentHost::InspectElement(
     DevToolsSession* session,
     int x,
     int y) {
-  if (current_)
-    current_->InspectElement(session->session_id(), x, y);
-  if (pending_)
-    pending_->InspectElement(session->session_id(), x, y);
+  if (IsBrowserSideNavigationEnabled()) {
+    if (frame_host_) {
+      frame_host_->Send(new DevToolsAgentMsg_InspectElement(
+          frame_host_->GetRoutingID(), session->session_id(), x, y));
+    }
+  } else {
+    if (current_)
+      current_->InspectElement(session->session_id(), x, y);
+    if (pending_)
+      pending_->InspectElement(session->session_id(), x, y);
+  }
 }
 
 void RenderFrameDevToolsAgentHost::OnClientAttached() {
@@ -583,6 +614,8 @@ void RenderFrameDevToolsAgentHost::OnClientAttached() {
 #if defined(OS_ANDROID)
   GetWakeLock()->RequestWakeLock();
 #endif
+  if (IsBrowserSideNavigationEnabled())
+    GrantPolicy(frame_host_);
 }
 
 void RenderFrameDevToolsAgentHost::OnClientDetached() {
@@ -590,7 +623,11 @@ void RenderFrameDevToolsAgentHost::OnClientDetached() {
   GetWakeLock()->CancelWakeLock();
 #endif
   frame_trace_recorder_.reset();
-  in_navigation_protocol_message_buffer_.clear();
+  waiting_for_response_messages_.clear();
+  suspended_messages_.clear();
+  chunk_processor_.Reset();
+  if (IsBrowserSideNavigationEnabled())
+    RevokePolicy(frame_host_);
 }
 
 RenderFrameDevToolsAgentHost::~RenderFrameDevToolsAgentHost() {
@@ -603,41 +640,29 @@ RenderFrameDevToolsAgentHost::~RenderFrameDevToolsAgentHost() {
 
 void RenderFrameDevToolsAgentHost::ReadyToCommitNavigation(
     NavigationHandle* navigation_handle) {
-  // CommitPending may destruct |this|.
-  scoped_refptr<RenderFrameDevToolsAgentHost> protect(this);
-
-  // TODO(clamy): Switch RenderFrameDevToolsAgentHost to always buffer messages
-  // until ReadyToCommitNavigation is called, now that it is also called in
-  // non-PlzNavigate mode.
+  fprintf(stderr, "[%p] ReadyToCommitNavigation [handle=%p, committed=%d] to %p\n", frame_host_, navigation_handle, navigation_handle->HasCommitted(), navigation_handle->GetRenderFrameHost());
   if (!IsBrowserSideNavigationEnabled())
     return;
-
-  // If the navigation is not tracked, return;
-  if (navigating_handles_.count(navigation_handle) == 0)
+  NavigationHandleImpl* handle =
+      static_cast<NavigationHandleImpl*>(navigation_handle);
+  if (handle->frame_tree_node() != frame_tree_node_)
     return;
 
-  RenderFrameHostImpl* render_frame_host_impl =
-      static_cast<RenderFrameHostImpl*>(
-          navigation_handle->GetRenderFrameHost());
-  if (current_->host() != render_frame_host_impl || current_frame_crashed_) {
-    SetPending(render_frame_host_impl);
-    pending_handle_ = navigation_handle;
-    // Commit when navigating the same frame after crash, avoiding the same
-    // host in current_ and pending_.
-    if (current_->host() == render_frame_host_impl) {
-      pending_handle_ = nullptr;
-      CommitPending();
-    }
-  }
+  // UpdateFrameHost may destruct |this|.
+  scoped_refptr<RenderFrameDevToolsAgentHost> protect(this);
+  UpdateFrameHost(handle->GetRenderFrameHost());
   DCHECK(CheckConsistency());
 }
 
 void RenderFrameDevToolsAgentHost::DidFinishNavigation(
     NavigationHandle* navigation_handle) {
-  // CommitPending may destruct |this|.
-  scoped_refptr<RenderFrameDevToolsAgentHost> protect(this);
-
+  if (navigation_handle->HasCommitted())
+    fprintf(stderr, "[%p] DidFinishNavigation [handle=%p] committed to %p\n", frame_host_, navigation_handle, navigation_handle->GetRenderFrameHost());
+  else
+    fprintf(stderr, "[%p] DidFinishNavigation [handle=%p] not committed\n", frame_host_, navigation_handle);
   if (!IsBrowserSideNavigationEnabled()) {
+    // CommitPending may destruct |this|.
+    scoped_refptr<RenderFrameDevToolsAgentHost> protect(this);
     if (navigation_handle->HasCommitted() &&
         !navigation_handle->IsErrorPage()) {
       if (pending_ &&
@@ -654,45 +679,134 @@ void RenderFrameDevToolsAgentHost::DidFinishNavigation(
     return;
   }
 
-  // If the navigation is not tracked, return;
-  if (navigating_handles_.count(navigation_handle) == 0)
+  NavigationHandleImpl* handle =
+      static_cast<NavigationHandleImpl*>(navigation_handle);
+  if (handle->frame_tree_node() != frame_tree_node_)
     return;
 
-  // Now that the navigation is finished, remove the handle from the list of
-  // navigating handles.
-  navigating_handles_.erase(navigation_handle);
-
-  if (pending_handle_ == navigation_handle) {
-    // This navigation handle did set the pending FrameHostHolder.
-    DCHECK(pending_);
-    if (navigation_handle->HasCommitted()) {
-      DCHECK(pending_->host() == navigation_handle->GetRenderFrameHost());
-      CommitPending();
-    } else {
-      DiscardPending();
+  // UpdateFrameHost may destruct |this|.
+  scoped_refptr<RenderFrameDevToolsAgentHost> protect(this);
+  if (handle->HasCommitted() && !handle->IsErrorPage())
+    UpdateFrameHost(handle->GetRenderFrameHost());
+  DCHECK(CheckConsistency());
+  if (!--suspended_count_) {
+    if (session() && frame_host_) {
+      for (const auto& pair : suspended_messages_) {
+        const Message& message = pair.second;
+        frame_host_->Send(new DevToolsAgentMsg_DispatchOnInspectorBackend(
+            frame_host_->GetRoutingID(), message.session_id, message.call_id,
+            message.method, message.message));
+      }
     }
-    pending_handle_ = nullptr;
-  } else if (navigating_handles_.empty()) {
-    current_->Resume();
+    suspended_messages_.clear();

[pfeldman, 2017/06/10 00:32:04]

swap with waiting or extract method DispatchMessageOnRenderer.

[dgozman, 2017/06/10 01:36:34]

Done.

   }
-  DispatchBufferedProtocolMessagesIfNecessary();
-
-  DCHECK(CheckConsistency());
-  if (navigation_handle->HasCommitted()) {
+  if (handle->HasCommitted()) {
     for (auto* target : protocol::TargetHandler::ForAgentHost(this))
       target->UpdateServiceWorkers();
   }
 }
 
+void RenderFrameDevToolsAgentHost::UpdateFrameHost(RenderFrameHostImpl* frame_host) {
+  if (frame_host == frame_host_) {
+    if (frame_host && !render_frame_alive_) {
+      render_frame_alive_ = true;
+      MaybeReattachToRenderFrame();
+    }
+    return;
+  }
+
+  if (session())
+    RevokePolicy(frame_host_);
+
+  if (frame_host && !ShouldCreateDevToolsForHost(frame_host)) {
+    DestroyOnRenderFrameGone();
+    // |this| may be deleted at this point.
+    return;
+  }
+
+  frame_host_ = frame_host;
+  render_frame_alive_ = true;
+  if (session()) {
+    GrantPolicy(frame_host_);
+    session()->SetRenderFrameHost(frame_host);
+  }
+  MaybeReattachToRenderFrame();

[caseq, 2017/06/10 00:02:46]

nit: move into the if above.

[dgozman, 2017/06/10 01:36:35]

Done.

+}
+
+void RenderFrameDevToolsAgentHost::MaybeReattachToRenderFrame() {
+  DCHECK(IsBrowserSideNavigationEnabled());
+  if (!session() || !frame_host_)
+    return;
+  int session_id = session()->session_id();
+  frame_host_->Send(new DevToolsAgentMsg_Reattach(
+      frame_host_->GetRoutingID(), GetId(), session_id,
+      chunk_processor_.state_cookie()));
+  for (const auto& pair : waiting_for_response_messages_) {
+    const Message& message = pair.second;
+    frame_host_->Send(new DevToolsAgentMsg_DispatchOnInspectorBackend(
+        frame_host_->GetRoutingID(), message.session_id, message.call_id,
+        message.method, message.message));
+  }
+}
+
+void RenderFrameDevToolsAgentHost::SendMessageFromProcessor(
+    int session_id,
+    const std::string& message) {
+  int id = chunk_processor_.last_call_id();
+  waiting_for_response_messages_.erase(id);
+  SendMessageToClient(session_id, message);
+  // |this| may be deleted at this point.
+}
+
+void RenderFrameDevToolsAgentHost::GrantPolicy(RenderFrameHostImpl* host) {
+  if (!host)
+    return;
+  ChildProcessSecurityPolicyImpl::GetInstance()->GrantReadRawCookies(
+      host->GetProcess()->GetID());
+}
+
+void RenderFrameDevToolsAgentHost::RevokePolicy(RenderFrameHostImpl* host) {
+  if (!host)
+    return;
+
+  bool process_has_agents = false;
+  RenderProcessHost* process_host = host->GetProcess();
+  for (RenderFrameDevToolsAgentHost* agent : g_instances.Get()) {
+    if (!agent->IsAttached())
+      continue;
+    if (IsBrowserSideNavigationEnabled()) {
+      if (agent->frame_host_ && agent->frame_host_ != host &&
+          agent->frame_host_->GetProcess() == process_host) {
+        process_has_agents = true;
+      }
+      continue;
+    }
+    if (agent->current_ && agent->current_->host() != host &&
+        agent->current_->host()->GetProcess() == process_host) {
+      process_has_agents = true;
+    }
+    if (agent->pending_ && agent->pending_->host() != host &&
+        agent->pending_->host()->GetProcess() == process_host) {
+      process_has_agents = true;
+    }
+  }
+
+  // We are the last to disconnect from the renderer -> revoke permissions.
+  if (!process_has_agents) {
+    ChildProcessSecurityPolicyImpl::GetInstance()->RevokeReadRawCookies(
+        process_host->GetID());
+  }
+}
+
 void RenderFrameDevToolsAgentHost::AboutToNavigateRenderFrame(
     RenderFrameHost* old_host,
     RenderFrameHost* new_host) {
-  // CommitPending may destruct |this|.
-  scoped_refptr<RenderFrameDevToolsAgentHost> protect(this);
-
   if (IsBrowserSideNavigationEnabled())
     return;
 
+  // CommitPending may destruct |this|.
+  scoped_refptr<RenderFrameDevToolsAgentHost> protect(this);
+
   DCHECK(!pending_ || pending_->host() != old_host);
   if (!current_ || current_->host() != old_host) {
     DCHECK(CheckConsistency());
@@ -711,36 +825,39 @@ void RenderFrameDevToolsAgentHost::AboutToNavigateRenderFrame(
   DCHECK(CheckConsistency());
 }
 
-void RenderFrameDevToolsAgentHost::AboutToNavigate(
+void RenderFrameDevToolsAgentHost::DidStartNavigation(
     NavigationHandle* navigation_handle) {
+  fprintf(stderr, "[%p] DidStartNavigation\n", frame_host_);
   if (!IsBrowserSideNavigationEnabled())
     return;
-  DCHECK(current_);
-  navigating_handles_.insert(navigation_handle);
-  current_->Suspend();
+  NavigationHandleImpl* handle =
+      static_cast<NavigationHandleImpl*>(navigation_handle);
+  if (handle->frame_tree_node() != frame_tree_node_)
+    return;
+  suspended_count_++;
   DCHECK(CheckConsistency());
 }
 
-void RenderFrameDevToolsAgentHost::OnFailedNavigation(
-    const CommonNavigationParams& common_params,
-    const BeginNavigationParams& begin_params,
-    net::Error error_code) {
-  DCHECK(IsBrowserSideNavigationEnabled());
-  for (auto* network : protocol::NetworkHandler::ForAgentHost(this))
-    network->NavigationFailed(common_params, begin_params, error_code);
-}
-
 void RenderFrameDevToolsAgentHost::RenderFrameHostChanged(
     RenderFrameHost* old_host,
     RenderFrameHost* new_host) {
-  // CommitPending may destruct |this|.
-  scoped_refptr<RenderFrameDevToolsAgentHost> protect(this);
-
+  fprintf(stderr, "[%p] RenderFrameHostChanged from %p to %p\n", frame_host_, old_host, new_host);
   for (auto* target : protocol::TargetHandler::ForAgentHost(this))
     target->UpdateFrames();
 
-  if (IsBrowserSideNavigationEnabled() && !current_frame_crashed_)
+  if (IsBrowserSideNavigationEnabled()) {
+    if (old_host != frame_host_)
+      return;
+
+    // UpdateFrameHost may destruct |this|.
+    scoped_refptr<RenderFrameDevToolsAgentHost> protect(this);
+    UpdateFrameHost(static_cast<RenderFrameHostImpl*>(new_host));

[pfeldman, 2017/06/10 00:32:05]

Try passing null here.

+    DCHECK(CheckConsistency());
     return;
+  }
+
+  // CommitPending may destruct |this|.
+  scoped_refptr<RenderFrameDevToolsAgentHost> protect(this);
 
   DCHECK(!pending_ || pending_->host() != old_host);
   if (!current_ || current_->host() != old_host) {
@@ -757,6 +874,15 @@ void RenderFrameDevToolsAgentHost::RenderFrameHostChanged(
 }
 
 void RenderFrameDevToolsAgentHost::FrameDeleted(RenderFrameHost* rfh) {
+  fprintf(stderr, "[%p] FrameDeleted %p\n", frame_host_, rfh);
+  if (IsBrowserSideNavigationEnabled()) {
+    if (static_cast<RenderFrameHostImpl*>(rfh)->frame_tree_node() == frame_tree_node_)
+      DestroyOnRenderFrameGone();  // |this| may be deleted at this point.
+    else
+      DCHECK(CheckConsistency());
+    return;
+  }
+
   if (pending_ && pending_->host() == rfh) {
     if (!IsBrowserSideNavigationEnabled())
       DiscardPending();
@@ -769,6 +895,14 @@ void RenderFrameDevToolsAgentHost::FrameDeleted(RenderFrameHost* rfh) {
 }
 
 void RenderFrameDevToolsAgentHost::RenderFrameDeleted(RenderFrameHost* rfh) {
+  fprintf(stderr, "[%p] RenderFrameDeleted %p\n", frame_host_, rfh);
+  if (IsBrowserSideNavigationEnabled()) {
+    if (rfh == frame_host_)
+      render_frame_alive_ = false;

[caseq, 2017/06/10 00:02:46]

should we also have frame_host_ = nullptr here?

[dgozman, 2017/06/10 01:36:35]

Nope, this means renderer is dead, but not the host.

+    DCHECK(CheckConsistency());
+    return;
+  }
+
   if (!current_frame_crashed_)
     FrameDeleted(rfh);
   else
@@ -776,30 +910,36 @@ void RenderFrameDevToolsAgentHost::RenderFrameDeleted(RenderFrameHost* rfh) {
 }
 
 void RenderFrameDevToolsAgentHost::DestroyOnRenderFrameGone() {
-  DCHECK(current_);
   scoped_refptr<RenderFrameDevToolsAgentHost> protect(this);
   UpdateProtocolHandlers(nullptr);
   if (IsAttached())
     OnClientDetached();
   ForceDetach(false);
+  frame_host_ = nullptr;
   pending_.reset();
   current_.reset();
   frame_tree_node_ = nullptr;
-  pending_handle_ = nullptr;
   WebContentsObserver::Observe(nullptr);
   Release();
 }
 
 bool RenderFrameDevToolsAgentHost::CheckConsistency() {
-  if (current_ && pending_ && current_->host() == pending_->host())
-    return false;
-  if (IsBrowserSideNavigationEnabled())
-    return true;
+  if (!IsBrowserSideNavigationEnabled()) {
+    if (current_ && pending_ && current_->host() == pending_->host())
+      return false;
+  }
+
   if (!frame_tree_node_)
-    return !handlers_frame_host_;
+    return !frame_host_;
+
   RenderFrameHostManager* manager = frame_tree_node_->render_manager();
-  return handlers_frame_host_ == manager->current_frame_host() ||
-      handlers_frame_host_ == manager->pending_frame_host();
+  RenderFrameHostImpl* current = manager->current_frame_host();
+  RenderFrameHostImpl* pending = manager->pending_frame_host();
+  RenderFrameHostImpl* speculative = manager->speculative_frame_host();
+  RenderFrameHostImpl* host = IsBrowserSideNavigationEnabled() ? frame_host_ : handlers_frame_host_;
+  if (host != current && host != pending && host != speculative)
+    fprintf(stderr, "[%p] CheckConsistency differs from %p, %p and %p\n", host, current, pending, speculative);
+  return host == current || host == pending || host == speculative;

[caseq, 2017/06/10 00:02:46]

can we add a check for !current_ && !pending_ here?

[dgozman, 2017/06/10 01:36:34]

Done.

 }
 
 #if defined(OS_ANDROID)
@@ -848,10 +988,15 @@ void RenderFrameDevToolsAgentHost::RenderProcessGone(
 bool RenderFrameDevToolsAgentHost::OnMessageReceived(
     const IPC::Message& message,
     RenderFrameHost* render_frame_host) {
-  bool is_current = current_ && current_->host() == render_frame_host;
-  bool is_pending = pending_ && pending_->host() == render_frame_host;
-  if (!is_current && !is_pending)
-    return false;
+  if (IsBrowserSideNavigationEnabled()) {
+    if (render_frame_host != frame_host_)
+      return false;
+  } else {
+    bool is_current = current_ && current_->host() == render_frame_host;
+    bool is_pending = pending_ && pending_->host() == render_frame_host;
+    if (!is_current && !is_pending)
+      return false;
+  }
   if (!IsAttached())
     return false;
   bool handled = true;
@@ -870,6 +1015,9 @@ void RenderFrameDevToolsAgentHost::DidAttachInterstitialPage() {
   for (auto* page : protocol::PageHandler::ForAgentHost(this))
     page->DidAttachInterstitialPage();
 
+  if (IsBrowserSideNavigationEnabled())
+    return;
+
   // TODO(dgozman): this may break for cross-process subframes.
   if (!pending_) {
     DCHECK(CheckConsistency());
@@ -878,7 +1026,6 @@ void RenderFrameDevToolsAgentHost::DidAttachInterstitialPage() {
   // Pending set in AboutToNavigateRenderFrame turned out to be interstitial.
   // Connect back to the real one.
   DiscardPending();
-  pending_handle_ = nullptr;
   DCHECK(CheckConsistency());
 }
 
@@ -920,30 +1067,15 @@ void RenderFrameDevToolsAgentHost::DidReceiveCompositorFrame() {
   }
 }
 
-void RenderFrameDevToolsAgentHost::
-    DispatchBufferedProtocolMessagesIfNecessary() {
-  if (navigating_handles_.empty() &&
-      in_navigation_protocol_message_buffer_.size()) {
-    DCHECK(current_);
-    for (const auto& pair : in_navigation_protocol_message_buffer_) {
-      current_->DispatchProtocolMessage(
-          pair.second.session_id, pair.first, pair.second.method,
-          pair.second.message);
-    }
-    in_navigation_protocol_message_buffer_.clear();
-  }
-}
-
 void RenderFrameDevToolsAgentHost::UpdateProtocolHandlers(
     RenderFrameHostImpl* host) {
+  if (IsBrowserSideNavigationEnabled())
+    return;
 #if DCHECK_IS_ON()
-  // TODO(dgozman): fix this for browser side navigation.
-  if (!IsBrowserSideNavigationEnabled()) {
-    // Check that we don't have stale host object here by accessing some random
-    // properties inside.
-    if (handlers_frame_host_ && handlers_frame_host_->GetRenderWidgetHost())
-      handlers_frame_host_->GetRenderWidgetHost()->GetRoutingID();
-  }
+  // Check that we don't have stale host object here by accessing some random
+  // properties inside.
+  if (handlers_frame_host_ && handlers_frame_host_->GetRenderWidgetHost())
+    handlers_frame_host_->GetRenderWidgetHost()->GetRoutingID();
 #endif
   handlers_frame_host_ = host;
   if (session())
@@ -951,6 +1083,12 @@ void RenderFrameDevToolsAgentHost::UpdateProtocolHandlers(
 }
 
 void RenderFrameDevToolsAgentHost::DisconnectWebContents() {
+  if (IsBrowserSideNavigationEnabled()) {
+    UpdateFrameHost(nullptr);
+    frame_tree_node_ = nullptr;

[pfeldman, 2017/06/10 00:32:04]

suspended_count = нулю;

[dgozman, 2017/06/10 01:36:35]

Done.

+    WebContentsObserver::Observe(nullptr);
+    return;
+  }
   if (pending_)
     DiscardPending();
   UpdateProtocolHandlers(nullptr);
@@ -958,13 +1096,20 @@ void RenderFrameDevToolsAgentHost::DisconnectWebContents() {
   if (session())
     disconnected_->Detach(session()->session_id());
   frame_tree_node_ = nullptr;
-  in_navigation_protocol_message_buffer_.clear();
-  navigating_handles_.clear();
-  pending_handle_ = nullptr;
   WebContentsObserver::Observe(nullptr);
 }
 
 void RenderFrameDevToolsAgentHost::ConnectWebContents(WebContents* wc) {
+  if (IsBrowserSideNavigationEnabled()) {
+    RenderFrameHostImpl* host =
+        static_cast<RenderFrameHostImpl*>(wc->GetMainFrame());
+    DCHECK(host);
+    frame_tree_node_ = host->frame_tree_node();
+    WebContentsObserver::Observe(wc);
+    UpdateFrameHost(host);
+    return;
+  }
+
   // CommitPending may destruct |this|.
   scoped_refptr<RenderFrameDevToolsAgentHost> protect(this);
 
@@ -1020,8 +1165,13 @@ std::string RenderFrameDevToolsAgentHost::GetTitle() {
     if (!title.empty())
       return title;
   }
-  if (current_ && current_->host()->GetParent())
-    return current_->host()->GetLastCommittedURL().spec();
+  if (IsBrowserSideNavigationEnabled()) {
+    if (IsChildFrame() && frame_host_)
+      return frame_host_->GetLastCommittedURL().spec();
+  } else {
+    if (current_ && current_->host()->GetParent())
+      return current_->host()->GetLastCommittedURL().spec();
+  }
   if (web_contents())
     return base::UTF16ToUTF8(web_contents()->GetTitle());
   return GetURL().spec();
@@ -1039,6 +1189,8 @@ GURL RenderFrameDevToolsAgentHost::GetURL() {
   WebContents* web_contents = GetWebContents();
   if (web_contents && !IsChildFrame())
     return web_contents->GetVisibleURL();
+  if (IsBrowserSideNavigationEnabled() && frame_host_)

[pfeldman, 2017/06/10 00:32:04]

nested ifs, return for browser-side early.

[dgozman, 2017/06/10 01:36:35]

Done.

+    return frame_host_->GetLastCommittedURL();
   if (pending_)
     return pending_->host()->GetLastCommittedURL();
   if (current_)
@@ -1114,8 +1266,13 @@ void RenderFrameDevToolsAgentHost::SynchronousSwapCompositorFrame(
   for (auto* tracing : protocol::TracingHandler::ForAgentHost(this))
     did_initiate_recording |= tracing->did_initiate_recording();
   if (did_initiate_recording) {
-    frame_trace_recorder_->OnSynchronousSwapCompositorFrame(
-        current_ ? current_->host() : nullptr, frame_metadata);
+    if (IsBrowserSideNavigationEnabled()) {
+      frame_trace_recorder_->OnSynchronousSwapCompositorFrame(
+          frame_host_, frame_metadata);
+    } else {
+      frame_trace_recorder_->OnSynchronousSwapCompositorFrame(
+          current_ ? current_->host() : nullptr, frame_metadata);
+    }
   }
 }
 
@@ -1123,10 +1280,15 @@ void RenderFrameDevToolsAgentHost::OnDispatchOnInspectorFrontend(
     RenderFrameHost* sender,
     const DevToolsMessageChunk& message) {
   bool success = true;
-  if (current_ && current_->host() == sender)
-    success = current_->ProcessChunkedMessageFromAgent(message);
-  else if (pending_ && pending_->host() == sender)
-    success = pending_->ProcessChunkedMessageFromAgent(message);
+  if (IsBrowserSideNavigationEnabled()) {
+    if (sender == frame_host_)
+      success = chunk_processor_.ProcessChunkedMessageFromAgent(message);
+  } else {
+    if (current_ && current_->host() == sender)
+      success = current_->ProcessChunkedMessageFromAgent(message);
+    else if (pending_ && pending_->host() == sender)
+      success = pending_->ProcessChunkedMessageFromAgent(message);
+  }
   if (!success) {
     bad_message::ReceivedBadMessage(
         sender->GetProcess(),
@@ -1153,7 +1315,7 @@ void RenderFrameDevToolsAgentHost::OnRequestNewWindow(
 }
 
 bool RenderFrameDevToolsAgentHost::IsChildFrame() {
-  return current_ && current_->host()->GetParent();
+  return frame_tree_node_ && frame_tree_node_->parent();
 }
 
 }  // namespace content