dbus: Suppress "Rejecting a message from a wrong sender" error from ObjectManager

dbus/object_manager.cc

 // 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.
 
 #include "dbus/object_manager.h"
 
 #include "base/bind.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
@@ skipping 192 matching lines @@
     return false;
 
   if (!bus_->Connect() || !bus_->SetUpAsyncOperations())
     return false;
 
   service_name_owner_ =
       bus_->GetServiceOwnerAndBlock(service_name_, Bus::SUPPRESS_ERRORS);
 
   const std::string match_rule =
       base::StringPrintf(
           "type='signal', sender='%s', interface='%s', member='%s'",

[hashimoto, 2015/08/17 07:07:49]

ObjectManager::HandleMessage looks like a dead copy of
ObjectProxy::HandleMessage, and ObjectProxy uses "type='signal', interface='%s',
path='%s'" as the match rule. (why aren't we using
ObjectProxy::ConnectToSignal() here btw?)
Can we use the same (or a similar) match rule to avoid unnecessary
HandleMessage() calls?

[satorux1, 2015/08/17 07:35:07]

Good points, but I'd rather not to do non-trivial refactoring in the CL.

Re the filter rule, it's a great catch that there was a slight difference
(sender='%s' is used here, but not in object_proxy.cc). However, per the finding
at https://code.google.com/p/chromium/issues/detail?id=507206#c14 the object
managers for peerd, privetd, apmanager were receiving signals from bluez for
some reason...

           service_name_.c_str(),
           kPropertiesInterface,
           kPropertiesChanged);
 
   bus_->AddFilterFunction(&ObjectManager::HandleMessageThunk, this);
 
   ScopedDBusError error;
   bus_->AddMatch(match_rule, error.get());
   if (error.is_set()) {
     LOG(ERROR) << "ObjectManager failed to add match rule \"" << match_rule
@@ skipping 21 matching lines @@
                                                     void* user_data) {
   ObjectManager* self = reinterpret_cast<ObjectManager*>(user_data);
   return self->HandleMessage(connection, raw_message);
 }
 
 DBusHandlerResult ObjectManager::HandleMessage(DBusConnection* connection,
                                                DBusMessage* raw_message) {
   DCHECK(bus_);
   bus_->AssertOnDBusThread();
 
+  // Handle the message only if it is a signal.
   if (dbus_message_get_type(raw_message) != DBUS_MESSAGE_TYPE_SIGNAL)
     return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 
   // raw_message will be unrefed on exit of the function. Increment the
   // reference so we can use it in Signal.
   dbus_message_ref(raw_message);
   scoped_ptr<Signal> signal(
       Signal::FromRawMessage(raw_message));
 
   const std::string interface = signal->GetInterface();
   const std::string member = signal->GetMember();
 
   statistics::AddReceivedSignal(service_name_, interface, member);
 
-  // Only handle the PropertiesChanged signal.
+  // Handle the signal only if it is PropertiesChanged.
   const std::string absolute_signal_name =
       GetAbsoluteMemberName(interface, member);
   const std::string properties_changed_signal_name =
       GetAbsoluteMemberName(kPropertiesInterface, kPropertiesChanged);
   if (absolute_signal_name != properties_changed_signal_name)
     return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 
   VLOG(1) << "Signal received: " << signal->ToString();
 
-  // Make sure that the signal originated from the correct sender.
+  // Handle the signal only if it is from the service that the ObjectManager
+  // instance is interested in.
   std::string sender = signal->GetSender();
-  if (service_name_owner_ != sender) {
-    LOG(ERROR) << "Rejecting a message from a wrong sender.";
-    UMA_HISTOGRAM_COUNTS("DBus.RejectedSignalCount", 1);
+  if (service_name_owner_ != sender)
     return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
-  }
 
   const ObjectPath path = signal->GetPath();
 
   if (bus_->HasDBusThread()) {
     // Post a task to run the method in the origin thread. Transfer ownership of
     // |signal| to NotifyPropertiesChanged, which will handle the clean up.
     Signal* released_signal = signal.release();
     bus_->GetOriginTaskRunner()->PostTask(
         FROM_HERE,
         base::Bind(&ObjectManager::NotifyPropertiesChanged,
@@ skipping 222 matching lines @@
         RemoveInterface(object_path, *iiter);
     }
 
   }
 
   if (!new_owner.empty())
     GetManagedObjects();
 }
 
 }  // namespace dbus