Remove null-check for |delegate_| in RWHI::GetVisualProperties().

RenderWidgetHostImpl already checks for |delegate_| before calling the
method. The other caller does so in the same stack that it uses the
WebContents, which is the delegate. The delegate is removed only when
the WebContents is destroyed, so we know it will be present in both
cases.

TBR=ajwong@chromium.org

Change-Id: Ia0fe71c00a87bb92b22ebeff545762fbcf72848d
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1443933
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628349}

content/browser/renderer_host/render_widget_host_impl.cc

@@ -850,40 +850,39 @@ void RenderWidgetHostImpl::SetImportance(ChildProcessImportance importance) {
 bool RenderWidgetHostImpl::GetVisualProperties(
     VisualProperties* visual_properties,
     bool* needs_ack) {
+  // This is only called while the RenderWidgetHost is attached to a delegate
+  // still.
+  DCHECK(delegate_);
+
   *visual_properties = VisualProperties();
 
   GetScreenInfo(&visual_properties->screen_info);
 
-  if (!delegate_) {
-    visual_properties->display_mode = blink::kWebDisplayModeBrowser;
-  } else {
-    visual_properties->is_fullscreen_granted =
-        delegate_->IsFullscreenForCurrentTab();
-    visual_properties->display_mode = delegate_->GetDisplayMode(this);
-    visual_properties->zoom_level = delegate_->GetPendingPageZoomLevel();
-
-    RenderViewHostDelegateView* rvh_delegate_view =
-        delegate_->GetDelegateView();
-    DCHECK(rvh_delegate_view);
-
-    visual_properties->browser_controls_shrink_blink_size =
-        rvh_delegate_view->DoBrowserControlsShrinkRendererSize();
-
-    float top_controls_height = rvh_delegate_view->GetTopControlsHeight();
-    float bottom_controls_height = rvh_delegate_view->GetBottomControlsHeight();
-    float browser_controls_dsf_multiplier = 1.f;
-    // The top and bottom control sizes are physical pixels but the IPC wants
-    // DIPs *when not using page zoom for DSF* because blink layout is working
-    // in DIPs then.
-    if (!IsUseZoomForDSFEnabled()) {
-      browser_controls_dsf_multiplier =
-          visual_properties->screen_info.device_scale_factor;
-    }
-    visual_properties->top_controls_height =
-        top_controls_height / browser_controls_dsf_multiplier;
-    visual_properties->bottom_controls_height =
-        bottom_controls_height / browser_controls_dsf_multiplier;
+  visual_properties->is_fullscreen_granted =
+      delegate_->IsFullscreenForCurrentTab();
+  visual_properties->display_mode = delegate_->GetDisplayMode(this);
+  visual_properties->zoom_level = delegate_->GetPendingPageZoomLevel();
+
+  RenderViewHostDelegateView* rvh_delegate_view = delegate_->GetDelegateView();
+  DCHECK(rvh_delegate_view);
+
+  visual_properties->browser_controls_shrink_blink_size =
+      rvh_delegate_view->DoBrowserControlsShrinkRendererSize();
+
+  float top_controls_height = rvh_delegate_view->GetTopControlsHeight();
+  float bottom_controls_height = rvh_delegate_view->GetBottomControlsHeight();
+  float browser_controls_dsf_multiplier = 1.f;
+  // The top and bottom control sizes are physical pixels but the IPC wants
+  // DIPs *when not using page zoom for DSF* because blink layout is working
+  // in DIPs then.
+  if (!IsUseZoomForDSFEnabled()) {
+    browser_controls_dsf_multiplier =
+        visual_properties->screen_info.device_scale_factor;
   }
+  visual_properties->top_controls_height =
+      top_controls_height / browser_controls_dsf_multiplier;
+  visual_properties->bottom_controls_height =
+      bottom_controls_height / browser_controls_dsf_multiplier;
 
   visual_properties->auto_resize_enabled = auto_resize_enabled_;
   visual_properties->min_size_for_auto_resize = min_size_for_auto_resize_;
@@ -2168,6 +2167,8 @@ void RenderWidgetHostImpl::Destroy(bool also_delete) {
   g_routing_id_widget_map.Get().erase(
       RenderWidgetHostID(process_->GetID(), routing_id_));
 
+  // The |delegate_| may have been destroyed (or is in the process of being
+  // destroyed) and detached first.
   if (delegate_)
     delegate_->RenderWidgetDeleted(this);
 

content/browser/renderer_host/render_widget_host_impl.h

@@ -510,6 +510,9 @@ class CONTENT_EXPORT RenderWidgetHostImpl
   // properties.
   void ResetSentVisualProperties();
 
+  // When the WebContents (which acts as the Delegate) is destroyed, this object
+  // may still outlive it while the renderer is shutting down. In that case the
+  // delegate pointer is removed (since it would be a UAF).
   void DetachDelegate();
 
   // Update the renderer's cache of the screen rect of the view and window.