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..abcf08e29228ce22ea8346cafa1f8dddbd7d767d 100644
--- a/chrome/browser/extensions/api/mdns/mdns_api.cc
+++ b/chrome/browser/extensions/api/mdns/mdns_api.cc
@@ -9,7 +9,11 @@
 #include "base/lazy_instance.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/common/extensions/api/mdns.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/render_view_host.h"
+#include "extensions/browser/extension_host.h"
 #include "extensions/browser/extension_registry.h"
+#include "extensions/common/extension_messages.h"
 
 namespace extensions {
 
@@ -84,35 +88,9 @@ void MDnsAPI::OnListenerRemoved(const EventListenerInfo& details) {
 
 void MDnsAPI::UpdateMDnsListeners(const EventListenerInfo& details) {
   std::set<std::string> new_service_types;
-
-  // Check all listeners for service type filters.
-  const EventListenerMap::ListenerList& listeners =
-      extensions::EventRouter::Get(browser_context_)
-          ->listeners()
-          .GetEventListenersByName(details.event_name);
-  for (EventListenerMap::ListenerList::const_iterator it = listeners.begin();
-       it != listeners.end(); ++it) {
-    base::DictionaryValue* filter = ((*it)->filter());
-
-    std::string filter_value;
-    filter->GetStringASCII(kEventFilterServiceTypeKey, &filter_value);
-    if (filter_value.empty())
-      continue;
-
-    const Extension* extension = ExtensionRegistry::Get(browser_context_)->
-        enabled_extensions().GetByID((*it)->extension_id());
-    // Don't listen for services associated only with disabled extensions.
-    if (!extension)
-      continue;
-
-    // Platform apps may query for all services; other types of extensions are
-    // restricted to a whitelist.
-    if (!extension->is_platform_app() &&
-        !IsServiceTypeWhitelisted(filter_value))
-      continue;
-
-    new_service_types.insert(filter_value);
-  }
+  GetValidOnServiceListListeners("" /* sevice_type_filter */,

[mark a. foltz, 2015/04/07 22:43:32]

What does an empty filter mean?  Please document.

[mark a. foltz, 2015/04/07 22:43:32]

service_type_filter

[Red Daly, 2015/04/07 23:16:56]

Done.

[Red Daly, 2015/04/07 23:16:57]

Done.  Added explanation to inline comment; there was already some language in
the header file.

+                                 nullptr /* extension_ids */,
+                                 &new_service_types);
 
   // Find all the added and removed service types since last update.
   std::set<std::string> added_service_types =
@@ -140,6 +118,22 @@ 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) {
+    if (static_cast<long>(args.size()) ==
+        api::mdns::MAX_SERVICE_INSTANCES_PER_EVENT) {
+      // TODO(reddaly): This is not the most meaningful way of notifying the
+      // application that something bad happened.  It will go to the user's
+      // console (which most users don't look at)and the developer will be none
+      // the wiser.  Instead, changing the event to pass the number of
+      // discovered instances would allow the caller to know when the list is
+      // truncated and tell the user something meaningful in the extension/app.
+      WriteToConsole(service_type,
+                     content::CONSOLE_MESSAGE_LEVEL_WARNING,
+                     base::StringPrintf(
+                         "Truncating number of service instances in "
+                         "onServiceList to maximum allowed: %d",
+                         api::mdns::MAX_SERVICE_INSTANCES_PER_EVENT));
+      break;
+    }
     linked_ptr<mdns::MDnsService> mdns_service =
         make_linked_ptr(new mdns::MDnsService);
     mdns_service->service_name = (*it).service_name;
@@ -162,4 +156,70 @@ void MDnsAPI::OnDnsSdEvent(const std::string& service_type,
   extensions::EventRouter::Get(browser_context_)->BroadcastEvent(event.Pass());
 }
 
+void MDnsAPI::GetValidOnServiceListListeners(
+    const std::string& service_type_filter,
+    std::set<std::string>* extension_ids,
+    std::set<std::string>* service_types) {
+  for (const auto& listener :
+           extensions::EventRouter::Get(browser_context_)->listeners()

[mark a. foltz, 2015/04/07 22:43:33]

This indentation looks funny, but might be right.

[Red Daly, 2015/04/07 23:16:56]

Acknowledged - changed to 2 spaces for indentation.  I can't tell 100% from the
C++ style guide but 2 spaces is the default:
https://google-styleguide.googlecode.com/svn/trunk/cppguide.html

+           .GetEventListenersByName(mdns::OnServiceList::kEventName)) {
+    base::DictionaryValue* filter = (listener->filter());

[mark a. foltz, 2015/04/07 22:43:33]

Extra ( )

[Red Daly, 2015/04/07 23:16:56]

Done.

+
+    std::string service_type;
+    filter->GetStringASCII(kEventFilterServiceTypeKey, &service_type);
+    if (service_type.empty())
+      continue;
+
+    // Match service type when filter isn't ""
+    if (!service_type_filter.empty() && service_type_filter != service_type)
+      continue;
+
+    const Extension* extension = ExtensionRegistry::Get(browser_context_)->
+        enabled_extensions().GetByID(listener->extension_id());
+    // Don't listen for services associated only with disabled extensions.
+    if (!extension)
+      continue;
+
+    // Platform apps may query for all services; other types of extensions are
+    // restricted to a whitelist.
+    if (!extension->is_platform_app() &&
+        !IsServiceTypeWhitelisted(service_type))
+      continue;
+
+    if (extension_ids)
+      extension_ids->insert(listener->extension_id());
+    if (service_types)
+      service_types->insert(service_type);
+  }
+}
+
+void MDnsAPI::WriteToConsole(const std::string& service_type,
+                             content::ConsoleMessageLevel level,
+                             const std::string& message) {
+  // Get all the extensions with an onServiceList listener for a particular
+  // service type.
+  std::set<std::string> extension_ids;
+  GetValidOnServiceListListeners(service_type,
+                                 &extension_ids,
+                                 nullptr /* service_types */);
+
+  std::string logged_message(std::string("[chrome.mdns] ") + message);
+
+  // Log to the consoles of the background pages for those extensions.
+  for (const std::string& extension_id : extension_ids) {
+    extensions::ExtensionHost* host =
+        extensions::ProcessManager::Get(browser_context_)
+        ->GetBackgroundHostForExtension(extension_id);
+    if (!host)
+      continue;
+    content::RenderViewHost* rvh = host->render_view_host();
+    if (!rvh)
+      continue;
+    rvh->Send(new ExtensionMsg_AddMessageToConsole(
+        rvh->GetRoutingID(),
+        level,
+        logged_message));
+  }
+}
+
 }  // namespace extensions