Refactor RenderProcessHost to use IPC::Listener instead of RenderWidgetHost

content/browser/renderer_host/render_widget_host_impl.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/renderer_host/render_widget_host_impl.h"
 
 #include <math.h>
 #include <utility>
 
 #include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
+#include "base/hash_tables.h"
 #include "base/i18n/rtl.h"
 #include "base/lazy_instance.h"
 #include "base/message_loop.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
 #include "base/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "cc/output/compositor_frame.h"
 #include "cc/output/compositor_frame_ack.h"
 #include "content/browser/accessibility/browser_accessibility_state_impl.h"
@@ skipping 51 matching lines @@
 using base::TimeDelta;
 using base::TimeTicks;
 using webkit::npapi::WebPluginDelegateImpl;
 using WebKit::WebGestureEvent;
 using WebKit::WebInputEvent;
 using WebKit::WebKeyboardEvent;
 using WebKit::WebMouseEvent;
 using WebKit::WebMouseWheelEvent;
 using WebKit::WebTextDirection;
 
+namespace {
+
+struct RenderWidgetID {
+  RenderWidgetID(int32 process_id, int32 routing_id)
+    : process_id_(process_id),
+      routing_id_(routing_id) {}
+  size_t hash() const {
+    return (process_id_ << sizeof(int32)) | routing_id_;

[dcheng, 2013/06/11 00:34:58]

Is it intentional that you're only left shifting by 4?

+  }
+  bool operator==(const RenderWidgetID& rwh) const {
+    return process_id_ ==  rwh.process_id_ && routing_id_ == rwh.routing_id_;
+  }
+  bool operator<(const RenderWidgetID& rwh) const {
+    return hash() < rwh.hash();
+  }
+  int32 process_id_;

[dcheng, 2013/06/11 00:34:58]

Nit: No _ when naming structs.
Also, I might personally just use a union here but that's just preference (I
don't know what convention is).

+  int32 routing_id_;
+};
+
+} //  namespace
+
+namespace BASE_HASH_NAMESPACE {
+#if defined(COMPILER_GCC)
+template <>
+struct hash<RenderWidgetID> {
+  size_t operator()(const RenderWidgetID& rwh) const {
+    return rwh.hash();
+  }
+};
+#elif defined(COMPILER_MSVC)
+inline size_t hash_value(const RenderWidgetID& rwh) {
+  return rwh.hash();
+}
+#endif
+}  // namespace BASE_HASH_NAMESPACE
+
+
 namespace content {
 namespace {
 
 bool g_check_for_pending_resize_ack = true;
 
 // How long to (synchronously) wait for the renderer to respond with a
 // PaintRect message, when our backing-store is invalid, before giving up and
 // returning a null or incorrectly sized backing-store from GetBackingStore.
 // This timeout impacts the "choppiness" of our window resize perf.
 const int kPaintMsgTimeoutMS = 50;
@@ skipping 15 matching lines @@
 
 float GetAccelerationRatio(float accelerated_delta, float unaccelerated_delta) {
   if (unaccelerated_delta == 0.f || accelerated_delta == 0.f)
     return 1.f;
   return unaccelerated_delta / accelerated_delta;
 }
 
 base::LazyInstance<std::vector<RenderWidgetHost::CreatedCallback> >
 g_created_callbacks = LAZY_INSTANCE_INITIALIZER;
 
+
 }  // namespace
 
 
+typedef base::hash_map<RenderWidgetID, RenderWidgetHostImpl*>
+    RoutingIDWidgetMap;
+static base::LazyInstance<RoutingIDWidgetMap> g_routing_id_widget_map =
+    LAZY_INSTANCE_INITIALIZER;
+
 // static
 void RenderWidgetHost::RemoveAllBackingStores() {
   BackingStoreManager::RemoveAllBackingStores();
 }
 
 // static
 size_t RenderWidgetHost::BackingStoreMemorySize() {
   return BackingStoreManager::MemorySize();
 }
 
@@ skipping 58 matching lines @@
     // problem, for example by tracking in the RenderWidgetHelper the routing id
     // (and surface id) that have been created, but whose RWH haven't yet.
     surface_id_ = GpuSurfaceTracker::Get()->LookupSurfaceForRenderer(
         process_->GetID(),
         routing_id_);
     DCHECK(surface_id_);
   }
 
   is_threaded_compositing_enabled_ = IsThreadedCompositingEnabled();
 
