Improve resource record implementation in the mdns package.

Improve resource record implementation in the mdns package.

This change adds the ability to query RR types PTR, SRV, and TXT and handle arbitrary responses.

R=sgjesse@google.com

Committed: https://github.com/dart-lang/fletch/commit/8ed2a1804b1c6408ba70954779472a6c6e21571c

[Søren Gjesse, 2015-11-06 08:44:48]

LGTM, with a few comments

Also it would be nice to have a few decoding tests. The ones I added were just
packages I grabbed from the protocol handler.

https://codereview.chromium.org/1412063015/diff/1/pkg/mdns/lib/mdns-sd.dart
File pkg/mdns/lib/mdns-sd.dart (right):

https://codereview.chromium.org/1412063015/diff/1/pkg/mdns/lib/mdns-sd.dart#n...
pkg/mdns/lib/mdns-sd.dart:1: // Copyright (c) 2015, the Fletch project authors.
Please see the AUTHORS file
Maybe move this into ../bin.

https://codereview.chromium.org/1412063015/diff/1/pkg/mdns/lib/mdns.dart
File pkg/mdns/lib/mdns.dart (right):

https://codereview.chromium.org/1412063015/diff/1/pkg/mdns/lib/mdns.dart#newc...
pkg/mdns/lib/mdns.dart:50: /// Lookup [hostname] using mDNS.
The comment needs to be updated.

https://codereview.chromium.org/1412063015/diff/1/pkg/mdns/lib/src/native_ext...
File pkg/mdns/lib/src/native_extension_client.dart (right):

https://codereview.chromium.org/1412063015/diff/1/pkg/mdns/lib/src/native_ext...
pkg/mdns/lib/src/native_extension_client.dart:97: new
DateTime.now().millisecondsSinceEpoch + 2000);
Please add a comment on the 2s constant here.

https://codereview.chromium.org/1412063015/diff/1/pkg/mdns/lib/src/native_pro...
File pkg/mdns/lib/src/native_protocol_client.dart (right):

https://codereview.chromium.org/1412063015/diff/1/pkg/mdns/lib/src/native_pro...
pkg/mdns/lib/src/native_protocol_client.dart:16: class ResourceRecordCache {
Maybe explain how this cache works. Any reason for not just using a HashMap with
a key from type and name?

https://codereview.chromium.org/1412063015/diff/1/pkg/mdns/lib/src/native_pro...
pkg/mdns/lib/src/native_protocol_client.dart:29: for (int i = position; i <
position + size; i++) {
The clearing could just run from 0 to size, but maybe this is better.

https://codereview.chromium.org/1412063015/diff/20001/pkg/mdns/lib/src/native...
File pkg/mdns/lib/src/native_extension_client.dart (right):

https://codereview.chromium.org/1412063015/diff/20001/pkg/mdns/lib/src/native...
pkg/mdns/lib/src/native_extension_client.dart:59: Future<InternetAddress>
lookup(
This should be a Stream of ResourceRecord now.

https://codereview.chromium.org/1412063015/diff/20001/pkg/mdns/lib/src/native...
File pkg/mdns/lib/src/native_protocol_client.dart (right):

https://codereview.chromium.org/1412063015/diff/20001/pkg/mdns/lib/src/native...
pkg/mdns/lib/src/native_protocol_client.dart:113: Stream<InternetAddress>
lookup(
InternetAddress -> ResourceRecord

[karlklose, 2015-11-06 12:09:32]

Patchset #3 (id:40001) has been deleted

[karlklose, 2015-11-06 12:20:00]

I updated and added tests and fixed an issue where we did not cancel the timers
for pending requests with a long timeout (this would make the test hang for a
day).

https://codereview.chromium.org/1412063015/diff/1/pkg/mdns/lib/mdns-sd.dart
File pkg/mdns/lib/mdns-sd.dart (right):

https://codereview.chromium.org/1412063015/diff/1/pkg/mdns/lib/mdns-sd.dart#n...
pkg/mdns/lib/mdns-sd.dart:1: // Copyright (c) 2015, the Fletch project authors.
Please see the AUTHORS file
On 2015/11/06 08:44:48, Søren Gjesse wrote:
> Maybe move this into ../bin.

Done.

https://codereview.chromium.org/1412063015/diff/1/pkg/mdns/lib/mdns.dart
File pkg/mdns/lib/mdns.dart (right):

https://codereview.chromium.org/1412063015/diff/1/pkg/mdns/lib/mdns.dart#newc...
pkg/mdns/lib/mdns.dart:50: /// Lookup [hostname] using mDNS.
On 2015/11/06 08:44:48, Søren Gjesse wrote:
> The comment needs to be updated.

Done.

https://codereview.chromium.org/1412063015/diff/1/pkg/mdns/lib/src/native_ext...
File pkg/mdns/lib/src/native_extension_client.dart (right):

https://codereview.chromium.org/1412063015/diff/1/pkg/mdns/lib/src/native_ext...
pkg/mdns/lib/src/native_extension_client.dart:97: new
DateTime.now().millisecondsSinceEpoch + 2000);
On 2015/11/06 08:44:48, Søren Gjesse wrote:
> Please add a comment on the 2s constant here.

Done.

https://codereview.chromium.org/1412063015/diff/1/pkg/mdns/lib/src/native_pro...
File pkg/mdns/lib/src/native_protocol_client.dart (right):

https://codereview.chromium.org/1412063015/diff/1/pkg/mdns/lib/src/native_pro...
pkg/mdns/lib/src/native_protocol_client.dart:16: class ResourceRecordCache {
On 2015/11/06 08:44:48, Søren Gjesse wrote:
> Maybe explain how this cache works. Any reason for not just using a HashMap
with
> a key from type and name?

Done.

The main reason was to control the size of the cache. It seemed less complicated
than using a Map<Pair<String, int>, List<ResourceRecord>> as a backing store.

https://codereview.chromium.org/1412063015/diff/20001/pkg/mdns/lib/src/native...
File pkg/mdns/lib/src/native_extension_client.dart (right):

https://codereview.chromium.org/1412063015/diff/20001/pkg/mdns/lib/src/native...
pkg/mdns/lib/src/native_extension_client.dart:59: Future<InternetAddress>
lookup(
On 2015/11/06 08:44:48, Søren Gjesse wrote:
> This should be a Stream of ResourceRecord now.

Done.

https://codereview.chromium.org/1412063015/diff/20001/pkg/mdns/lib/src/native...
File pkg/mdns/lib/src/native_protocol_client.dart (right):

https://codereview.chromium.org/1412063015/diff/20001/pkg/mdns/lib/src/native...
pkg/mdns/lib/src/native_protocol_client.dart:113: Stream<InternetAddress>
lookup(
On 2015/11/06 08:44:48, Søren Gjesse wrote:
> InternetAddress -> ResourceRecord

Done.

[Søren Gjesse, 2015-11-06 12:28:34]

LGTM!

https://codereview.chromium.org/1412063015/diff/60001/pkg/mdns/test/lookup_re...
File pkg/mdns/test/lookup_resolver_test.dart (right):

https://codereview.chromium.org/1412063015/diff/60001/pkg/mdns/test/lookup_re...
pkg/mdns/test/lookup_resolver_test.dart:28: Expect.isTrue(await result.isEmpty);
Remove await here?

[karlklose, 2015-11-06 12:33:09]

Committed patchset #4 (id:80001) manually as
8ed2a1804b1c6408ba70954779472a6c6e21571c (presubmit successful).