Use Netlink instead of NetworkManager (via D-bus) to monitor network

net/base/address_tracker_linux_unittest.cc

Index: net/base/address_tracker_linux_unittest.cc
diff --git a/net/base/address_tracker_linux_unittest.cc b/net/base/address_tracker_linux_unittest.cc
index c366f5aa2c3edcbebaf24667eac35149eb99cbac..821b6cd709c9b16d80d20c33675ecf0cac774630 100644
--- a/net/base/address_tracker_linux_unittest.cc
+++ b/net/base/address_tracker_linux_unittest.cc
@@ -4,6 +4,8 @@
 
 #include "net/base/address_tracker_linux.h"
 
+#include <linux/if.h>
+
 #include <vector>
 
 #include "base/bind.h"
@@ -16,16 +18,21 @@ void Noop() {}
 
 class AddressTrackerLinuxTest : public testing::Test {
  protected:
-  AddressTrackerLinuxTest() : tracker_(base::Bind(&Noop)) {}
+  AddressTrackerLinuxTest() : tracker_(base::Bind(&Noop), base::Bind(&Noop)) {}
 
-  bool HandleMessage(char* buf, size_t length) {
-    return tracker_.HandleMessage(buf, length);
+  void HandleMessage(
+      char* buf, size_t length, bool* address_changed, bool* link_changed) {

[szym, 2012/11/21 20:14:10]

Style says that in multi-line function declarations one argument per line.

I also suggest moving the typedef Buffer from line 41 above and accepting that
as input.

+    tracker_.HandleMessage(buf, length, address_changed, link_changed);
   }
 
   AddressTrackerLinux::AddressMap GetAddressMap() {
     return tracker_.GetAddressMap();
   }
 
+  const base::hash_set<int>* GetOnlineLinks() const {
+    return &tracker_.online_links_;
+  }
+
   AddressTrackerLinux tracker_;
 };
 
@@ -83,12 +90,12 @@ class NetlinkMessage {
   Buffer buffer_;
 };
 
-void MakeMessage(uint16 type,
-                 uint8 flags,
-                 uint8 family,
-                 const IPAddressNumber& address,
-                 const IPAddressNumber& local,
-                 Buffer* output) {
+void MakeAddrMessage(uint16 type,
+                     uint8 flags,
+                     uint8 family,
+                     const IPAddressNumber& address,
+                     const IPAddressNumber& local,
+                     Buffer* output) {
   NetlinkMessage nlmsg(type);
   struct ifaddrmsg msg = {};
   msg.ifa_family = family;
@@ -101,6 +108,16 @@ void MakeMessage(uint16 type,
   nlmsg.AppendTo(output);
 }
 
+void MakeLinkMessage(uint16 type, uint32 flags, uint32 index, Buffer* output) {
+  NetlinkMessage nlmsg(type);
+  struct ifinfomsg msg = {};
+  msg.ifi_index = index;
+  msg.ifi_flags = flags;
+  nlmsg.AddPayload(&msg, sizeof(msg));
+  output->clear();
+  nlmsg.AppendTo(output);
+}
+
 const unsigned char kAddress0[] = { 127, 0, 0, 1 };
 const unsigned char kAddress1[] = { 10, 0, 0, 1 };
 const unsigned char kAddress2[] = { 192, 168, 0, 1 };
@@ -115,18 +132,38 @@ TEST_F(AddressTrackerLinuxTest, NewAddress) {
   const IPAddressNumber kAddr1(kAddress1, kAddress1 + arraysize(kAddress1));
   const IPAddressNumber kAddr2(kAddress2, kAddress2 + arraysize(kAddress2));
   const IPAddressNumber kAddr3(kAddress3, kAddress3 + arraysize(kAddress3));
+  bool address_changed;
+  bool link_changed;
 
   Buffer buffer;
-  MakeMessage(RTM_NEWADDR, IFA_F_TEMPORARY, AF_INET, kAddr0, kEmpty, &buffer);
-  EXPECT_TRUE(HandleMessage(&buffer[0], buffer.size()));
+  MakeAddrMessage(RTM_NEWADDR,

[szym, 2012/11/21 20:14:10]

This does not have to be one arg per line. I think two lines would do just fine.

+                  IFA_F_TEMPORARY,
+                  AF_INET,
+                  kAddr0,
+                  kEmpty,
+                  &buffer);
+  address_changed = false;
+  link_changed = false;
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_TRUE(address_changed);
+  EXPECT_FALSE(link_changed);
   AddressTrackerLinux::AddressMap map = GetAddressMap();
   EXPECT_EQ(1u, map.size());
   EXPECT_TRUE(map.find(kAddr0) != map.end());
   EXPECT_EQ(IFA_F_TEMPORARY, map[kAddr0].ifa_flags);
 
   buffer.clear();
-  MakeMessage(RTM_NEWADDR, IFA_F_HOMEADDRESS, AF_INET, kAddr1, kAddr2, &buffer);
-  EXPECT_TRUE(HandleMessage(&buffer[0], buffer.size()));
+  MakeAddrMessage(RTM_NEWADDR,
+                  IFA_F_HOMEADDRESS,
+                  AF_INET,
+                  kAddr1,
+                  kAddr2,
+                  &buffer);
+  address_changed = false;

[szym, 2012/11/21 20:14:10]

Suggest adding two helper functions to the fixture instead of the current
HandleMessage proxy:

bool CheckAddrChange(char* buffer, size_t size) {
  bool address_changed;
  bool link_changed;
  HandleMessage(buffer, size, &address_changed, &link_changed);
  EXPECT_FALSE(link_changed);
  return address_changed;
}

bool CheckLinkChange(...)

Failures are not going to be as informative as before, but it should suffice.

In fact, I suggest CheckAddrChange(const Buffer& buffer);

+  link_changed = false;
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_TRUE(address_changed);
+  EXPECT_FALSE(link_changed);
   map = GetAddressMap();
   EXPECT_EQ(2u, map.size());
   EXPECT_TRUE(map.find(kAddr0) != map.end());
@@ -134,8 +171,12 @@ TEST_F(AddressTrackerLinuxTest, NewAddress) {
   EXPECT_EQ(IFA_F_HOMEADDRESS, map[kAddr2].ifa_flags);
 
   buffer.clear();
-  MakeMessage(RTM_NEWADDR, 0, AF_INET6, kEmpty, kAddr3, &buffer);
-  EXPECT_TRUE(HandleMessage(&buffer[0], buffer.size()));
+  MakeAddrMessage(RTM_NEWADDR, 0, AF_INET6, kEmpty, kAddr3, &buffer);
+  address_changed = false;
+  link_changed = false;
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_TRUE(address_changed);
+  EXPECT_FALSE(link_changed);
   map = GetAddressMap();
   EXPECT_EQ(3u, map.size());
   EXPECT_TRUE(map.find(kAddr3) != map.end());
@@ -144,18 +185,38 @@ TEST_F(AddressTrackerLinuxTest, NewAddress) {
 TEST_F(AddressTrackerLinuxTest, NewAddressChange) {
   const IPAddressNumber kEmpty;
   const IPAddressNumber kAddr0(kAddress0, kAddress0 + arraysize(kAddress0));
+  bool address_changed;
+  bool link_changed;
 
   Buffer buffer;
-  MakeMessage(RTM_NEWADDR, IFA_F_TEMPORARY, AF_INET, kAddr0, kEmpty, &buffer);
-  EXPECT_TRUE(HandleMessage(&buffer[0], buffer.size()));
+  MakeAddrMessage(RTM_NEWADDR,
+                  IFA_F_TEMPORARY,
+                  AF_INET,
+                  kAddr0,
+                  kEmpty,
+                  &buffer);
+  address_changed = false;
+  link_changed = false;
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_TRUE(address_changed);
+  EXPECT_FALSE(link_changed);
   AddressTrackerLinux::AddressMap map = GetAddressMap();
   EXPECT_EQ(1u, map.size());
   EXPECT_TRUE(map.find(kAddr0) != map.end());
   EXPECT_EQ(IFA_F_TEMPORARY, map[kAddr0].ifa_flags);
 
   buffer.clear();
-  MakeMessage(RTM_NEWADDR, IFA_F_HOMEADDRESS, AF_INET, kAddr0, kEmpty, &buffer);
-  EXPECT_TRUE(HandleMessage(&buffer[0], buffer.size()));
+  MakeAddrMessage(RTM_NEWADDR,
+                  IFA_F_HOMEADDRESS,
+                  AF_INET,
+                  kAddr0,
+                  kEmpty,
+                  &buffer);
+  address_changed = false;
+  link_changed = false;
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_TRUE(address_changed);
+  EXPECT_FALSE(link_changed);
   map = GetAddressMap();
   EXPECT_EQ(1u, map.size());
   EXPECT_TRUE(map.find(kAddr0) != map.end());
@@ -163,9 +224,23 @@ TEST_F(AddressTrackerLinuxTest, NewAddressChange) {
 
   // Both messages in one buffer.
   buffer.clear();
-  MakeMessage(RTM_NEWADDR, IFA_F_TEMPORARY, AF_INET, kAddr0, kEmpty, &buffer);
-  MakeMessage(RTM_NEWADDR, IFA_F_HOMEADDRESS, AF_INET, kAddr0, kEmpty, &buffer);
-  EXPECT_TRUE(HandleMessage(&buffer[0], buffer.size()));
+  MakeAddrMessage(RTM_NEWADDR,
+                  IFA_F_TEMPORARY,
+                  AF_INET,
+                  kAddr0,
+                  kEmpty,
+                  &buffer);
+  MakeAddrMessage(RTM_NEWADDR,
+                  IFA_F_HOMEADDRESS,
+                  AF_INET,
+                  kAddr0,
+                  kEmpty,
+                  &buffer);
+  address_changed = false;
+  link_changed = false;
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_TRUE(address_changed);
+  EXPECT_FALSE(link_changed);
   map = GetAddressMap();
   EXPECT_EQ(1u, map.size());
   EXPECT_EQ(IFA_F_HOMEADDRESS, map[kAddr0].ifa_flags);
@@ -173,16 +248,31 @@ TEST_F(AddressTrackerLinuxTest, NewAddressChange) {
 
 TEST_F(AddressTrackerLinuxTest, NewAddressDuplicate) {
   const IPAddressNumber kAddr0(kAddress0, kAddress0 + arraysize(kAddress0));
+  bool address_changed;
+  bool link_changed;
 
   Buffer buffer;
-  MakeMessage(RTM_NEWADDR, IFA_F_TEMPORARY, AF_INET, kAddr0, kAddr0, &buffer);
-  EXPECT_TRUE(HandleMessage(&buffer[0], buffer.size()));
+  MakeAddrMessage(RTM_NEWADDR,
+                  IFA_F_TEMPORARY,
+                  AF_INET,
+                  kAddr0,
+                  kAddr0,
+                  &buffer);
+  address_changed = false;
+  link_changed = false;
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_TRUE(address_changed);
+  EXPECT_FALSE(link_changed);
   AddressTrackerLinux::AddressMap map = GetAddressMap();
   EXPECT_EQ(1u, map.size());
   EXPECT_TRUE(map.find(kAddr0) != map.end());
   EXPECT_EQ(IFA_F_TEMPORARY, map[kAddr0].ifa_flags);
 
-  EXPECT_FALSE(HandleMessage(&buffer[0], buffer.size()));
+  address_changed = false;
+  link_changed = false;
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_FALSE(address_changed);
+  EXPECT_FALSE(link_changed);
   map = GetAddressMap();
   EXPECT_EQ(1u, map.size());
   EXPECT_EQ(IFA_F_TEMPORARY, map[kAddr0].ifa_flags);
@@ -193,32 +283,50 @@ TEST_F(AddressTrackerLinuxTest, DeleteAddress) {
   const IPAddressNumber kAddr0(kAddress0, kAddress0 + arraysize(kAddress0));
   const IPAddressNumber kAddr1(kAddress1, kAddress1 + arraysize(kAddress1));
   const IPAddressNumber kAddr2(kAddress2, kAddress2 + arraysize(kAddress2));
+  bool address_changed;
+  bool link_changed;
 
   Buffer buffer;
-  MakeMessage(RTM_NEWADDR, 0, AF_INET, kAddr0, kEmpty, &buffer);
-  MakeMessage(RTM_NEWADDR, 0, AF_INET, kAddr1, kAddr2, &buffer);
-  EXPECT_TRUE(HandleMessage(&buffer[0], buffer.size()));
+  MakeAddrMessage(RTM_NEWADDR, 0, AF_INET, kAddr0, kEmpty, &buffer);
+  MakeAddrMessage(RTM_NEWADDR, 0, AF_INET, kAddr1, kAddr2, &buffer);
+  address_changed = false;
+  link_changed = false;
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_TRUE(address_changed);
+  EXPECT_FALSE(link_changed);
   AddressTrackerLinux::AddressMap map = GetAddressMap();
   EXPECT_EQ(2u, map.size());
 
   buffer.clear();
-  MakeMessage(RTM_DELADDR, 0, AF_INET, kEmpty, kAddr0, &buffer);
-  EXPECT_TRUE(HandleMessage(&buffer[0], buffer.size()));
+  MakeAddrMessage(RTM_DELADDR, 0, AF_INET, kEmpty, kAddr0, &buffer);
+  address_changed = false;
+  link_changed = false;
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_TRUE(address_changed);
+  EXPECT_FALSE(link_changed);
   map = GetAddressMap();
   EXPECT_EQ(1u, map.size());
   EXPECT_TRUE(map.find(kAddr0) == map.end());
   EXPECT_TRUE(map.find(kAddr2) != map.end());
 
   buffer.clear();
-  MakeMessage(RTM_DELADDR, 0, AF_INET, kAddr2, kAddr1, &buffer);
+  MakeAddrMessage(RTM_DELADDR, 0, AF_INET, kAddr2, kAddr1, &buffer);
   // kAddr1 does not exist in the map.
-  EXPECT_FALSE(HandleMessage(&buffer[0], buffer.size()));
+  address_changed = false;
+  link_changed = false;
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_FALSE(address_changed);
+  EXPECT_FALSE(link_changed);
   map = GetAddressMap();
   EXPECT_EQ(1u, map.size());
 
   buffer.clear();
-  MakeMessage(RTM_DELADDR, 0, AF_INET, kAddr2, kEmpty, &buffer);
-  EXPECT_TRUE(HandleMessage(&buffer[0], buffer.size()));
+  MakeAddrMessage(RTM_DELADDR, 0, AF_INET, kAddr2, kEmpty, &buffer);
+  address_changed = false;
+  link_changed = false;
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_TRUE(address_changed);
+  EXPECT_FALSE(link_changed);
   map = GetAddressMap();
   EXPECT_EQ(0u, map.size());
 }
@@ -227,15 +335,21 @@ TEST_F(AddressTrackerLinuxTest, IgnoredMessage) {
   const IPAddressNumber kEmpty;
   const IPAddressNumber kAddr0(kAddress0, kAddress0 + arraysize(kAddress0));
   const IPAddressNumber kAddr3(kAddress3, kAddress3 + arraysize(kAddress3));
+  bool address_changed;
+  bool link_changed;
 
   Buffer buffer;
   // Ignored family.
-  MakeMessage(RTM_NEWADDR, 0, AF_UNSPEC, kAddr3, kAddr0, &buffer);
+  MakeAddrMessage(RTM_NEWADDR, 0, AF_UNSPEC, kAddr3, kAddr0, &buffer);
   // No address.
-  MakeMessage(RTM_NEWADDR, 0, AF_INET, kEmpty, kEmpty, &buffer);
+  MakeAddrMessage(RTM_NEWADDR, 0, AF_INET, kEmpty, kEmpty, &buffer);
   // Ignored type.
-  MakeMessage(RTM_DELROUTE, 0, AF_INET6, kAddr3, kEmpty, &buffer);
-  EXPECT_FALSE(HandleMessage(&buffer[0], buffer.size()));
+  MakeAddrMessage(RTM_DELROUTE, 0, AF_INET6, kAddr3, kEmpty, &buffer);
+  address_changed = false;
+  link_changed = false;
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_FALSE(address_changed);
+  EXPECT_FALSE(link_changed);
   EXPECT_EQ(0u, GetAddressMap().size());
 
   // Valid message after ignored messages.
@@ -249,10 +363,157 @@ TEST_F(AddressTrackerLinuxTest, IgnoredMessage) {
   nlmsg.AddAttribute(IFA_ADDRESS, &kAddr0[0], kAddr0.size());
   nlmsg.AppendTo(&buffer);
 
-  EXPECT_TRUE(HandleMessage(&buffer[0], buffer.size()));
+  address_changed = false;
+  link_changed = false;
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_TRUE(address_changed);
+  EXPECT_FALSE(link_changed);
   EXPECT_EQ(1u, GetAddressMap().size());
 }
 
+TEST_F(AddressTrackerLinuxTest, AddInterface) {
+  Buffer buffer;
+  bool address_changed;
+  bool link_changed;
+
+  // Ignores loopback
+  MakeLinkMessage(RTM_NEWLINK,
+                  IFF_LOOPBACK | IFF_UP | IFF_LOWER_UP | IFF_RUNNING,
+                  0,
+                  &buffer);
+  address_changed = false;
+  link_changed = false;
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_FALSE(address_changed);
+  EXPECT_FALSE(link_changed);
+  EXPECT_TRUE(GetOnlineLinks()->empty());
+
+  // Ignores not IFF_LOWER_UP
+  MakeLinkMessage(RTM_NEWLINK,
+                  IFF_UP | IFF_RUNNING,
+                  0,
+                  &buffer);
+  address_changed = false;
+  link_changed = false;
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_FALSE(address_changed);
+  EXPECT_FALSE(link_changed);
+  EXPECT_TRUE(GetOnlineLinks()->empty());
+
+  // Ignores deletion
+  MakeLinkMessage(RTM_DELLINK,
+                  IFF_UP | IFF_LOWER_UP | IFF_RUNNING,
+                  0,
+                  &buffer);
+  address_changed = false;
+  link_changed = false;
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_FALSE(address_changed);
+  EXPECT_FALSE(link_changed);
+  EXPECT_TRUE(GetOnlineLinks()->empty());
+
+  // Verify success
+  MakeLinkMessage(RTM_NEWLINK,
+                  IFF_UP | IFF_LOWER_UP | IFF_RUNNING,
+                  0,
+                  &buffer);
+  address_changed = false;
+  link_changed = false;
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_FALSE(address_changed);
+  EXPECT_TRUE(link_changed);
+  EXPECT_TRUE(GetOnlineLinks()->find(0) != GetOnlineLinks()->end());

[szym, 2012/11/21 20:14:10]

Suggest: |count(value) != 0|

+
+  // Ignores redundant enables
+  address_changed = false;
+  link_changed = false;
+  MakeLinkMessage(RTM_NEWLINK,
+                  IFF_UP | IFF_LOWER_UP | IFF_RUNNING,
+                  0,
+                  &buffer);
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_FALSE(address_changed);
+  EXPECT_FALSE(link_changed);
+  EXPECT_TRUE(GetOnlineLinks()->find(0) != GetOnlineLinks()->end());

[szym, 2012/11/21 20:14:10]

Suggest also EXPECT_EQ(1, GetOnlineLinks()->size());

+
+  // Verify adding another online device (e.g. VPN) is considered a change
+  address_changed = false;
+  link_changed = false;
+  MakeLinkMessage(RTM_NEWLINK,
+                  IFF_UP | IFF_LOWER_UP | IFF_RUNNING,
+                  1,
+                  &buffer);
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_FALSE(address_changed);
+  EXPECT_TRUE(link_changed);
+  EXPECT_TRUE(GetOnlineLinks()->find(0) != GetOnlineLinks()->end());
+  EXPECT_TRUE(GetOnlineLinks()->find(1) != GetOnlineLinks()->end());

[szym, 2012/11/21 20:14:10]

Suggest also EXPECT_EQ(2, GetOnlineLinks()->size())

+}
+
+TEST_F(AddressTrackerLinuxTest, RemoveInterface) {
+  Buffer buffer;
+  bool address_changed;
+  bool link_changed;
+
+  // Should disappear when not IFF_LOWER_UP
+  MakeLinkMessage(RTM_NEWLINK,
+                  IFF_UP | IFF_LOWER_UP | IFF_RUNNING,
+                  0,
+                  &buffer);
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);

[szym, 2012/11/21 20:14:10]

Add expectation that GetOnlineLinks is not empty.

+  address_changed = false;
+  link_changed = false;
+  MakeLinkMessage(RTM_NEWLINK,
+                  IFF_UP | IFF_RUNNING,
+                  0,
+                  &buffer);
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_FALSE(address_changed);
+  EXPECT_TRUE(link_changed);
+  EXPECT_TRUE(GetOnlineLinks()->empty());
+
+  // Ignores redundant disables
+  address_changed = false;
+  link_changed = false;
+  MakeLinkMessage(RTM_NEWLINK,
+                  IFF_UP | IFF_RUNNING,
+                  0,
+                  &buffer);
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_FALSE(address_changed);
+  EXPECT_FALSE(link_changed);
+  EXPECT_TRUE(GetOnlineLinks()->empty());
+
+  // Ignores deleting down interfaces
+  address_changed = false;
+  link_changed = false;
+  MakeLinkMessage(RTM_DELLINK,
+                  IFF_UP | IFF_RUNNING,
+                  0,
+                  &buffer);
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_FALSE(address_changed);
+  EXPECT_FALSE(link_changed);
+  EXPECT_TRUE(GetOnlineLinks()->empty());
+
+  // Should disappear when deleted
+  MakeLinkMessage(RTM_NEWLINK,
+                  IFF_UP | IFF_LOWER_UP | IFF_RUNNING,
+                  0,
+                  &buffer);
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);

[szym, 2012/11/21 20:14:10]

ditto

+  address_changed = false;
+  link_changed = false;
+  MakeLinkMessage(RTM_DELLINK,
+                  IFF_UP | IFF_LOWER_UP | IFF_RUNNING,
+                  0,
+                  &buffer);
+  HandleMessage(&buffer[0], buffer.size(), &address_changed, &link_changed);
+  EXPECT_FALSE(address_changed);
+  EXPECT_TRUE(link_changed);
+  EXPECT_TRUE(GetOnlineLinks()->empty());
+}
+
 }  // namespace
 
 }  // namespace internal