shill: Adds test for multi-part netlink message handling.
BUG=chromium:224652
TEST=unittest
Change-Id: I7e7ae7e974fe69de67df7a5c84856a8f778fc077
Reviewed-on: https://gerrit.chromium.org/gerrit/56088
Commit-Queue: Wade Guthrie <wdg@chromium.org>
Reviewed-by: Wade Guthrie <wdg@chromium.org>
Tested-by: Wade Guthrie <wdg@chromium.org>
diff --git a/netlink_manager.h b/netlink_manager.h
index fc53984..d324cd0 100644
--- a/netlink_manager.h
+++ b/netlink_manager.h
@@ -189,10 +189,11 @@
friend class NetlinkMessageTest;
friend class ShillDaemonTest;
FRIEND_TEST(NetlinkManagerTest, AddLinkTest);
- FRIEND_TEST(NetlinkManagerTest, BroadcastHandlerTest);
+ FRIEND_TEST(NetlinkManagerTest, BroadcastHandler);
FRIEND_TEST(NetlinkManagerTest, GetFamilyOneInterstitialMessage);
FRIEND_TEST(NetlinkManagerTest, GetFamilyTimeout);
- FRIEND_TEST(NetlinkManagerTest, MessageHandlerTest);
+ FRIEND_TEST(NetlinkManagerTest, MessageHandler);
+ FRIEND_TEST(NetlinkManagerTest, MultipartMessageHandler);
FRIEND_TEST(NetlinkMessageTest, Parse_NL80211_CMD_TRIGGER_SCAN);
FRIEND_TEST(NetlinkMessageTest, Parse_NL80211_CMD_NEW_SCAN_RESULTS);
FRIEND_TEST(NetlinkMessageTest, Parse_NL80211_CMD_NEW_STATION);
diff --git a/netlink_manager_unittest.cc b/netlink_manager_unittest.cc
index 2f5b0a6..9ac4b6d 100644
--- a/netlink_manager_unittest.cc
+++ b/netlink_manager_unittest.cc
@@ -20,6 +20,7 @@
#include "shill/mock_time.h"
#include "shill/netlink_attribute.h"
#include "shill/nl80211_message.h"
+#include "shill/scope_logger.h"
using base::Bind;
using base::Unretained;
@@ -205,7 +206,6 @@
} // namespace
-// TODO(wdg): Add a test for multi-part messages. crbug.com/224652
// TODO(wdg): Add a test for SubscribeToEvents (verify that it handles bad input
// appropriately, and that it calls NetlinkSocket::SubscribeToEvents if input
// is good.)
@@ -299,7 +299,7 @@
netlink_manager_->GetFamily(kSampleMessageName, null_factory));
}
-TEST_F(NetlinkManagerTest, BroadcastHandlerTest) {
+TEST_F(NetlinkManagerTest, BroadcastHandler) {
nlmsghdr *message = const_cast<nlmsghdr *>(
reinterpret_cast<const nlmsghdr *>(kNL80211_CMD_DISCONNECT));
@@ -347,7 +347,7 @@
netlink_manager_->OnNlMessageReceived(message);
}
-TEST_F(NetlinkManagerTest, MessageHandlerTest) {
+TEST_F(NetlinkManagerTest, MessageHandler) {
Reset();
MockHandler80211 handler_broadcast;
EXPECT_TRUE(netlink_manager_->AddBroadcastHandler(
@@ -413,4 +413,54 @@
netlink_manager_->OnNlMessageReceived(received_message);
}
+TEST_F(NetlinkManagerTest, MultipartMessageHandler) {
+ Reset();
+
+ // Install a broadcast handler.
+ MockHandler80211 broadcast_handler;
+ EXPECT_TRUE(netlink_manager_->AddBroadcastHandler(
+ broadcast_handler.on_netlink_message()));
+
+ // Build a message and send it in order to install a response handler.
+ TriggerScanMessage trigger_scan_message;
+ MockHandler80211 response_handler;
+ EXPECT_CALL(netlink_socket_, SendMessage(_)).WillOnce(Return(true));
+ EXPECT_TRUE(netlink_manager_->SendMessage(
+ &trigger_scan_message, response_handler.on_netlink_message()));
+
+ // Build a multi-part response (well, it's just one message but it'll be
+ // received multiple times).
+ const uint32_t kSequenceNumber = 32; // Arbitrary (replaced, later).
+ NewScanResultsMessage new_scan_results;
+ new_scan_results.AddFlag(NLM_F_MULTI);
+ ByteString new_scan_results_bytes(new_scan_results.Encode(kSequenceNumber));
+ nlmsghdr *received_message =
+ reinterpret_cast<nlmsghdr *>(new_scan_results_bytes.GetData());
+ received_message->nlmsg_seq = netlink_socket_.GetLastSequenceNumber();
+
+ // Verify that the message-specific handler is called.
+ EXPECT_CALL(response_handler, OnNetlinkMessage(_));
+ netlink_manager_->OnNlMessageReceived(received_message);
+
+ // Verify that the message-specific handler is still called.
+ EXPECT_CALL(response_handler, OnNetlinkMessage(_));
+ netlink_manager_->OnNlMessageReceived(received_message);
+
+ // Build a Done message with the sent-message sequence number.
+ DoneMessage done_message;
+ ByteString done_message_bytes(
+ done_message.Encode(netlink_socket_.GetLastSequenceNumber()));
+
+ // Verify that the message-specific handler is called for the done message.
+ EXPECT_CALL(response_handler, OnNetlinkMessage(_));
+ netlink_manager_->OnNlMessageReceived(
+ reinterpret_cast<nlmsghdr *>(done_message_bytes.GetData()));
+
+ // Verify that broadcast handler is called now that the done message has
+ // been seen.
+ EXPECT_CALL(response_handler, OnNetlinkMessage(_)).Times(0);
+ EXPECT_CALL(broadcast_handler, OnNetlinkMessage(_)).Times(1);
+ netlink_manager_->OnNlMessageReceived(received_message);
+}
+
} // namespace shill
diff --git a/netlink_message.cc b/netlink_message.cc
index dfb75af..9d0ba3b 100644
--- a/netlink_message.cc
+++ b/netlink_message.cc
@@ -200,9 +200,7 @@
const uint16_t DoneMessage::kMessageType = NLMSG_DONE;
ByteString DoneMessage::Encode(uint32_t sequence_number) {
- LOG(ERROR)
- << "We're not supposed to send Done messages (are we?) to the kernel";
- return ByteString();
+ return EncodeHeader(sequence_number);
}
void DoneMessage::Print(int header_log_level, int /*detail_log_level*/) const {