Update IPC browser_plugin_instance_id before sending queued messages

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 <algorithm>
 
 #include "base/message_loop/message_loop.h"
 #include "base/pickle.h"
@@ skipping 546 matching lines @@
     seen_embedder_drag_source_ended_at_ = false;
     dragged_url_ = GURL();
   }
 }
 
 void BrowserPluginGuest::EmbedderSystemDragEnded() {
   seen_embedder_system_drag_ended_ = true;
   EndSystemDragIfApplicable();
 }
 
+IPC::Message* BrowserPluginGuest::UpdateInstanceIdIfNecessary(
+    IPC::Message* msg) const {
+  DCHECK(msg);
+
+  int msg_browser_plugin_instance_id = browser_plugin::kInstanceIDNone;
+  base::PickleIterator iter(*msg);
+  if (!iter.ReadInt(&msg_browser_plugin_instance_id) ||
+      msg_browser_plugin_instance_id != browser_plugin::kInstanceIDNone) {
+    return msg;
+  }
+
+  // This method may be called with no browser_plugin_instance_id in tests.
+  if (!browser_plugin_instance_id())
+    return msg;
+
+  scoped_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;
+  if (!iter.ReadBytes(&data, remaining_bytes)) {
+    LOG(ERROR) << "Unexpected failure reading remaining IPC::Message payload.";

[lfg, 2015/10/16 16:26:36]

This should never happen, and if it does, we still don't want to send an IPC
with the wrong instance id. Also, we need to track if this every fails. We
should just have a CHECK() and crash if this fails.

[wjmaclean, 2015/10/16 16:50:34]

Ok. I had sort of thought that falling back to the existing behaviour and
logging an error message might be reasonable, but I'd be OK with a CHECK() to
force the failure ... at least that way we would soon enough know if it was
happening.

+    return msg;
+  }
+
+  if (!new_msg->WriteBytes(data, remaining_bytes)) {
+    LOG(ERROR) << "Unexpected failure writing remaining IPC::Message payload.";
+    return msg;
+  }
+
+  delete msg;
+  return new_msg.release();
+}
+
 void BrowserPluginGuest::SendQueuedMessages() {
   if (!attached())
     return;
 
   while (!pending_messages_.empty()) {
     linked_ptr<IPC::Message> message_ptr = pending_messages_.front();
     pending_messages_.pop_front();
-    SendMessageToEmbedder(message_ptr.release());
+    SendMessageToEmbedder(
+        UpdateInstanceIdIfNecessary(message_ptr.release()));
   }
 }
 
 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 426 matching lines @@
           range, character_bounds);
 }
 #endif
 
 void BrowserPluginGuest::SetContextMenuPosition(const gfx::Point& position) {
   if (delegate_)
     delegate_->SetContextMenuPosition(position);
 }
 
 }  // namespace content