Separate RenderViewHost from RenderWidgetHost, part 4: delegate

content/browser/renderer_host/render_view_host_impl.cc

Index: content/browser/renderer_host/render_view_host_impl.cc
diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
index 328f40b336f153c8038f25550a10e307866fb8af..a46231df77bb0fd9275694b5d30a676144e837d8 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -188,15 +188,19 @@ RenderViewHostImpl* RenderViewHostImpl::FromID(int render_process_id,
                                                int render_view_id) {
   RenderWidgetHost* widget =
       RenderWidgetHost::FromID(render_process_id, render_view_id);
-  if (!widget || !widget->IsRenderView())
-    return NULL;
-  return static_cast<RenderViewHostImpl*>(RenderWidgetHostImpl::From(widget));
+  if (!widget)
+    return nullptr;
+  return From(widget);
 }
 
 // static
 RenderViewHostImpl* RenderViewHostImpl::From(RenderWidgetHost* rwh) {
-  DCHECK(rwh->IsRenderView());
-  return static_cast<RenderViewHostImpl*>(RenderWidgetHostImpl::From(rwh));
+  DCHECK(rwh);
+  RenderWidgetHostOwnerDelegate* owner_delegate =
+      RenderWidgetHostImpl::From(rwh)->owner_delegate();
+  if (!owner_delegate)

[ncarter (slow), 2015/10/26 18:35:02]

Seems a little dirty for this code to assume it's the only owner delegate. I'm
okay with leaving it that way if we're pretty sure that
RenderWidgetHostOwnerDelegate will have a truly short life. But if we think in
the interim we might want to port other behaviors to this delegate (e.g.
RWHI::owned_by_render_frame_host_) then we should make it safer maybe?

Two options I'd be okay with:
 1. Add one of those lame-o methods
RenderWidgetHostOwnerDelegate::GetAsRenderViewHost()
 2. Or, alternatively just add here a DCHECK(result->GetWidget() == rwh) so that
we'll at least fail here if things do go south.

[Avi (use Gerrit), 2015/10/26 19:41:28]

Yes, this is the weakest part of the scheme.
BTW I want to rename that to a more general "is owned / should not delete self"
name.
That just adds the layering violation back in :(
That's fair. I do want to make that weird delegate go away quickly, FWIW.

+    return nullptr;
+  return static_cast<RenderViewHostImpl*>(owner_delegate);
 }
 
 RenderViewHostImpl::RenderViewHostImpl(
@@ -234,6 +238,8 @@ RenderViewHostImpl::RenderViewHostImpl(
   DCHECK(instance_.get());
   CHECK(delegate_);  // http://crbug.com/82827
 
+  GetWidget()->set_owner_delegate(this);
+
   GetProcess()->AddObserver(this);
   GetProcess()->EnableSendQueue();
 
@@ -1004,10 +1010,6 @@ void RenderViewHostImpl::WasShown(const ui::LatencyInfo& latency_info) {
   RenderWidgetHostImpl::WasShown(latency_info);
 }
 
-bool RenderViewHostImpl::IsRenderView() const {
-  return true;
-}
-
 void RenderViewHostImpl::CreateNewWindow(
     int route_id,
     int main_frame_route_id,