Limit number of service instances passed to onServiceList listeners.

chrome/browser/extensions/api/mdns/mdns_api.cc

Index: chrome/browser/extensions/api/mdns/mdns_api.cc
diff --git a/chrome/browser/extensions/api/mdns/mdns_api.cc b/chrome/browser/extensions/api/mdns/mdns_api.cc
index 2a7c41d210931d1540e61b8cb89fe402a203e1e8..b69899823eac3f243511c25e24b4b5fb756eacbc 100644
--- a/chrome/browser/extensions/api/mdns/mdns_api.cc
+++ b/chrome/browser/extensions/api/mdns/mdns_api.cc
@@ -22,6 +22,10 @@ const char kCastServiceType[] = "_googlecast._tcp.local";
 const char kPrivetServiceType[] = "_privet._tcp.local";
 const char kTestServiceType[] = "_testing._tcp.local";
 
+// Maximum number of services that may be included in an onServiceList event
+// as arguments.
+const uint8 kMaxServicesPerOnServiceListEvent = 64;

[not at google - send to devlin, 2015/03/30 17:08:50]

You can, and should, declare this constant in the schema file. You can access
that from here, and
- it'll be automatically generated as part of the API documentation.
- be available to developers programmatically.

[Red Daly, 2015/03/31 19:23:50]

I didn't find great support for constants in the IDL for these APIs:
https://code.google.com/p/chromium/codesearch#chromium/src/tools/idl_parser/i...

In web IDL there is a concept of a "const," but that isn't part of the IDL
available to the mdns API.  I see two possibilities:
1) A new interface ("Functions") with a method "static long
getMaxServiceInstancesPerEvent ();"
2) A new enum ("Constants") with a member MaxServiceInstancesPerEvent.

2 seems like a perversion of enum semantics.  I implemented 1; let me know if
you think it's a good solution.

+
 bool IsServiceTypeWhitelisted(const std::string& service_type) {
   return service_type == kCastServiceType ||
          service_type == kPrivetServiceType ||
@@ -139,7 +143,8 @@ void MDnsAPI::OnDnsSdEvent(const std::string& service_type,
 
   std::vector<linked_ptr<mdns::MDnsService> > args;
   for (DnsSdRegistry::DnsSdServiceList::const_iterator it = services.begin();
-       it != services.end(); ++it) {
+       it != services.end() && args.size() < kMaxServicesPerOnServiceListEvent;

[not at google - send to devlin, 2015/03/30 17:08:50]

It would be nice to warn the developer somehow if this will fail. Silently
limiting could be hard to debug.

[mark a. foltz, 2015/03/30 17:57:22]

That would depend on the number of devices advertising on the LAN, which is hard
to tell in advance.

[Red Daly, 2015/03/31 19:23:50]

I added a VLOG(1) message, which will help a developer who looks at VLOG
messages.  Beyond that, I could change the callback interface to take another
argument to indicate truncation (overkill), or output a warning to the
JavaScript console.  Is the JavaScript console output worth doing?

[not at google - send to devlin, 2015/04/01 20:25:58]

JS console output is worth doing insofar as VLOG output is unlikely to get
noticed, and this is for developer's help after all.

It's simple enough:
https://code.google.com/p/chromium/codesearch#search/&q=%22new%20ExtensionMsg...

Incidentally: can all of this be done at registration time, rather than at event
sending time? At least at the warning level? I think that would be better.

[Red Daly, 2015/04/02 22:25:21]

I modified this to do JS console output but didn't yet make the change to warn
at registration time.  I'm worried a warning at registration time would spam the
error console too much to educate about a rather rare case.  Do you think the
new const is enough of a warning to the developer?

+       ++it) {
     linked_ptr<mdns::MDnsService> mdns_service =
         make_linked_ptr(new mdns::MDnsService);
     mdns_service->service_name = (*it).service_name;