-  process_->Attach(this, routing_id_);
+  g_routing_id_widget_map.Get().insert(std::make_pair(
+      RenderWidgetID(process->GetID(), routing_id_), this));
+  process_->AddRoute(routing_id_, this);
   // Because the widget initializes as is_hidden_ == false,
   // tell the process host that we're alive.
   process_->WidgetRestored();
 
   accessibility_mode_ =
       BrowserAccessibilityStateImpl::GetInstance()->accessibility_mode();
 
   for (size_t i = 0; i < g_created_callbacks.Get().size(); i++)
     g_created_callbacks.Get().at(i).Run(this);
 
 #if defined(USE_AURA)
   bool overscroll_enabled = CommandLine::ForCurrentProcess()->
       GetSwitchValueASCII(switches::kOverscrollHistoryNavigation) == "1";
   SetOverscrollControllerEnabled(overscroll_enabled);
 #endif
 }
 
 RenderWidgetHostImpl::~RenderWidgetHostImpl() {
   SetView(NULL);
 
   // Clear our current or cached backing store if either remains.
   BackingStoreManager::RemoveBackingStore(this);
 
   GpuSurfaceTracker::Get()->RemoveSurface(surface_id_);
   surface_id_ = 0;
 
-  process_->Release(routing_id_);
+  process_->RemoveRoute(routing_id_);
+  g_routing_id_widget_map.Get().erase(
+      RenderWidgetID(process_->GetID(), routing_id_));
 
   if (delegate_)
     delegate_->RenderWidgetDeleted(this);
 }
 
 // static
+RenderWidgetHost* RenderWidgetHost::FromID(
+    int32 process_id,
+    int32 routing_id) {
+  return RenderWidgetHostImpl::FromID(process_id, routing_id);
+}
+
+// static
+RenderWidgetHostImpl* RenderWidgetHostImpl::FromID(
+    int32 process_id,
+    int32 routing_id) {
+  RoutingIDWidgetMap* widgets = g_routing_id_widget_map.Pointer();
+  RoutingIDWidgetMap::iterator it = widgets->find(
+      RenderWidgetID(process_id, routing_id));
+  return it == widgets->end() ? NULL : it->second;
+}
+
+// static
+scoped_ptr<std::vector<RenderWidgetHost*> >
+RenderWidgetHost::GetRenderWidgetHosts() {

[dcheng, 2013/06/11 00:34:58]

From what I can see, it's fine to just use return by value here (or pass a
pointer to the output hash_map). Note that a lot of code in Chrome already
depends on RVO though; for example,
render_view_devtools_agent_host.cc:DevToolsAgentHost::GetValidRenderViewHosts,
which your patch touches, depends on RVO.

Note another advantage to using RVO is you could make the returned value const
if you want to insure that it is not accidentally modified, which you can't do
if you pass it in by pointer.

+  scoped_ptr<std::vector<RenderWidgetHost*> > hosts(
+      new std::vector<RenderWidgetHost*>());
+  RoutingIDWidgetMap* widgets = g_routing_id_widget_map.Pointer();
+  for (RoutingIDWidgetMap::const_iterator it = widgets->begin();
+       it != widgets->end();
+       ++it) {
+    hosts->push_back(it->second);
+  }
+  return hosts.Pass();
+}
+
+// static
 RenderWidgetHostImpl* RenderWidgetHostImpl::From(RenderWidgetHost* rwh) {
   return rwh->AsRenderWidgetHostImpl();
 }
 
 // static
 void RenderWidgetHost::AddCreatedCallback(const CreatedCallback& callback) {
   g_created_callbacks.Get().push_back(callback);
 }
 
 // static
@@ skipping 2291 matching lines @@
     const ui::LatencyInfo& latency_info) {
   for (ui::LatencyInfo::LatencyMap::const_iterator b =
            latency_info.latency_components.begin();
        b != latency_info.latency_components.end();
        ++b) {
     if (b->first.first != ui::INPUT_EVENT_LATENCY_COMPONENT)
       continue;
     // Matches with GetLatencyComponentId
     int routing_id = b->first.second & 0xffffffff;
     int process_id = (b->first.second >> 32) & 0xffffffff;
-    RenderProcessHost* host = RenderProcessHost::FromID(process_id);
-    if (!host)
-      continue;
-    RenderWidgetHost* rwh = host->GetRenderWidgetHostByID(routing_id);
+    RenderWidgetHost* rwh =
+        RenderWidgetHost::FromID(process_id, routing_id);
     if (!rwh)
       continue;
     RenderWidgetHostImpl::From(rwh)->FrameSwapped(latency_info);
   }
 }
 
 }  // namespace content