GuestViewManager mapped <owner WebContents, element instance ID> => guest instance ID on attachment. This routed IPCs from a given BrowserPlugin to the appropriate guest.

Element instance IDs are unique per process. This mapping is fine in Chrome Apps where the embedder doesn't navigate but not for when the embedder is capable of cross-process navigation. In that case, element instance IDs of two BrowserPlugins in two different embedder processes of the same WebContents have the same key, and would thus route to the same guest.

This is an issue because the lifetime of the exiting document overlaps with the lifetime of the entering document. Thus, racy behavior can occur. In particular, when navigating from one PDF to another, IPCs for tear down destined for the exiting BrowserPlugin can occasionally get routed to the entering BrowserPlugin. In bug 436339's case, the first step of tear down is to hide the guest content. That IPC ends up going to the entering guest, and so the new PDF is not displayed on screen.

This CL fixes the issue by using <embedder process id, element instance ID> as the key to map to a guest instead of the embedder WebContents as the first component.

BUG=436339

Review URL: https://codereview.chromium.org/921473006

Cr-Commit-Position: refs/heads/master@{#316328}

3 files changed, 1 insertions, 5 deletions

diff --git a/extensions/renderer/guest_view/extensions_guest_view_container.cc b/extensions/renderer/guest_view/extensions_guest_view_container.cc
index 6f2dd0e..77456ab 100644
--- a/extensions/renderer/guest_view/extensions_guest_view_container.cc
+++ b/extensions/renderer/guest_view/extensions_guest_view_container.cc
@@ -62,8 +62,7 @@ void ExtensionsGuestViewContainer::AttachRequest::PerformRequest() {
 
   // Step 1, send the attach params to extensions/.
   container()->render_frame()->Send(
-      new GuestViewHostMsg_AttachGuest(container()->render_view_routing_id(),
-                                       container()->element_instance_id(),
+      new GuestViewHostMsg_AttachGuest(container()->element_instance_id(),
                                        guest_instance_id_,
                                        *params_));
 
diff --git a/extensions/renderer/guest_view/guest_view_container.cc b/extensions/renderer/guest_view/guest_view_container.cc
index 636acb8..90e94c8 100644
--- a/extensions/renderer/guest_view/guest_view_container.cc
+++ b/extensions/renderer/guest_view/guest_view_container.cc
@@ -39,7 +39,6 @@ void GuestViewContainer::RenderFrameLifetimeObserver::OnDestruct() {
 
 GuestViewContainer::GuestViewContainer(content::RenderFrame* render_frame)
     : element_instance_id_(guestview::kInstanceIDNone),
-      render_view_routing_id_(render_frame->GetRenderView()->GetRoutingID()),
       render_frame_(render_frame) {
   render_frame_lifetime_observer_.reset(
       new RenderFrameLifetimeObserver(this, render_frame_));
diff --git a/extensions/renderer/guest_view/guest_view_container.h b/extensions/renderer/guest_view/guest_view_container.h
index 85af57a..42ec6e14 100644
--- a/extensions/renderer/guest_view/guest_view_container.h
+++ b/extensions/renderer/guest_view/guest_view_container.h
@@ -25,7 +25,6 @@ class GuestViewContainer : public content::BrowserPluginDelegate {
   void SetElementInstanceID(int element_instance_id) override;
 
   int element_instance_id() const { return element_instance_id_; }
-  int render_view_routing_id() const { return render_view_routing_id_; }
   content::RenderFrame* render_frame() const { return render_frame_; }
 
   virtual void OnRenderFrameDestroyed() {}
@@ -34,7 +33,6 @@ class GuestViewContainer : public content::BrowserPluginDelegate {
   class RenderFrameLifetimeObserver;
 
   int element_instance_id_;
-  const int render_view_routing_id_;
   content::RenderFrame* render_frame_;
   scoped_ptr<RenderFrameLifetimeObserver> render_frame_lifetime_observer_;