Use WeakPtr to RenderWidgetHostViewBase in InputEventRouter.

content/browser/renderer_host/render_widget_host_input_event_router.cc

 // Copyright 2015 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_input_event_router.h"
 
 #include "cc/surfaces/surface_manager.h"
 #include "content/browser/renderer_host/render_widget_host_view_base.h"
 #include "third_party/WebKit/public/web/WebInputEvent.h"
 
 namespace content {
 
 RenderWidgetHostInputEventRouter::RenderWidgetHostInputEventRouter()
-    : current_touch_target_(nullptr), active_touches_(0) {}
+    : has_current_touch_target_(false), active_touches_(0) {}
 
 RenderWidgetHostInputEventRouter::~RenderWidgetHostInputEventRouter() {
   owner_map_.clear();
 }
 
-RenderWidgetHostViewBase* RenderWidgetHostInputEventRouter::FindEventTarget(
+RenderWidgetHostInputEventRouter::WeakTarget

[nasko, 2015/12/22 18:16:33]

Why does this method need to return the WeakTarget type? Can't it return a the
pointer type, since it needs to be checked by the callers anyway.

[wjmaclean, 2015/12/22 18:39:55]

It doesn't have to return the WeakPtr ... I'll change this.

+RenderWidgetHostInputEventRouter::FindEventTarget(
     RenderWidgetHostViewBase* root_view,
     const gfx::Point& point,
     gfx::Point* transformed_point) {
   // Short circuit if owner_map has only one RenderWidgetHostView, no need for
   // hit testing.
   if (owner_map_.size() <= 1) {
     *transformed_point = point;
-    return root_view;
+    return root_view->GetWeakPtr();
   }
 
   // The conversion of point to transform_point is done over the course of the
   // hit testing, and reflect transformations that would normally be applied in
   // the renderer process if the event was being routed between frames within a
   // single process with only one RenderWidgetHost.
   uint32_t surface_id_namespace =
       root_view->SurfaceIdNamespaceAtPoint(point, transformed_point);
   const SurfaceIdNamespaceOwnerMap::iterator iter =
       owner_map_.find(surface_id_namespace);
   // If the point hit a Surface whose namspace is no longer in the map, then
   // it likely means the RenderWidgetHostView has been destroyed but its
   // parent frame has not sent a new compositor frame since that happened.
   if (iter == owner_map_.end())
-    return root_view;
+    return root_view->GetWeakPtr();
   return iter->second;
 }
 
 void RenderWidgetHostInputEventRouter::RouteMouseEvent(
     RenderWidgetHostViewBase* root_view,
     blink::WebMouseEvent* event) {
   gfx::Point transformed_point;
-  RenderWidgetHostViewBase* target = FindEventTarget(
-      root_view, gfx::Point(event->x, event->y), &transformed_point);
+  WeakTarget target = FindEventTarget(root_view, gfx::Point(event->x, event->y),
+                                      &transformed_point);
   event->x = transformed_point.x();
   event->y = transformed_point.y();
 
-  target->ProcessMouseEvent(*event);
+  if (target)

[nasko, 2015/12/22 18:16:33]

Let'd use early returns - if (!target) return, right after the actual call to
FindEventTarget.

[wjmaclean, 2015/12/22 18:39:55]

Done.

+    target->ProcessMouseEvent(*event);
 }
 
 void RenderWidgetHostInputEventRouter::RouteMouseWheelEvent(
     RenderWidgetHostViewBase* root_view,
     blink::WebMouseWheelEvent* event) {
   gfx::Point transformed_point;
-  RenderWidgetHostViewBase* target = FindEventTarget(
-      root_view, gfx::Point(event->x, event->y), &transformed_point);
+  WeakTarget target = FindEventTarget(root_view, gfx::Point(event->x, event->y),
+                                      &transformed_point);
   event->x = transformed_point.x();
   event->y = transformed_point.y();
 
-  target->ProcessMouseWheelEvent(*event);
+  if (target)

[nasko, 2015/12/22 18:16:33]

Same as above, early return.

[wjmaclean, 2015/12/22 18:39:55]

Done.

+    target->ProcessMouseWheelEvent(*event);
 }
 
 void RenderWidgetHostInputEventRouter::RouteTouchEvent(
     RenderWidgetHostViewBase* root_view,
     blink::WebTouchEvent* event,
     const ui::LatencyInfo& latency) {
   switch (event->type) {
     case blink::WebInputEvent::TouchStart: {
       if (!active_touches_) {
         // Since this is the first touch, it defines the target for the rest
         // of this sequence.
         DCHECK(!current_touch_target_);
         gfx::Point transformed_point;
         gfx::Point original_point(event->touches[0].position.x,
                                   event->touches[0].position.y);
         current_touch_target_ =
             FindEventTarget(root_view, original_point, &transformed_point);

[nasko, 2015/12/22 18:16:33]

You could just call ->GetWeakPtr() here, since it is the only place you really
need a weak pointer.

[wjmaclean, 2015/12/22 18:39:55]

I don't understand? You mean call it on current_touch_target? or root_view?

The former could be stale (we'll only know if we store the WeakPtr in the map).
The latter is always a value provided by the caller, so we trust it not to be
stale.

+        has_current_touch_target_ = true;

[Fady Samuel, 2015/12/22 18:06:43]

Is this flag necessary? It seems you only use it for DCHECKing?

       }
       ++active_touches_;
-      current_touch_target_->ProcessTouchEvent(*event, latency);
+      if (current_touch_target_.get())
+        current_touch_target_->ProcessTouchEvent(*event, latency);
       break;
     }
     case blink::WebInputEvent::TouchMove:
-      DCHECK(current_touch_target_);
-      current_touch_target_->ProcessTouchEvent(*event, latency);
+      DCHECK(has_current_touch_target_);
+      if (current_touch_target_.get())
+        current_touch_target_->ProcessTouchEvent(*event, latency);
       break;
     case blink::WebInputEvent::TouchEnd:
     case blink::WebInputEvent::TouchCancel:
       DCHECK(active_touches_);
-      DCHECK(current_touch_target_);
-      current_touch_target_->ProcessTouchEvent(*event, latency);
+      DCHECK(has_current_touch_target_);
+      if (current_touch_target_.get())
+        current_touch_target_->ProcessTouchEvent(*event, latency);
       --active_touches_;
-      if (!active_touches_)
-        current_touch_target_ = nullptr;
+      if (!active_touches_) {
+        current_touch_target_ = WeakTarget();
+        has_current_touch_target_ = false;
+      }
       break;
     default:
       NOTREACHED();
   }
 }
 
 void RenderWidgetHostInputEventRouter::AddSurfaceIdNamespaceOwner(
     uint32_t id,
     RenderWidgetHostViewBase* owner) {
   DCHECK(owner_map_.find(id) == owner_map_.end());
-  owner_map_.insert(std::make_pair(id, owner));
+  owner_map_.insert(std::make_pair(id, owner->GetWeakPtr()));
 }
 
 void RenderWidgetHostInputEventRouter::RemoveSurfaceIdNamespaceOwner(
     uint32_t id) {
   owner_map_.erase(id);
 }
 
 }  // namespace content