Bug 1567201 - Use Maybe rather than empty string for timedout queries; r=ng

Differential Revision: https://phabricator.services.mozilla.com/D47266

--HG--
extra : moz-landing-system : lando

media/mtransport/ipc/PStunAddrsParams.h

@@ -5,6 +5,7 @@
 #ifndef PStunAddrsParams_h
 #define PStunAddrsParams_h
 
+#include "mozilla/Maybe.h"
 #include "nsTArray.h"
 
 #ifdef MOZ_WEBRTC
@@ -24,6 +25,8 @@ typedef nsTArray<NrIceStunAddr> NrIceStunAddrArray;
 typedef nsTArray<int> NrIceStunAddrArray;
 #endif
 
+typedef Maybe<nsCString> MaybeNsCString;
+
 }  // namespace net
 }  // namespace mozilla
 

media/mtransport/ipc/PStunAddrsRequest.ipdl

@@ -5,6 +5,7 @@
 include protocol PNecko;
 
 using NrIceStunAddrArray from "mozilla/net/PStunAddrsParams.h";
+using MaybeNsCString from "mozilla/net/PStunAddrsParams.h";
 
 include "mozilla/net/NrIceStunAddrMessageUtils.h";
 
@@ -25,7 +26,7 @@ parent:
   async __delete__();
 
 child:
-  async OnMDNSQueryComplete(nsCString hostname, nsCString address);
+  async OnMDNSQueryComplete(nsCString hostname, MaybeNsCString address);
   async OnStunAddrsAvailable(NrIceStunAddrArray iceStunAddrs);
 };
 

media/mtransport/ipc/StunAddrsRequestChild.cpp

