Fix mapping of routing IDs to browser objects (issue #1012).

git-svn-id: https://chromiumembedded.googlecode.com/svn/trunk@1305 5089003a-bbd8-11dd-ad1f-f1f9622dbc98

libcef/browser/browser_host_impl.cc

@@ -388,11 +388,12 @@ CefRefPtr<CefBrowserHostImpl> CefBrowserHostImpl::GetBrowserForRequest(
   DCHECK(request);
   CEF_REQUIRE_IOT();
   int render_process_id = -1;
-  int render_view_id = -1;
+  int render_view_id = MSG_ROUTING_NONE;
 
-  if (!content::ResourceRequestInfo::GetRenderViewForRequest(request,
-                                                             &render_process_id,
-                                                             &render_view_id)) {
+  if (!content::ResourceRequestInfo::GetRenderViewForRequest(
+          request, &render_process_id, &render_view_id) ||
+      render_process_id == -1 ||
+      render_view_id == MSG_ROUTING_NONE) {
     return NULL;
   }
 
@@ -402,6 +403,9 @@ CefRefPtr<CefBrowserHostImpl> CefBrowserHostImpl::GetBrowserForRequest(
 // static
 CefRefPtr<CefBrowserHostImpl> CefBrowserHostImpl::GetBrowserByRoutingID(
     int render_process_id, int render_view_id) {
+  if (render_process_id == -1 || render_view_id == MSG_ROUTING_NONE)
+    return NULL;
+
   if (CEF_CURRENTLY_ON_UIT()) {
     // Use the non-thread-safe but potentially faster approach.
     content::RenderViewHost* render_view_host =
@@ -445,7 +449,7 @@ CefRefPtr<CefBrowserHostImpl> CefBrowserHostImpl::GetBrowserByChildID(
     return NULL;
   } else {
     // Use the thread-safe approach.
-    return _Context->GetBrowserByRoutingID(render_process_id, 0);
+    return _Context->GetBrowserByRoutingID(render_process_id, -1);
   }
 }
 
@@ -1793,11 +1797,8 @@ void CefBrowserHostImpl::RequestMediaAccessPermission(
 
 void CefBrowserHostImpl::RenderViewCreated(
     content::RenderViewHost* render_view_host) {
-  // When navigating cross-origin the new (pending) RenderViewHost will be
-  // created before the old (current) RenderViewHost is destroyed. It may be
-  // necessary in the future to track both current and pending render IDs.
-  browser_info_->set_render_ids(render_view_host->GetProcess()->GetID(),
-                                render_view_host->GetRoutingID());
+  browser_info_->add_render_id(render_view_host->GetProcess()->GetID(),
+                               render_view_host->GetRoutingID());
 
   // Update the DevTools URLs, if any.
   CefDevToolsDelegate* devtools_delegate = _Context->devtools_delegate();
@@ -1820,6 +1821,9 @@ void CefBrowserHostImpl::RenderViewCreated(
 
 void CefBrowserHostImpl::RenderViewDeleted(
     content::RenderViewHost* render_view_host) {
+  browser_info_->remove_render_id(render_view_host->GetProcess()->GetID(),
+                                  render_view_host->GetRoutingID());
+
   if (registrar_->IsRegistered(
       this, content::NOTIFICATION_FOCUS_CHANGED_IN_PAGE,
       content::Source<content::RenderViewHost>(render_view_host))) {

libcef/browser/browser_host_impl.h

@@ -100,7 +100,9 @@ class CefBrowserHostImpl : public CefBrowserHost,
   // Returns the browser associated with the specified routing IDs.
   static CefRefPtr<CefBrowserHostImpl> GetBrowserByRoutingID(
       int render_process_id, int render_view_id);
-  // Returns the browser associated with the specified child process ID.
+  // Returns the first browser associated with the specified child process ID.
+  // There may be multiple browsers using the same render process so this method
+  // should be used with caution.
   static CefRefPtr<CefBrowserHostImpl> GetBrowserByChildID(
       int render_process_id);
 

libcef/browser/browser_info.cc

@@ -8,9 +8,7 @@
 CefBrowserInfo::CefBrowserInfo(int browser_id, bool is_popup)
     : browser_id_(browser_id),
       is_popup_(is_popup),
-      is_window_rendering_disabled_(false),
-      render_process_id_(MSG_ROUTING_NONE),
-      render_view_id_(MSG_ROUTING_NONE) {
+      is_window_rendering_disabled_(false) {
   DCHECK_GT(browser_id, 0);
 }
 
@@ -21,19 +19,51 @@ void CefBrowserInfo::set_window_rendering_disabled(bool disabled) {
   is_window_rendering_disabled_ = disabled;
 }
 
-void CefBrowserInfo::set_render_ids(
+void CefBrowserInfo::add_render_id(
     int render_process_id, int render_view_id) {
+  DCHECK_GT(render_process_id, 0);
+  DCHECK_GT(render_view_id, 0);
+
   base::AutoLock lock_scope(lock_);
-  render_process_id_ = render_process_id;
-  render_view_id_ = render_view_id;
+
+  if (!render_id_set_.empty()) {
+    RenderIdSet::const_iterator it =
+        render_id_set_.find(std::make_pair(render_process_id, render_view_id));
+    if (it != render_id_set_.end())
+      return;
+  }
+
+  render_id_set_.insert(std::make_pair(render_process_id, render_view_id));
+}
+
+void CefBrowserInfo::remove_render_id(
+    int render_process_id, int render_view_id) {
+  DCHECK_GT(render_process_id, 0);
+  DCHECK_GT(render_view_id, 0);
+
+  base::AutoLock lock_scope(lock_);
+
+  DCHECK(!render_id_set_.empty());
+  if (render_id_set_.empty())
+    return;
+
+  RenderIdSet::iterator it =
+      render_id_set_.find(std::make_pair(render_process_id, render_view_id));
+  DCHECK(it != render_id_set_.end());
+  if (it != render_id_set_.end())
+    render_id_set_.erase(it);
 }
 
 bool CefBrowserInfo::is_render_id_match(
     int render_process_id, int render_view_id) {
   base::AutoLock lock_scope(lock_);
-  if (render_process_id != render_process_id_)
+
+  if (render_id_set_.empty())
     return false;
-  return (render_view_id == 0 || render_view_id == render_view_id_);
+
+  RenderIdSet::const_iterator it =
+      render_id_set_.find(std::make_pair(render_process_id, render_view_id));
+  return (it != render_id_set_.end());
 }
 
 CefRefPtr<CefBrowserHostImpl> CefBrowserInfo::browser() {

libcef/browser/browser_info.h

@@ -6,6 +6,8 @@
 #define CEF_LIBCEF_BROWSER_BROWSER_INFO_H_
 #pragma once
 
+#include <set>
+
 #include "libcef/browser/browser_host_impl.h"
 #include "base/memory/ref_counted.h"
 
@@ -28,10 +30,11 @@ class CefBrowserInfo : public base::RefCountedThreadSafe<CefBrowserInfo> {
 
   void set_window_rendering_disabled(bool disabled);
   
-  void set_render_ids(int render_process_id, int render_view_id);
+  void add_render_id(int render_process_id, int render_view_id);
+  void remove_render_id(int render_process_id, int render_view_id);
 
   // Returns true if this browser matches the specified ID values. If
-  // |render_view_id| is 0 any browser with the specified |render_process_id|
+  // |render_view_id| is -1 any browser with the specified |render_process_id|
   // will match.
   bool is_render_id_match(int render_process_id, int render_view_id);
 
@@ -46,8 +49,18 @@ class CefBrowserInfo : public base::RefCountedThreadSafe<CefBrowserInfo> {
   base::Lock lock_;
 
   // The below members must be protected by |lock_|.
-  int render_process_id_;
-  int render_view_id_;
+
+  // Set of mapped (process_id, view_id) pairs. Keeping this set is necessary
+  // for the following reasons:
+  // 1. When navigating cross-origin the new (pending) RenderViewHost will be
+  //    created before the old (current) RenderViewHost is destroyed.
+  // 2. When canceling and asynchronously continuing navigation of the same URL
+  //    a new RenderViewHost may be created for the first (canceled) navigation
+  //    and then destroyed as a result of the second (allowed) navigation.
+  // 3. Out-of-process iframes have their own render IDs which must also be
+  //    associated with the host browser.
+  typedef std::set<std::pair<int, int> > RenderIdSet;
+  RenderIdSet render_id_set_;
 
   // May be NULL if the browser has not yet been created or if the browser has
   // been destroyed.

libcef/browser/content_browser_client.cc

@@ -290,7 +290,7 @@ scoped_refptr<CefBrowserInfo>
   // Must be a popup if it hasn't already been created.
   scoped_refptr<CefBrowserInfo> browser_info =
       new CefBrowserInfo(++next_browser_id_, true);
-  browser_info->set_render_ids(render_process_id, render_view_id);
+  browser_info->add_render_id(render_process_id, render_view_id);
   browser_info_list_.push_back(browser_info);
   return browser_info;
 }