Remove some linked_ptr from /content

content/browser/browser_plugin/browser_plugin_guest.cc

 // Copyright (c) 2012 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 "content/browser/browser_plugin/browser_plugin_guest.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 
 #include "base/macros.h"
+#include "base/memory/ptr_util.h"
 #include "base/message_loop/message_loop.h"
 #include "base/pickle.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "cc/surfaces/surface.h"
 #include "cc/surfaces/surface_manager.h"
 #include "content/browser/browser_plugin/browser_plugin_embedder.h"
 #include "content/browser/browser_thread_impl.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/compositor/surface_utils.h"
@@ skipping 463 matching lines @@
 gfx::Point BrowserPluginGuest::GetScreenCoordinates(
     const gfx::Point& relative_position) const {
   if (!attached())
     return relative_position;
 
   gfx::Point screen_pos(relative_position);
   screen_pos += guest_window_rect_.OffsetFromOrigin();
   return screen_pos;
 }
 
 void BrowserPluginGuest::SendMessageToEmbedder(IPC::Message* msg) {

[Avi (use Gerrit), 2016/10/19 18:10:32]

Does it make sense, rather than doing a WrapUnique below, to make this function
take an owning pointer?

   // During tests, attache() may be true when there is no owner_web_contents_;
   // in this case just queue any messages we receive.
   if (!attached() || !owner_web_contents_) {
     // Some pages such as data URLs, javascript URLs, and about:blank
     // do not load external resources and so they load prior to attachment.
     // As a result, we must save all these IPCs until attachment and then
     // forward them so that the embedder gets a chance to see and process
     // the load events.
-    pending_messages_.push_back(linked_ptr<IPC::Message>(msg));
+    pending_messages_.push_back(base::WrapUnique(msg));
     return;
   }
   owner_web_contents_->Send(msg);
 }
 
 void BrowserPluginGuest::DragSourceEndedAt(int client_x, int client_y,
     int screen_x, int screen_y, blink::WebDragOperation operation) {
   web_contents()->GetRenderViewHost()->DragSourceEndedAt(client_x, client_y,
       screen_x, screen_y, operation);
   seen_embedder_drag_source_ended_at_ = true;
@@ skipping 34 matching lines @@
 
 void BrowserPluginGuest::EmbedderSystemDragEnded() {
   seen_embedder_system_drag_ended_ = true;
   EndSystemDragIfApplicable();
 }
 
 // TODO(wjmaclean): Replace this approach with ones based on std::function
 // as in https://codereview.chromium.org/1404353004/ once all Chrome platforms
 // support this. https://crbug.com/544212
 IPC::Message* BrowserPluginGuest::UpdateInstanceIdIfNecessary(
-    IPC::Message* msg) const {
-  DCHECK(msg);
+    std::unique_ptr<IPC::Message> msg) const {
+  DCHECK(msg.get());
 
   int msg_browser_plugin_instance_id = browser_plugin::kInstanceIDNone;
-  base::PickleIterator iter(*msg);
+  base::PickleIterator iter(*msg.get());
   if (!iter.ReadInt(&msg_browser_plugin_instance_id) ||
       msg_browser_plugin_instance_id != browser_plugin::kInstanceIDNone) {
-    return msg;
+    return msg.release();

[Avi (use Gerrit), 2016/10/19 18:08:23]

What?

It makes little sense to have the parameter be a properly owning pointer but
make the return value unowned-yet-owning.

   }
 
   // This method may be called with no browser_plugin_instance_id in tests.
   if (!browser_plugin_instance_id())
-    return msg;
+    return msg.release();

[Avi (use Gerrit), 2016/10/19 18:08:23]

ditto

 
   std::unique_ptr<IPC::Message> new_msg(
       new IPC::Message(msg->routing_id(), msg->type(), msg->priority()));
   new_msg->WriteInt(browser_plugin_instance_id());
 
   // Copy remaining payload from original message.
   // TODO(wjmaclean): it would be nice if IPC::PickleIterator had a method
   // like 'RemainingBytes()' so that we don't have to include implementation-
   // specific details like sizeof() in the next line.
   DCHECK(msg->payload_size() > sizeof(int));
   size_t remaining_bytes = msg->payload_size() - sizeof(int);
   const char* data = nullptr;
   bool read_success = iter.ReadBytes(&data, remaining_bytes);
   CHECK(read_success)
       << "Unexpected failure reading remaining IPC::Message payload.";
   bool write_success = new_msg->WriteBytes(data, remaining_bytes);
   CHECK(write_success)
       << "Unexpected failure writing remaining IPC::Message payload.";
 
-  delete msg;
   return new_msg.release();

[Avi (use Gerrit), 2016/10/19 18:08:23]

All paths out of this function return a unique_ptr but call .release() on it.
Modify this function to return the unique_ptr then.

[limasdf, 2016/10/20 12:09:02]

Done.

 }
 
 void BrowserPluginGuest::SendQueuedMessages() {
   if (!attached())
     return;
 
   while (!pending_messages_.empty()) {
-    linked_ptr<IPC::Message> message_ptr = pending_messages_.front();
+    std::unique_ptr<IPC::Message> message_ptr =
+        std::move(pending_messages_.front());
     pending_messages_.pop_front();
-    SendMessageToEmbedder(
-        UpdateInstanceIdIfNecessary(message_ptr.release()));
+    SendMessageToEmbedder(UpdateInstanceIdIfNecessary(std::move(message_ptr)));

[Avi (use Gerrit), 2016/10/19 18:10:32]

And it all comes together here. Have UpdateInstanceIdIfNecessary return an owned
pointer, have SendMessageToEmbedder take an owned pointer, and boom we're good.

[limasdf, 2016/10/20 12:09:02]

Done.

   }
 }
 
 void BrowserPluginGuest::SendTextInputTypeChangedToView(
     RenderWidgetHostViewBase* guest_rwhv) {
   if (!guest_rwhv)
     return;
 
   if (!owner_web_contents_) {
     // If we were showing an interstitial, then we can end up here during
@@ skipping 420 matching lines @@
           range, character_bounds);
 }
 #endif
 
 void BrowserPluginGuest::SetContextMenuPosition(const gfx::Point& position) {
   if (delegate_)
     delegate_->SetContextMenuPosition(position);
 }
 
 }  // namespace content