UdpSocket: Update Read - Update cast/*
This change is a subset of CL:
https://chromium-review.googlesource.com/c/openscreen/+/1764117
This CL makes 2 changes:
1) Update cast/ to use the new callback scheme instead of the old one
2) Add enable_reading() and disable_reading() to UdpSocket
Change-Id: I590548c2fa082c717506ecbdc83c25c40c75480e
Reviewed-on: https://chromium-review.googlesource.com/c/openscreen/+/1764118
Commit-Queue: Ryan Keane <rwkeane@google.com>
Reviewed-by: Max Yakimakha <yakimakha@chromium.org>
Reviewed-by: mark a. foltz <mfoltz@chromium.org>
diff --git a/cast/common/mdns/mdns_receiver.cc b/cast/common/mdns/mdns_receiver.cc
index a097838..e6fa855 100644
--- a/cast/common/mdns/mdns_receiver.cc
+++ b/cast/common/mdns/mdns_receiver.cc
@@ -10,12 +10,9 @@
namespace cast {
namespace mdns {
-MdnsReceiver::MdnsReceiver(UdpSocket* socket,
- NetworkRunner* network_runner,
- Delegate* delegate)
- : socket_(socket), network_runner_(network_runner), delegate_(delegate) {
+MdnsReceiver::MdnsReceiver(UdpSocket* socket, Delegate* delegate)
+ : socket_(socket), delegate_(delegate) {
OSP_DCHECK(socket_);
- OSP_DCHECK(network_runner_);
OSP_DCHECK(delegate_);
}
@@ -25,29 +22,22 @@
}
}
-Error MdnsReceiver::Start() {
- if (state_ == State::kRunning) {
- return Error::Code::kNone;
- }
- Error result = network_runner_->ReadRepeatedly(socket_, this);
- if (result.ok()) {
- state_ = State::kRunning;
- }
- return result;
+void MdnsReceiver::Start() {
+ state_ = State::kRunning;
}
-Error MdnsReceiver::Stop() {
- if (state_ == State::kStopped) {
- return Error::Code::kNone;
- }
- Error result = network_runner_->CancelRead(socket_);
- if (result.ok()) {
- state_ = State::kStopped;
- }
- return result;
+void MdnsReceiver::Stop() {
+ state_ = State::kStopped;
}
-void MdnsReceiver::OnRead(UdpPacket packet, NetworkRunner* network_runner) {
+void MdnsReceiver::OnRead(UdpSocket* socket,
+ openscreen::ErrorOr<UdpPacket> packet_or_error) {
+ if (state_ != State::kRunning || packet_or_error.is_error()) {
+ return;
+ }
+
+ UdpPacket packet = packet_or_error.MoveValue();
+
TRACE_SCOPED(TraceCategory::mDNS, "MdnsReceiver::OnRead");
MdnsReader reader(packet.data(), packet.size());
MdnsMessage message;
@@ -61,5 +51,15 @@
}
}
+void MdnsReceiver::OnError(UdpSocket* socket, Error error) {
+ // This method should never be called for MdnsReciever.
+ OSP_UNIMPLEMENTED();
+}
+
+void MdnsReceiver::OnSendError(UdpSocket* socket, Error error) {
+ // This method should never be called for MdnsReciever.
+ OSP_UNIMPLEMENTED();
+}
+
} // namespace mdns
} // namespace cast
diff --git a/cast/common/mdns/mdns_receiver.h b/cast/common/mdns/mdns_receiver.h
index 62aff21..b2d998e 100644
--- a/cast/common/mdns/mdns_receiver.h
+++ b/cast/common/mdns/mdns_receiver.h
@@ -6,9 +6,8 @@
#define CAST_COMMON_MDNS_MDNS_RECEIVER_H_
#include "cast/common/mdns/mdns_records.h"
-#include "platform/api/network_runner.h"
+#include "platform/api/task_runner.h"
#include "platform/api/udp_packet.h"
-#include "platform/api/udp_read_callback.h"
#include "platform/api/udp_socket.h"
#include "platform/base/error.h"
#include "platform/base/ip_address.h"
@@ -17,13 +16,12 @@
namespace mdns {
using Error = openscreen::Error;
-using NetworkRunner = openscreen::platform::NetworkRunner;
using UdpSocket = openscreen::platform::UdpSocket;
-using UdpReadCallback = openscreen::platform::UdpReadCallback;
using UdpPacket = openscreen::platform::UdpPacket;
+using TaskRunner = openscreen::platform::TaskRunner;
using IPEndpoint = openscreen::IPEndpoint;
-class MdnsReceiver : UdpReadCallback {
+class MdnsReceiver : UdpSocket::Client {
public:
class Delegate {
public:
@@ -34,12 +32,10 @@
const IPEndpoint& sender) = 0;
};
- // MdnsReceiver does not own |socket|, |network_runner| and |delegate|
+ // MdnsReceiver does not own |socket| and |delegate|
// and expects that the lifetime of these objects exceeds the lifetime of
// MdnsReceiver.
- MdnsReceiver(UdpSocket* socket,
- NetworkRunner* network_runner,
- Delegate* delegate);
+ MdnsReceiver(UdpSocket* socket, Delegate* delegate);
MdnsReceiver(const MdnsReceiver& other) = delete;
MdnsReceiver(MdnsReceiver&& other) noexcept = delete;
~MdnsReceiver() override;
@@ -48,16 +44,17 @@
MdnsReceiver& operator=(MdnsReceiver&& other) noexcept = delete;
// The receiver can be started and stopped multiple times.
- // Start and Stop return Error::Code::kNone on success and return an error on
- // failure. Start returns Error::Code::kNone when called on a receiver that
- // has already been started. Stop returns Error::Code::kNone when called on a
- // receiver that has already been stopped or not yet started. Start and Stop
- // are both synchronous calls. After MdnsReceiver has been started it will
- // receive OnRead callbacks from the network runner.
- Error Start();
- Error Stop();
+ // Start and Stop are both synchronous calls. When MdnsReceiver has not yet
+ // been started, OnRead callbacks it receives from the task runner will be
+ // no-ops.
+ void Start();
+ void Stop();
- void OnRead(UdpPacket packet, NetworkRunner* network_runner) override;
+ // UdpSocket::Client overrides.
+ void OnRead(UdpSocket* socket,
+ openscreen::ErrorOr<UdpPacket> packet) override;
+ void OnError(UdpSocket* socket, Error error) override;
+ void OnSendError(UdpSocket* socket, Error error) override;
private:
enum class State {
@@ -66,7 +63,6 @@
};
UdpSocket* const socket_;
- NetworkRunner* const network_runner_;
Delegate* const delegate_;
State state_ = State::kStopped;
};
diff --git a/cast/common/mdns/mdns_receiver_unittest.cc b/cast/common/mdns/mdns_receiver_unittest.cc
index ce796dd..01fb779 100644
--- a/cast/common/mdns/mdns_receiver_unittest.cc
+++ b/cast/common/mdns/mdns_receiver_unittest.cc
@@ -16,18 +16,6 @@
using ::testing::Return;
using MockUdpSocket = openscreen::platform::MockUdpSocket;
-// TODO(yakimakha): Update tests to use a fake NetworkRunner when implemented
-class MockNetworkRunner : public NetworkRunner {
- public:
- MOCK_METHOD2(ReadRepeatedly, Error(UdpSocket*, UdpReadCallback*));
- MOCK_METHOD1(CancelRead, Error(UdpSocket*));
-
- void PostPackagedTask(Task task) override {}
- void PostPackagedTaskWithDelay(
- Task task,
- openscreen::platform::Clock::duration delay) override {}
-};
-
class MockMdnsReceiverDelegate : public MdnsReceiver::Delegate {
public:
MOCK_METHOD2(OnQueryReceived, void(const MdnsMessage&, const IPEndpoint&));
@@ -54,12 +42,8 @@
std::unique_ptr<openscreen::platform::MockUdpSocket> socket_info =
MockUdpSocket::CreateDefault(openscreen::IPAddress::Version::kV4);
- MockNetworkRunner runner;
MockMdnsReceiverDelegate delegate;
- MdnsReceiver receiver(socket_info.get(), &runner, &delegate);
-
- EXPECT_CALL(runner, ReadRepeatedly(socket_info.get(), _))
- .WillOnce(Return(Error::Code::kNone));
+ MdnsReceiver receiver(socket_info.get(), &delegate);
receiver.Start();
MdnsQuestion question(DomainName{"testing", "local"}, DnsType::kA,
@@ -77,10 +61,8 @@
// Imitate a call to OnRead from NetworkRunner by calling it manually here
EXPECT_CALL(delegate, OnQueryReceived(message, packet.source())).Times(1);
- receiver.OnRead(std::move(packet), &runner);
+ receiver.OnRead(socket_info.get(), std::move(packet));
- EXPECT_CALL(runner, CancelRead(socket_info.get()))
- .WillOnce(Return(Error::Code::kNone));
receiver.Stop();
}
@@ -112,12 +94,8 @@
std::unique_ptr<openscreen::platform::MockUdpSocket> socket_info =
MockUdpSocket::CreateDefault(openscreen::IPAddress::Version::kV6);
- MockNetworkRunner runner;
MockMdnsReceiverDelegate delegate;
- MdnsReceiver receiver(socket_info.get(), &runner, &delegate);
-
- EXPECT_CALL(runner, ReadRepeatedly(socket_info.get(), _))
- .WillOnce(Return(Error::Code::kNone));
+ MdnsReceiver receiver(socket_info.get(), &delegate);
receiver.Start();
MdnsRecord record(DomainName{"testing", "local"}, DnsType::kA, DnsClass::kIN,
@@ -137,10 +115,8 @@
// Imitate a call to OnRead from NetworkRunner by calling it manually here
EXPECT_CALL(delegate, OnResponseReceived(message, packet.source())).Times(1);
- receiver.OnRead(std::move(packet), &runner);
+ receiver.OnRead(socket_info.get(), std::move(packet));
- EXPECT_CALL(runner, CancelRead(socket_info.get()))
- .WillOnce(Return(Error::Code::kNone));
receiver.Stop();
}
