Added a debug info UI for Blimp

blimp/net/blimp_connection_details.h

Index: blimp/net/blimp_connection_details.h
diff --git a/blimp/net/blimp_connection_details.h b/blimp/net/blimp_connection_details.h
new file mode 100644
index 0000000000000000000000000000000000000000..9a039f50b01a2dad760fa7142003373f2fb562ce
--- /dev/null
+++ b/blimp/net/blimp_connection_details.h
@@ -0,0 +1,68 @@
+// Copyright 2016 The Chromium Authors. All rights reserved.

[Khushal, 2016/05/18 00:23:01]

A test for this class?

+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef BLIMP_NET_BLIMP_CONNECTION_DETAILS_H_
+#define BLIMP_NET_BLIMP_CONNECTION_DETAILS_H_
+
+#include "base/macros.h"
+#include "base/memory/weak_ptr.h"
+#include "blimp/net/blimp_net_export.h"
+
+namespace blimp {
+
+class NetworkActivityObserver {

[Khushal, 2016/05/18 00:23:01]

Which thread are the NetworkActivityObservers expected to be called on?

[shaktisahu, 2016/05/19 21:39:19]

Done.

+ public:
+  virtual void UpdateDebugInfo(int received, int sent, int commits) = 0;
+};
+
+// Collects network traffic statistics. Maintains a counter for number of
+// completed commits, bytes sent and received over network and notifies its
+// observers on the main thread. Presents the data on a per navigation basis.
+class BLIMP_NET_EXPORT BlimpConnectionDetails {

[Khushal, 2016/05/18 00:23:01]

Could you add a comment about the lifetime of this class. Which thread it is
created and destroyed on?

[shaktisahu, 2016/05/19 21:39:19]

Done.

+ public:
+  BlimpConnectionDetails(
+      const scoped_refptr<base::TaskRunner>& main_thread_task_runner,
+      const scoped_refptr<base::TaskRunner>& io_thread_task_runner);
+
+  ~BlimpConnectionDetails();
+
+  void EnableDebugInfo(bool enable);
+
+  // Called on the IO thread.
+  void OnPacketReceived(int bytes);

[Khushal, 2016/05/18 00:23:01]

Would be nice to just divide the methods into 2 groups, with comments about
which thread they will be called on.

[shaktisahu, 2016/05/19 21:39:19]

Done.

+  void OnPacketSent(int bytes);
+
+  // Called on the main thread.
+  void OnCommit();
+
+  // Called on the main thread.
+  void ResetStats();
+
+  void SetObserver(base::WeakPtr<NetworkActivityObserver> observer) {

[Khushal, 2016/05/18 00:23:01]

Do you really need an Observer here? We usually use the term Observer when a
class has multiple observers that need to be notified of the events.
I think you can call it a Delegate or Client. Also, pass it in the ctor instead?

[shaktisahu, 2016/05/19 21:39:19]

Moved it to constructor. Actually I wanted to make it an observer that listens
to these statistics collection events. Theoretically there can be multiple
observers (one for UI, one for something else....).

+    observer_ = observer;
+  }
+
+ private:
+  void ResetStatsOnIOThread();
+
+  // Flag to show/hide the debug data on blimp view.
+  bool debug_info_enabled_;
+
+  // Total number of bytes sent/received during a navigation.
+  int bytes_received_;

[Khushal, 2016/05/18 00:23:01]

Please mark the thread on which these variables are accessed. I would suggest
put them in a struct and use an accessor to make it obvious.

+  int bytes_sent_;
+
+  // Total number of commits completed on the client.
+  int commits_;
+
+  base::WeakPtr<NetworkActivityObserver> observer_;
+  scoped_refptr<base::TaskRunner> main_thread_task_runner_;
+  scoped_refptr<base::TaskRunner> io_thread_task_runner_;
+
+  DISALLOW_COPY_AND_ASSIGN(BlimpConnectionDetails);
+};
+
+}  // namespace blimp
+
+#endif  // BLIMP_NET_BLIMP_CONNECTION_DETAILS_H_