Adding non-tracking mode support to AddressTracker.

net/base/address_tracker_linux.h

Index: net/base/address_tracker_linux.h
diff --git a/net/base/address_tracker_linux.h b/net/base/address_tracker_linux.h
index 415e8c89e9d8dc354d48c8a48dc24064068f71fd..c331aa808d7a323f837efafa55b80965cb513993 100644
--- a/net/base/address_tracker_linux.h
+++ b/net/base/address_tracker_linux.h
@@ -20,6 +20,8 @@
 #include "base/message_loop/message_loop.h"
 #include "base/synchronization/condition_variable.h"
 #include "base/synchronization/lock.h"
+#include "base/threading/thread_checker.h"
+#include "base/threading/thread_restrictions.h"

[pauljensen, 2014/09/19 19:04:29]

unnecessary?

[guoweis2, 2014/09/19 19:15:38]

Done.

 #include "net/base/net_util.h"
 #include "net/base/network_change_notifier.h"
 
@@ -33,19 +35,33 @@ class NET_EXPORT_PRIVATE AddressTrackerLinux :
  public:
   typedef std::map<IPAddressNumber, struct ifaddrmsg> AddressMap;
 
-  // Will run |address_callback| when the AddressMap changes, |link_callback|
-  // when the list of online links changes, and |tunnel_callback| when the list
-  // of online tunnels changes.
+  // Non-tracking version constructor: When AddressTrackerLinux is
+  // constructed without callbacks, it takes a snapshot of current

[pauljensen, 2014/09/19 19:04:29]

Remove "When AddressTrackerLinux is constructed without callbacks, " as I think
this is redundant with the constructor declared on the following line.

[guoweis2, 2014/09/19 19:15:37]

Done.

+  // system configuration once Init() returns. The configuration is
+  // available through GetOnlineLinks() and GetAddressMap (). Note

[pauljensen, 2014/09/19 19:04:29]

GetAddressMap ()->GetAddressMap()

[guoweis2, 2014/09/19 19:15:38]

Done.

+  // that the thread creating |AddressTrackerLinux| must be the one
+  // calling Init(). Otherwise, the |thread_checker_| will fail the
+  // assert on CalledOnValidThread().

[pauljensen, 2014/09/19 19:04:29]

I don't think this note is necessary.  I think classes in chromium are generally
considered single-threaded unless otherwise noted.

[guoweis2, 2014/09/19 19:15:37]

Done.

+  AddressTrackerLinux();
+
+  // Tracking version constructor: it will run |address_callback| when
+  // the AddressMap changes, |link_callback| when the list of online
+  // links changes, and |tunnel_callback| when the list of online
+  // tunnels changes.
   AddressTrackerLinux(const base::Closure& address_callback,
                       const base::Closure& link_callback,
                       const base::Closure& tunnel_callback);
   virtual ~AddressTrackerLinux();
 
-  // Starts watching system configuration for changes. The current thread must
-  // have a MessageLoopForIO.
+  // In tracking mode, it starts watching system configuration for
+  // changes. The current thread must have a MessageLoopForIO. For
+  // non-tracking mode, the |AddressMap| and |online_links_| will be a
+  // one time snapshot of current system configuration after Init()
+  // returns.
   void Init();
 
   AddressMap GetAddressMap() const;
+  base::hash_set<int> GetOnlineLinks() const;
 
   // Implementation of NetworkChangeNotifierLinux::GetCurrentConnectionType().
   // Safe to call from any thread, but will block until Init() has completed.
@@ -54,6 +70,21 @@ class NET_EXPORT_PRIVATE AddressTrackerLinux :
  private:
   friend class AddressTrackerLinuxTest;
 
+  // A private nested class to facilitate different locking behaviors

[pauljensen, 2014/09/19 19:04:29]

Remove "A private nested class " as that's redundant with where we are in the
file.
I don't really like "facilitate different locking behaviors" as in non-tracking
mode we don't actually lock.  How about "In tracking mode, holds |lock| while
alive.  In non-tracking mode, enforces single-threaded access."

[guoweis2, 2014/09/19 19:15:37]

Done.

+  // depending on whether the |AddressTrackerLinux| is in tracking
+  // mode or not. Lock will only be taken in the tracking mode.
+  class AddressTrackerAutoLock {
+   public:
+    AddressTrackerAutoLock(const AddressTrackerLinux& tracker,
+                           base::Lock& lock);
+    ~AddressTrackerAutoLock();
+
+   private:
+    const AddressTrackerLinux& tracker_;
+    base::Lock& lock_;
+    DISALLOW_COPY_AND_ASSIGN(AddressTrackerAutoLock);
+  };
+
   // A function that returns the name of an interface given the interface index
   // in |interface_index|.
   typedef const char* (*GetInterfaceNameFunction)(int interface_index);
@@ -105,12 +136,19 @@ class NET_EXPORT_PRIVATE AddressTrackerLinux :
   AddressMap address_map_;
 
   // Set of interface indices for links that are currently online.
+  mutable base::Lock online_links_lock_;
   base::hash_set<int> online_links_;
 
   base::Lock is_offline_lock_;
   bool is_offline_;
   bool is_offline_initialized_;
   base::ConditionVariable is_offline_initialized_cv_;
+  bool tracking_;
+
+  // A |ThreadChecker| used to verify single-threaded access in
+  // non-tracking mode. This is to catch any unexpected callback
+  // happening in the non-tracking mode.

[pauljensen, 2014/09/19 19:04:29]

Remove "This is to catch any unexpected callback happening in the non-tracking
mode." as there are no callbacks used.

+  base::ThreadChecker thread_checker_;
 };
 
 }  // namespace internal