Added a debug info UI for Blimp

blimp/client/session/blimp_client_session.cc

Index: blimp/client/session/blimp_client_session.cc
diff --git a/blimp/client/session/blimp_client_session.cc b/blimp/client/session/blimp_client_session.cc
index 760842abc72bb5cd32e63d9b1f4782aa071cb4d0..f3a89f327f82a6b162c1888ec9ebdbf3b81fa33a 100644
--- a/blimp/client/session/blimp_client_session.cc
+++ b/blimp/client/session/blimp_client_session.cc
@@ -73,7 +73,8 @@ class ClientNetworkComponents : public ConnectionHandler,
  public:
   // Can be created on any thread.
   explicit ClientNetworkComponents(
-      std::unique_ptr<NetworkEventObserver> observer);
+      std::unique_ptr<NetworkEventObserver> observer,
+      BlimpConnectionStatistics* blimp_connection_statistics);
   ~ClientNetworkComponents() override;
 
   // Sets up network components.
@@ -95,14 +96,17 @@ class ClientNetworkComponents : public ConnectionHandler,
   std::unique_ptr<BrowserConnectionHandler> connection_handler_;
   std::unique_ptr<ClientConnectionManager> connection_manager_;
   std::unique_ptr<NetworkEventObserver> network_observer_;
+  BlimpConnectionStatistics* connection_statistics_;
 
   DISALLOW_COPY_AND_ASSIGN(ClientNetworkComponents);
 };
 
 ClientNetworkComponents::ClientNetworkComponents(
-    std::unique_ptr<NetworkEventObserver> network_observer)
+    std::unique_ptr<NetworkEventObserver> network_observer,
+    BlimpConnectionStatistics* statistics)
     : connection_handler_(new BrowserConnectionHandler),
-      network_observer_(std::move(network_observer)) {}
+      network_observer_(std::move(network_observer)),
+      connection_statistics_(statistics) {}
 
 ClientNetworkComponents::~ClientNetworkComponents() {}
 
@@ -150,6 +154,7 @@ void ClientNetworkComponents::HandleConnection(
   VLOG(1) << "Connection established.";
   connection->AddConnectionErrorObserver(this);
   network_observer_->OnConnected();
+  connection->SetBlimpConnectionStatistics(connection_statistics_);
   connection_handler_->HandleConnection(std::move(connection));
 }
 
@@ -166,13 +171,15 @@ BlimpClientSession::BlimpClientSession(const GURL& assigner_endpoint)
       render_widget_feature_(new RenderWidgetFeature),
       settings_feature_(new SettingsFeature),
       weak_factory_(this) {
-  net_components_.reset(new ClientNetworkComponents(
-      base::WrapUnique(new CrossThreadNetworkEventObserver(
-          weak_factory_.GetWeakPtr(),
-          base::SequencedTaskRunnerHandle::Get()))));
   base::Thread::Options options;
   options.message_loop_type = base::MessageLoop::TYPE_IO;
   io_thread_.StartWithOptions(options);
+  blimp_connection_statistics_.reset(

[Kevin M, 2016/05/20 01:02:03]

Can you move this into ClientNetworkComponents? We consciously chose to relegate
all the IO thread affine stuff into that class for simplicity's sake, so we
limit the mixing of UI thread members and IO thread members in a single object.
That makes destruction pretty error-prone.

[shaktisahu, 2016/05/22 22:36:56]

This object needs to stay closer to the UI thread since it take care of
threadsafety itself. Also it has data from both IO and UI thread. (Bytes
sent/received from IO thread while commits from UI thread). If I keep it here,
as we saw, I wouldn't have to pass asynchronous callbacks into Java and Java
code would be much simpler.

+      new BlimpConnectionStatistics(base::SequencedTaskRunnerHandle::Get()));
+  net_components_.reset(new ClientNetworkComponents(
+      base::WrapUnique(new CrossThreadNetworkEventObserver(
+          weak_factory_.GetWeakPtr(), base::SequencedTaskRunnerHandle::Get())),
+      blimp_connection_statistics_.get()));
 
   assignment_source_.reset(new AssignmentSource(
       assigner_endpoint, io_thread_.task_runner(), io_thread_.task_runner()));
@@ -188,6 +195,8 @@ BlimpClientSession::BlimpClientSession(const GURL& assigner_endpoint)
 
 BlimpClientSession::~BlimpClientSession() {
   io_thread_.task_runner()->DeleteSoon(FROM_HERE, net_components_.release());
+  io_thread_.task_runner()->DeleteSoon(FROM_HERE,
+                                       blimp_connection_statistics_.release());
 }
 
 void BlimpClientSession::Connect(const std::string& client_auth_token) {
@@ -251,6 +260,14 @@ void BlimpClientSession::RegisterFeatures() {
                                             ime_feature_.get()));
 }
 
+void BlimpClientSession::GetDebugInfo() {
+  blimp_connection_statistics_->GetDebugInfo(base::Bind(
+      &BlimpClientSession::UpdateDebugInfo, weak_factory_.GetWeakPtr()));
+}
+
+void BlimpClientSession::UpdateDebugInfo(
+    BlimpConnectionStatistics::StatisticsMap stats) {}
+
 void BlimpClientSession::OnConnected() {}
 
 void BlimpClientSession::OnDisconnected(int result) {}
@@ -275,5 +292,10 @@ SettingsFeature* BlimpClientSession::GetSettingsFeature() const {
   return settings_feature_.get();
 }
 
+BlimpConnectionStatistics* BlimpClientSession::GetBlimpConnectionStatistics()

[Kevin M, 2016/05/20 01:02:03]

This is risky - we are exposing an IO thread member with this getter.

[shaktisahu, 2016/05/22 22:36:56]

Yea, but if it lives on UI thread and is called from UI thread, it should be
okay.

+    const {
+  return blimp_connection_statistics_.get();
+}
+
 }  // namespace client
 }  // namespace blimp