Move ActiveResolveAttempt types to Variant. (#18429)

* Move ActiveResolveAttempt types to Variant.

Will start expanding with AAAA query support, so switching to a
slightly more extensible way of storing values.

* Fix unit test

src/lib/dnssd/ActiveResolveAttempts.h

@@ -23,6 +23,7 @@
 #include <lib/core/Optional.h>
 #include <lib/core/PeerId.h>
 #include <lib/dnssd/Resolver.h>
+#include <lib/support/Variant.h>
 #include <system/SystemClock.h>
 
 namespace mdns {
@@ -44,44 +45,76 @@ class ActiveResolveAttempts
 
     struct ScheduledAttempt
     {
-        enum AttemptType
+        struct Browse
+        {
+            Browse(const chip::Dnssd::DiscoveryFilter discoveryFilter, const chip::Dnssd::DiscoveryType discoveryType) :
+                filter(discoveryFilter), type(discoveryType)
+            {}
+            chip::Dnssd::DiscoveryFilter filter;
+            chip::Dnssd::DiscoveryType type;
+        };
+
+        struct Resolve
         {
-            kInvalid,
-            kResolve,
-            kBrowse,
+            chip::PeerId peerId;
+
+            Resolve(chip::PeerId id) : peerId(id) {}
         };
 
-        ScheduledAttempt() : attemptType(kInvalid) {}
-        ScheduledAttempt(const chip::PeerId & peer, bool first) : attemptType(kResolve), peerId(peer), firstSend(first) {}
+        ScheduledAttempt() {}
+        ScheduledAttempt(const chip::PeerId & peer, bool first) :
+            resolveData(chip::InPlaceTemplateType<Resolve>(), peer), firstSend(first)
+        {}
         ScheduledAttempt(const chip::Dnssd::DiscoveryFilter discoveryFilter, const chip::Dnssd::DiscoveryType type, bool first) :
-            attemptType(kBrowse), browse(discoveryFilter, type), firstSend(first)
+            resolveData(chip::InPlaceTemplateType<Browse>(), discoveryFilter, type), firstSend(first)
         {}
         bool operator==(const ScheduledAttempt & other) const { return Matches(other) && other.firstSend == firstSend; }
         bool Matches(const ScheduledAttempt & other) const
         {
-            if (other.attemptType != attemptType)
+            if (!resolveData.Valid())
             {
-                return false;
+                return !other.resolveData.Valid();
             }
-            switch (attemptType)
+
+            if (resolveData.Is<Browse>())
             {
-            case kInvalid:
-                return true;
-            case kBrowse:
-                return (other.browse.filter == browse.filter && other.browse.type == browse.type);
-            case kResolve:
-                return other.peerId == peerId;
-            default:
-                return false;
+                if (!other.resolveData.Is<Browse>())
+                {
+                    return false;
+                }
+
+                auto & a = resolveData.Get<Browse>();
+                auto & b = other.resolveData.Get<Browse>();
+                return (a.filter == b.filter && a.type == b.type);
+            }
+
+            if (resolveData.Is<Resolve>())
+            {
+                if (!other.resolveData.Is<Resolve>())
+                {
+                    return false;
+                }
+                auto & a = resolveData.Get<Resolve>();
+                auto & b = other.resolveData.Get<Resolve>();
+
+                return a.peerId == b.peerId;
             }
+            return false;
+        }
+
+        bool Matches(const chip::PeerId & peer) const
+        {
+            return resolveData.Is<Resolve>() && (resolveData.Get<Resolve>().peerId == peer);
         }
-        bool Matches(const chip::PeerId & peer) const { return (attemptType == kResolve) && (peerId == peer); }
         bool Matches(const chip::Dnssd::DiscoveredNodeData & data) const
         {
-            if (attemptType != kBrowse)
+            if (!resolveData.Is<Browse>())
             {
                 return false;
             }
+
+            auto & browse = resolveData.Get<Browse>();
+
             // TODO: we should mark returned node data based on the query
             if (browse.type != chip::Dnssd::DiscoveryType::kCommissionableNode)
             {
@@ -113,26 +146,18 @@ class ActiveResolveAttempts
                 return false;
             }
         }
-        bool IsEmpty() const { return attemptType == kInvalid; }
-        bool IsResolve() const { return attemptType == kResolve; }
-        bool IsBrowse() const { return attemptType == kBrowse; }
-        void Clear() { attemptType = kInvalid; }
+        bool IsEmpty() const { return !resolveData.Valid(); }
+        bool IsResolve() const { return resolveData.Is<Resolve>(); }
+        bool IsBrowse() const { return resolveData.Is<Browse>(); }
+        void Clear() { resolveData = DataType(); }
+
+        const Browse & BrowseData() const { return resolveData.Get<Browse>(); }
+        const Resolve & ResolveData() const { return resolveData.Get<Resolve>(); }
+
+        using DataType = chip::Variant<Browse, Resolve>;
+
+        DataType resolveData;
 
-        // Not using Variant because it assumes a heap impl
-        AttemptType attemptType;
-        struct Browse
-        {
-            Browse(const chip::Dnssd::DiscoveryFilter discoveryFilter, const chip::Dnssd::DiscoveryType discoveryType) :
-                filter(discoveryFilter), type(discoveryType)
-            {}
-            chip::Dnssd::DiscoveryFilter filter;
-            chip::Dnssd::DiscoveryType type;
-        };
-        union
-        {
-            chip::PeerId peerId; // Peer id for resolve attempts
-            Browse browse;
-        };
         // First packet send is marked separately: minMDNS logic can choose
         // to first send a unicast query followed by a multicast one.
         bool firstSend = false;

src/lib/dnssd/Resolver_ImplMinimalMdns.cpp

@@ -528,38 +528,38 @@ CHIP_ERROR MinMdnsResolver::SendPendingBrowseQueries()
 
         mdns::Minimal::FullQName qname;
 
-        switch (attempt.Value().browse.type)
+        switch (attempt.Value().BrowseData().type)
         {
         case DiscoveryType::kOperational:
             qname = CheckAndAllocateQName(kOperationalServiceName, kOperationalProtocol, kLocalDomain);
             break;
         case DiscoveryType::kCommissionableNode:
-            if (attempt.Value().browse.filter.type == DiscoveryFilterType::kNone)
+            if (attempt.Value().BrowseData().filter.type == DiscoveryFilterType::kNone)
             {
                 qname = CheckAndAllocateQName(kCommissionableServiceName, kCommissionProtocol, kLocalDomain);
             }
-            else if (attempt.Value().browse.filter.type == DiscoveryFilterType::kInstanceName)
+            else if (attempt.Value().BrowseData().filter.type == DiscoveryFilterType::kInstanceName)
             {
-                qname = CheckAndAllocateQName(attempt.Value().browse.filter.instanceName, kCommissionableServiceName,
+                qname = CheckAndAllocateQName(attempt.Value().BrowseData().filter.instanceName, kCommissionableServiceName,
                                               kCommissionProtocol, kLocalDomain);
             }
             else
             {
                 char subtypeStr[Common::kSubTypeMaxLength + 1];
-                ReturnErrorOnFailure(MakeServiceSubtype(subtypeStr, sizeof(subtypeStr), attempt.Value().browse.filter));
+                ReturnErrorOnFailure(MakeServiceSubtype(subtypeStr, sizeof(subtypeStr), attempt.Value().BrowseData().filter));
                 qname = CheckAndAllocateQName(subtypeStr, kSubtypeServiceNamePart, kCommissionableServiceName, kCommissionProtocol,
                                               kLocalDomain);
             }
             break;
         case DiscoveryType::kCommissionerNode:
-            if (attempt.Value().browse.filter.type == DiscoveryFilterType::kNone)
+            if (attempt.Value().BrowseData().filter.type == DiscoveryFilterType::kNone)
             {
                 qname = CheckAndAllocateQName(kCommissionerServiceName, kCommissionProtocol, kLocalDomain);
             }
             else
             {
                 char subtypeStr[Common::kSubTypeMaxLength + 1];
-                ReturnErrorOnFailure(MakeServiceSubtype(subtypeStr, sizeof(subtypeStr), attempt.Value().browse.filter));
+                ReturnErrorOnFailure(MakeServiceSubtype(subtypeStr, sizeof(subtypeStr), attempt.Value().BrowseData().filter));
                 qname = CheckAndAllocateQName(subtypeStr, kSubtypeServiceNamePart, kCommissionerServiceName, kCommissionProtocol,
                                               kLocalDomain);
             }
@@ -638,7 +638,7 @@ CHIP_ERROR MinMdnsResolver::SendPendingResolveQueries()
             char nameBuffer[kMaxOperationalServiceNameSize] = "";
 
             // Node and fabricid are encoded in server names.
-            ReturnErrorOnFailure(MakeInstanceName(nameBuffer, sizeof(nameBuffer), resolve.Value().peerId));
+            ReturnErrorOnFailure(MakeInstanceName(nameBuffer, sizeof(nameBuffer), resolve.Value().ResolveData().peerId));
 
             const char * instanceQName[] = { nameBuffer, kOperationalServiceName, kOperationalProtocol, kLocalDomain };
             Query query(instanceQName);

src/lib/dnssd/tests/TestActiveResolveAttempts.cpp

@@ -242,7 +242,7 @@ void TestLRU(nlTestSuite * inSuite, void * inContext)
 
     for (Optional<ActiveResolveAttempts::ScheduledAttempt> s = attempts.NextScheduled(); s.HasValue(); s = attempts.NextScheduled())
     {
-        NL_TEST_ASSERT(inSuite, s.Value().peerId.GetNodeId() != 9999);
+        NL_TEST_ASSERT(inSuite, s.Value().ResolveData().peerId.GetNodeId() != 9999);
     }
 
     // Still have active pending items (queue is full)
@@ -266,7 +266,7 @@ void TestLRU(nlTestSuite * inSuite, void * inContext)
         Optional<ActiveResolveAttempts::ScheduledAttempt> s = attempts.NextScheduled();
         while (s.HasValue())
         {
-            NL_TEST_ASSERT(inSuite, s.Value().peerId.GetNodeId() != 9999);
+            NL_TEST_ASSERT(inSuite, s.Value().ResolveData().peerId.GetNodeId() != 9999);
             s = attempts.NextScheduled();
         }
     }