Multicast DNS implementation (initial)

net/dns/mdns_listener.h

+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef NET_DNS_MDNS_LISTENER_H_
+#define NET_DNS_MDNS_LISTENER_H_
+
+#include <string>
+#include <vector>
+
+#include "base/callback.h"
+#include "net/dns/dns_query.h"
+#include "net/dns/dns_response.h"
+#include "net/dns/record_parsed.h"
+
+namespace net {
+
+extern const char kMDNSMulticastGroupIPv4[];

[szym, 2013/05/24 15:32:22]

Why do you need this?

[Noam Samuel, 2013/05/24 21:59:18]

I put it here so that classes other than the implementation can access it.
However, this seems excessive. I will move it to the implmentation .cc.
On 2013/05/24 15:32:22, szym wrote:

[Noam Samuel, 2013/05/29 21:25:16]

Done.

+extern const char kMDNSMulticastGroupIPv6[];
+
+class RecordParsed;
+
+class MDnsTransaction;

[szym, 2013/05/24 15:32:22]

I suggest you define MDnsTransaction first so that you don't have to forward
declare it.

[Noam Samuel, 2013/05/29 21:25:16]

Done.

+
+class MDnsListener;
+
+enum MDnsUpdateType {
+  kMDnsRecordAdded,
+  kMDnsRecordChanged,
+  kMDnsRecordRemoved
+};

[szym, 2013/05/24 15:32:22]

Seems redundant with MDnsCache::UpdateType.

[Noam Samuel, 2013/05/24 19:00:49]

MDnsCache::UpdateType contains a NoChange member, which can never be used as an
update type. I thought it would be unclear if they used the same enum (users
will think they have to handle the NoChange case, and compilers will produce
errors for switch statements that don't handle it).

On 2013/05/24 15:32:22, szym wrote:

+
+enum MDnsTransactionResult {
+  kMDnsTransactionSuccess,
+  kMDnsTransactionTimeout,
+  kMDnsTransactionNsec
+};
+
+class MDnsListenerFactory {

[szym, 2013/05/24 15:32:22]

Needs a comment. I find the name confusing. It can create Listeners and
Transactions. I'd call it MDnsClient.

[Noam Samuel, 2013/05/29 21:25:16]

Done.

+ public:
+  class Delegate {
+   public:
+    virtual ~Delegate() {}
+
+    virtual void OnRecordUpdate(MDnsUpdateType update,
+                                const RecordParsed* record) = 0;
+
+    // TODO(noamsml): Called when a record is marked nonexistent by an NSEC
+    // record.

[szym, 2013/05/24 15:32:22]

Not sure why there's TODO here. What part is missing?

[Noam Samuel, 2013/05/24 19:00:49]

Maybe the TODO should be moved somewhere else. The TODO refers to actually
implementing NSEC record parsing.

On 2013/05/24 15:32:22, szym wrote:

[Noam Samuel, 2013/05/29 21:25:16]

Done.

+    virtual void OnNSecRecord(const std::string& name, unsigned type) = 0;

[szym, 2013/05/24 15:32:22]

Spell "NSec" vs. "Nsec" consistently here and in MDnsTransactionResult.

[Noam Samuel, 2013/05/29 21:25:16]

Done.

+  };
+
+  typedef base::Callback<void(MDnsTransactionResult, const RecordParsed*)>
+      QueryCallback;
+
+  virtual ~MDnsListenerFactory() {}
+
+  // Add delegate for RRType |rrtype| and name |name|.

[szym, 2013/05/24 15:32:22]

Comment says "Add delegate" but it creates a Listener. Probably best to have a
class-level comment explaining what each class represents.

[Noam Samuel, 2013/05/29 21:25:16]

Done.

+  // If |name| is an empty string, listen to all notification of type
+  // |rrtype|.
+  virtual scoped_ptr<MDnsListener> CreateListener(
+      uint16 rrtype,
+      const std::string& name,
+      bool active,

[szym, 2013/05/24 15:32:22]

Needs comment.

[Noam Samuel, 2013/05/29 21:25:16]

Done.

+      bool alert_existing_records,

[szym, 2013/05/24 15:32:22]

Needs comment.

[Noam Samuel, 2013/05/29 21:25:16]

Done.

+      Delegate* delegate) = 0;
+
+  // Create a transaction to Query MDNS for a single-value query
+  // (A, AAAA, TXT, and SRV) asynchronously. May defer to cache.
+  virtual scoped_ptr<MDnsTransaction> CreateTransaction(
+      uint16 rrtype,
+      const std::string& name,
+      const QueryCallback& callback) = 0;
+
+  // Lazily create and return static instance for MDnsListenerFactory.
+  static MDnsListenerFactory* GetInstance();
+
+  // Set the global instance (for testing). MUST be called before the first call
+  // to GetInstance.
+  static void SetInstance(MDnsListenerFactory* instance);
+};
+
+class MDnsTransaction {
+ public:
+  // Destroing the transaction cancels pending transactions.

[szym, 2013/05/24 15:32:22]

"Destroying"
"it" would be better than "pending transactions"

[Noam Samuel, 2013/05/29 21:25:16]

Done.

+  virtual ~MDnsTransaction() {}
+
+  // Get the host or service name for the query.
+  virtual const std::string& GetName() const = 0;
+
+  // Get the type for this query (SRV, TXT, A, AAA, etc)
+  virtual uint16 GetType() const = 0;
+};
+
+class MDnsListener {
+ public:
+  // Destroying the listener stops listening.
+  virtual ~MDnsListener() {}
+
+  // Get the host or service name for this query.
+  // Return an empty string for no name.
+  virtual const std::string& GetName() const = 0;
+
+  // Get the type for this query (SRV, TXT, A, AAA, etc)
+  virtual uint16 GetType() const = 0;
+
+  // Whether this listener is active (true) or passive (false)

[szym, 2013/05/24 15:32:22]

What does that mean?

[Noam Samuel, 2013/05/24 19:00:49]

An active listener is allowed to send periodic queries to see if new records of
a given type are available.

[Noam Samuel, 2013/05/29 21:25:16]

Done.

+  virtual bool IsActive() const = 0;
+
+  // Applies only to listeners with names. Will send out a query for new
+  // information. |force_refresh_cache| will force a refresh of all cached
+  // entities.
+  virtual bool SendQuery(bool force_refresh_cache) = 0;
+
+  // Applies only to listeners with names. Query mDNS cache synchronously for
+  // either single- or multi- valued records.
+  virtual bool QueryCache(std::vector<const RecordParsed*>* records) const = 0;
+};
+}  // net
+#endif  // NET_DNS_MDNS_LISTENER_H_