@@ -25,7 +25,7 @@ StunAddrsRequestChild::StunAddrsRequestChild(
 }
 
 mozilla::ipc::IPCResult StunAddrsRequestChild::RecvOnMDNSQueryComplete(
-    const nsCString& hostname, const nsCString& address) {
+    const nsCString& hostname, const Maybe<nsCString>& address) {
   if (mListener) {
     mListener->OnMDNSQueryComplete(hostname, address);
   }

media/mtransport/ipc/StunAddrsRequestChild.h

@@ -15,7 +15,7 @@ namespace net {
 class StunAddrsListener {
  public:
   virtual void OnMDNSQueryComplete(const nsCString& hostname,
-                                   const nsCString& address) = 0;
+                                   const Maybe<nsCString>& address) = 0;
   virtual void OnStunAddrsAvailable(const NrIceStunAddrArray& addrs) = 0;
 
   NS_IMETHOD_(MozExternalRefCountType) AddRef();
@@ -50,7 +50,7 @@ class StunAddrsRequestChild final : public PStunAddrsRequestChild {
   virtual ~StunAddrsRequestChild() {}
 
   virtual mozilla::ipc::IPCResult RecvOnMDNSQueryComplete(
-      const nsCString& aHostname, const nsCString& aAddress) override;
+      const nsCString& aHostname, const Maybe<nsCString>& aAddress) override;
 
   virtual mozilla::ipc::IPCResult RecvOnStunAddrsAvailable(
       const NrIceStunAddrArray& addrs) override;

media/mtransport/ipc/StunAddrsRequestParent.cpp

@@ -25,7 +25,12 @@ namespace net {
 static void mdns_service_resolved(void* cb, const char* hostname,
                                   const char* addr) {
   StunAddrsRequestParent* self = static_cast<StunAddrsRequestParent*>(cb);
-  self->OnQueryComplete(nsCString(hostname), nsCString(addr));
+  self->OnQueryComplete(nsCString(hostname), Some(nsCString(addr)));
+}
+
+void mdns_service_timedout(void* cb, const char* hostname) {
+  StunAddrsRequestParent* self = static_cast<StunAddrsRequestParent*>(cb);
+  self->OnQueryComplete(nsCString(hostname), Nothing());
 }
 
 StunAddrsRequestParent::StunAddrsRequestParent() : mIPCClosed(false) {
@@ -108,7 +113,7 @@ mozilla::ipc::IPCResult StunAddrsRequestParent::Recv__delete__() {
 }
 
 void StunAddrsRequestParent::OnQueryComplete(const nsCString& hostname,
-                                             const nsCString& address) {
+                                             const Maybe<nsCString>& address) {
   RUN_ON_THREAD(mMainThread,
                 WrapRunnable(RefPtr<StunAddrsRequestParent>(this),
                              &StunAddrsRequestParent::OnQueryComplete_m,
@@ -191,8 +196,8 @@ void StunAddrsRequestParent::SendStunAddrs_m(const NrIceStunAddrArray& addrs) {
   Unused << SendOnStunAddrsAvailable(addrs);
 }
 
-void StunAddrsRequestParent::OnQueryComplete_m(const nsCString& hostname,
-                                               const nsCString& address) {
+void StunAddrsRequestParent::OnQueryComplete_m(
+    const nsCString& hostname, const Maybe<nsCString>& address) {
   ASSERT_ON_THREAD(mMainThread);
 
   if (mIPCClosed) {
@@ -227,7 +232,7 @@ void StunAddrsRequestParent::MDNSServiceWrapper::QueryHostname(
   StartIfRequired();
   if (mMDNSService) {
     mdns_service_query_hostname(mMDNSService, data, mdns_service_resolved,
-                                hostname);
+                                mdns_service_timedout, hostname);
   }
 }
 

media/mtransport/ipc/StunAddrsRequestParent.h

@@ -26,7 +26,8 @@ class StunAddrsRequestParent : public PStunAddrsRequestParent {
 
   mozilla::ipc::IPCResult Recv__delete__() override;
 
-  void OnQueryComplete(const nsCString& hostname, const nsCString& address);
+  void OnQueryComplete(const nsCString& hostname,
+                       const Maybe<nsCString>& address);
 
  protected:
   virtual ~StunAddrsRequestParent();
@@ -46,7 +47,8 @@ class StunAddrsRequestParent : public PStunAddrsRequestParent {
   void GetStunAddrs_s();
   void SendStunAddrs_m(const NrIceStunAddrArray& addrs);
 
-  void OnQueryComplete_m(const nsCString& hostname, const nsCString& address);
+  void OnQueryComplete_m(const nsCString& hostname,
+                         const Maybe<nsCString>& address);
 
   ThreadSafeAutoRefCnt mRefCnt;
   NS_DECL_OWNINGTHREAD

media/mtransport/mdns_service/mdns_service.h

@@ -14,11 +14,10 @@ MDNSService* mdns_service_start(const char* ifaddr);
 
 void mdns_service_stop(MDNSService* serv);
 
-void mdns_service_query_hostname(MDNSService* serv, void* data,
-                                 void (*resolved)(void* data,
-                                                  const char* hostname,
-                                                  const char* address),
-                                 const char* hostname);
+void mdns_service_query_hostname(
+    MDNSService* serv, void* data,
+    void (*resolved)(void* data, const char* hostname, const char* address),
+    void (*timedout)(void* data, const char* hostname), const char* hostname);
 
 void mdns_service_unregister_hostname(MDNSService* serv, const char* hostname);
 

media/mtransport/mdns_service/src/lib.rs

@@ -21,6 +21,7 @@ extern crate log;
 struct Callback {
     data: *const c_void,
     resolved: extern "C" fn(*const c_void, *const c_char, *const c_char),
+    timedout: extern "C" fn(*const c_void, *const c_char),
 }
 
 unsafe impl Send for Callback {}
@@ -33,6 +34,12 @@ fn hostname_resolved(callback: &Callback, hostname: &str, addr: &str) {
     }
 }
 
+fn hostname_timedout(callback: &Callback, hostname: &str) {
+    if let Ok(hostname) = CString::new(hostname) {
+        (callback.timedout)(callback.data, hostname.as_ptr());
+    }
+}
+
 // This code is derived from code for creating questions in the dns-parser
 // crate. It would be nice to upstream this, or something similar.
 fn create_answer(id: u16, answers: &Vec<(String, &[u8])>) -> Result<Vec<u8>, io::Error> {
@@ -366,7 +373,7 @@ impl MDNSService {
                             query.timestamp = now;
                             unsent_queries.push_back(query);
                         } else {
-                            hostname_resolved(&query.callback, &hostname, "");
+                            hostname_timedout(&query.callback, &hostname);
                         }
                     }
                 }
@@ -552,6 +559,7 @@ pub extern "C" fn mdns_service_query_hostname(
     serv: *mut MDNSService,
     data: *const c_void,
     resolved: extern "C" fn(*const c_void, *const c_char, *const c_char),
+    timedout: extern "C" fn(*const c_void, *const c_char),
     hostname: *const c_char,
 ) {
     assert!(!serv.is_null());
@@ -562,6 +570,7 @@ pub extern "C" fn mdns_service_query_hostname(
         let callback = Callback {
             data: data,
             resolved: resolved,
+            timedout: timedout,
         };
         (*serv).query_hostname(callback, &hostname);
     }
@@ -621,6 +630,9 @@ mod tests {
     ) -> () {
     }
 
+    #[no_mangle]
+    pub extern "C" fn mdns_service_timedout(_: *const c_void, _: *const c_char) -> () {}
+
     fn listen_until(addr: &std::net::Ipv4Addr, stop: u64) -> thread::JoinHandle<Vec<String>> {
         let port = 5353;
 
@@ -723,6 +735,7 @@ mod tests {
         let callback = Callback {
             data: 0 as *const c_void,
             resolved: mdns_service_resolved,
+            timedout: mdns_service_timedout,
         };
         let hostname = Uuid::new_v4().to_hyphenated().to_string() + ".local";
         service.query_hostname(callback, &hostname);
@@ -744,6 +757,7 @@ mod tests {
             let callback = Callback {
                 data: 0 as *const c_void,
                 resolved: mdns_service_resolved,
+                timedout: mdns_service_timedout,
             };
             let hostname = Uuid::new_v4().to_hyphenated().to_string() + ".local";
             service.query_hostname(callback, &hostname);
@@ -767,6 +781,7 @@ mod tests {
         let callback = Callback {
             data: 0 as *const c_void,
             resolved: mdns_service_resolved,
+            timedout: mdns_service_timedout,
         };
         service.query_hostname(callback, &hostname);
         thread::sleep(time::Duration::from_secs(4));
@@ -790,11 +805,13 @@ mod tests {
             hostnames.push(hostname);
         }
 
-        let q = create_query(42, &hostnames);
-        match dns_parser::Packet::parse(&q) {
-            Ok(parsed) => {
-                assert_eq!(parsed.questions.len(), 100);
-            }
+        match create_query(42, &hostnames) {
+            Ok(q) => match dns_parser::Packet::parse(&q) {
+                Ok(parsed) => {
+                    assert_eq!(parsed.questions.len(), 100);
+                }
+                Err(_) => assert!(false),
+            },
             Err(_) => assert!(false),
         }
     }

media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.cpp

@@ -41,15 +41,15 @@ static const char* pcmLogTag = "PeerConnectionMedia";
 #define LOGTAG pcmLogTag
 
 void PeerConnectionMedia::StunAddrsHandler::OnMDNSQueryComplete(
-    const nsCString& hostname, const nsCString& address) {
+    const nsCString& hostname, const Maybe<nsCString>& address) {
   ASSERT_ON_THREAD(pcm_->mMainThread);
   auto itor = pcm_->mQueriedMDNSHostnames.find(hostname.BeginReading());
   if (itor != pcm_->mQueriedMDNSHostnames.end()) {
-    if (!address.IsEmpty()) {
+    if (address) {
       for (auto& cand : itor->second) {
         // Replace obfuscated address with actual address
         std::string obfuscatedAddr = cand.mTokenizedCandidate[4];
-        cand.mTokenizedCandidate[4] = address.BeginReading();
+        cand.mTokenizedCandidate[4] = address->BeginReading();
         std::ostringstream o;
         for (size_t i = 0; i < cand.mTokenizedCandidate.size(); ++i) {
           o << cand.mTokenizedCandidate[i];

media/webrtc/signaling/src/peerconnection/PeerConnectionMedia.h

@@ -146,7 +146,7 @@ class PeerConnectionMedia : public sigslot::has_slots<> {
     explicit StunAddrsHandler(PeerConnectionMedia* pcm) : pcm_(pcm) {}
 
     void OnMDNSQueryComplete(const nsCString& hostname,
-                             const nsCString& address) override;
+                             const Maybe<nsCString>& address) override;
 
     void OnStunAddrsAvailable(
         const mozilla::net::NrIceStunAddrArray& addrs) override;