Limit number of service instances passed to onServiceList listeners.

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

 // Copyright 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.
 
 #include "chrome/browser/extensions/api/mdns/mdns_api.h"
 
 #include <vector>
 
 #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 {
 
 namespace mdns = api::mdns;
 
 namespace {
 
 // Whitelisted mDNS service types.
 const char kCastServiceType[] = "_googlecast._tcp.local";
 const char kPrivetServiceType[] = "_privet._tcp.local";
@@ skipping 54 matching lines @@
   UpdateMDnsListeners(details);
 }
 
 void MDnsAPI::OnListenerRemoved(const EventListenerInfo& details) {
   DCHECK(thread_checker_.CalledOnValidThread());
   UpdateMDnsListeners(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(nullptr, &new_service_types);

[mark a. foltz, 2015/04/03 23:26:40]

What is this nullptr argument?  Optional arguments are discouraged.

Can you have the function new up a set<> and return it instead?

[Red Daly, 2015/04/04 00:56:15]

Done.  I did it the other way to reduce code duplication (one caller needed the
service types but not the listeners).

 
   // Find all the added and removed service types since last update.
   std::set<std::string> added_service_types =
       base::STLSetDifference<std::set<std::string> >(
           new_service_types, service_types_);
   std::set<std::string> removed_service_types =
       base::STLSetDifference<std::set<std::string> >(
           service_types_, new_service_types);
 
   // Update the registry.
   DnsSdRegistry* registry = dns_sd_registry();
   for (const auto& srv : added_service_types) {
     registry->RegisterDnsSdListener(srv);
   }
   for (const auto& srv : removed_service_types) {
     registry->UnregisterDnsSdListener(srv);
   }
   service_types_ = new_service_types;
 }
 
 void MDnsAPI::OnDnsSdEvent(const std::string& service_type,
                            const DnsSdRegistry::DnsSdServiceList& services) {
   DCHECK(thread_checker_.CalledOnValidThread());
 
   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) {

[mark a. foltz, 2015/04/03 23:26:40]

Can this constant be declared as size_t?

[Red Daly, 2015/04/04 00:56:15]

I don't think there's currently support for that in the IDL parser:
https://code.google.com/p/chromium/codesearch#chromium/src/tools/idl_parser/i...

+      WriteToConsole(service_type,
+                     content::CONSOLE_MESSAGE_LEVEL_WARNING,
+                     base::StringPrintf(
+                         "Truncating number of service instances in "

[mark a. foltz, 2015/04/03 23:26:40]

Is there a prefix logged for this?  If not, prepend [chrome.mdns]

[Red Daly, 2015/04/04 00:56:15]

Done.

+                         "onServiceList to maximum allowed: %d",
+                         api::mdns::MAX_SERVICE_INSTANCES_PER_EVENT));

[mark a. foltz, 2015/04/03 23:26:39]

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.

I guess this is okay for now (especially since the practical circumstances when
this would be hit are few and far between).

[Red Daly, 2015/04/04 00:56:15]

I agree, added a TODO for the future enhancement.

+      break;
+    }
     linked_ptr<mdns::MDnsService> mdns_service =
         make_linked_ptr(new mdns::MDnsService);
     mdns_service->service_name = (*it).service_name;
     mdns_service->service_host_port = (*it).service_host_port;
     mdns_service->ip_address = (*it).ip_address;
     mdns_service->service_data = (*it).service_data;
     args.push_back(mdns_service);
   }
 
   scoped_ptr<base::ListValue> results = mdns::OnServiceList::Create(args);
   scoped_ptr<Event> event(
       new Event(mdns::OnServiceList::kEventName, results.Pass()));
   event->restrict_to_browser_context = browser_context_;
   event->filter_info.SetServiceType(service_type);
 
   // TODO(justinlin): To avoid having listeners without filters getting all
   // events, modify API to have this event require filters.
   // TODO(reddaly): If event isn't on whitelist, ensure it does not get
   // broadcast to extensions.
   extensions::EventRouter::Get(browser_context_)->BroadcastEvent(event.Pass());
 }
 
+void MDnsAPI::GetValidOnServiceListListeners(
+    std::set<const EventListener*>* listeners,
+    std::set<std::string>* service_types) {
+ for (const auto& listener :
+          extensions::EventRouter::Get(browser_context_)->listeners()
+          .GetEventListenersByName(mdns::OnServiceList::kEventName)) {
+   base::DictionaryValue* filter = (listener->filter());
+
+   std::string service_type;
+   filter->GetStringASCII(kEventFilterServiceTypeKey, &service_type);
+   if (service_type.empty())
+     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 (listeners)
+     listeners->insert(listener.get());
+   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<const EventListener*> listeners;
+  GetValidOnServiceListListeners(&listeners, nullptr);
+  std::set<std::string> extension_ids;
+  for (const EventListener* listener : listeners) {
+    extension_ids.insert(listener->extension_id());
+  }
+
+  // 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,
+        message));
+  }
+}
+
 }  // namespace extensions