IPC: Remove unnecessary conversions of BrokerableAttachment.

ipc/ipc_message_attachment_set.cc

 // Copyright (c) 2011 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 "ipc/ipc_message_attachment_set.h"
 
 #include <algorithm>
 #include "base/logging.h"
 #include "base/posix/eintr_wrapper.h"
 #include "ipc/brokerable_attachment.h"
@@ skipping 157 matching lines @@
   scoped_refptr<BrokerableAttachment> brokerable_attachment(
       brokerable_attachments_[index]);
   return scoped_refptr<MessageAttachment>(brokerable_attachment.get());
 }
 
 void MessageAttachmentSet::CommitAllDescriptors() {
   attachments_.clear();
   consumed_descriptor_highwater_ = 0;
 }
 
-std::vector<BrokerableAttachment*>
+std::vector<scoped_refptr<IPC::BrokerableAttachment>>
 MessageAttachmentSet::GetBrokerableAttachments() const {
-  std::vector<BrokerableAttachment*> output;
-  for (const scoped_refptr<MessageAttachment>& attachment :
-       brokerable_attachments_) {
-      output.push_back(static_cast<BrokerableAttachment*>(attachment.get()));
-  }
-  return output;
+  return brokerable_attachments_;
 }
 
 void MessageAttachmentSet::ReplacePlaceholderWithAttachment(
-    const scoped_refptr<BrokerableAttachment>& attachment) {
+    scoped_refptr<BrokerableAttachment> attachment) {

[Tom Sepez, 2015/10/29 19:12:51]

I think this can still be const&

[erikchen, 2015/10/29 19:53:54]

Passing by const& will produce the wrong behavior for the line:
*it = attachment;

If |attachment| is the same object as *it (self assignment).

This is currently not possible, as we should always be replacing a placeholder
attachment with a non-placeholder attachment, but it is not enforced. I switched
the parameter back to const&, and then added an assertion which implicitly
enforced *it != attachment.

   for (auto it = brokerable_attachments_.begin();
        it != brokerable_attachments_.end(); ++it) {
-    BrokerableAttachment* brokerable_attachment =
-        static_cast<BrokerableAttachment*>(it->get());
-
-    if (brokerable_attachment->GetBrokerableType() ==
-            BrokerableAttachment::PLACEHOLDER &&
-        brokerable_attachment->GetIdentifier() == attachment->GetIdentifier()) {
+    if ((*it)->GetBrokerableType() == BrokerableAttachment::PLACEHOLDER &&
+        (*it)->GetIdentifier() == attachment->GetIdentifier()) {
       *it = attachment;
       return;
     }
   }
 
   // This function should only be called if there is a placeholder ready to be
   // replaced.
   NOTREACHED();
 }
 
@@ skipping 37 matching lines @@
   for (unsigned i = 0; i < count; ++i)
     AddAttachment(
         new internal::PlatformFileAttachment(base::ScopedFD(buffer[i])));
 }
 
 #endif  // OS_POSIX
 
 }  // namespace IPC