Speculative fix for OnSwapCompositorFrame crasher.

content/browser/frame_host/render_widget_host_view_guest.cc

 // Copyright 2014 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/frame_host/render_widget_host_view_guest.h"
 
 #include <utility>
 
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
@@ skipping 265 matching lines @@
 
   // Check whether we need to recreate the cc::Surface, which means the child
   // frame renderer has changed its output surface, or size, or scale factor.
   if (output_surface_id != last_output_surface_id_ && surface_factory_) {
     surface_factory_->Destroy(surface_id_);
     surface_factory_.reset();
   }
   if (output_surface_id != last_output_surface_id_ ||
       frame_size != current_surface_size_ ||
       scale_factor != current_surface_scale_factor_ ||
-      guest_->has_attached_since_surface_set()) {
+      (guest_ && guest_->has_attached_since_surface_set())) {
     ClearCompositorSurfaceIfNecessary();
     last_output_surface_id_ = output_surface_id;
     current_surface_size_ = frame_size;
     current_surface_scale_factor_ = scale_factor;
   }
 
   if (!surface_factory_) {
     cc::SurfaceManager* manager = GetSurfaceManager();
     surface_factory_ = base::MakeUnique<cc::SurfaceFactory>(manager, this);
   }
 
   if (surface_id_.is_null()) {
     surface_id_ = id_allocator_->GenerateId();
     surface_factory_->Create(surface_id_);
 
     cc::SurfaceSequence sequence = cc::SurfaceSequence(
         id_allocator_->client_id(), next_surface_sequence_++);
     // The renderer process will satisfy this dependency when it creates a
     // SurfaceLayer.
     cc::SurfaceManager* manager = GetSurfaceManager();
     manager->GetSurfaceForId(surface_id_)->AddDestructionDependency(sequence);
-    guest_->SetChildFrameSurface(surface_id_, frame_size, scale_factor,
-                                 sequence);
+    // TODO(wjmaclean): I'm not sure what it means to create a surface id
+    // without setting it on the child, though since we will in this case be
+    // guaranteed to call ClearCompositorSurfaceIfNecessary() below, I suspect
+    // skipping SetChildFrameSurface() here is irrelevant.
+    if (guest_ && !guest_->is_in_destruction()) {

[Charlie Reis, 2016/09/19 17:29:21]

If the crash is actually a UaF on guest_, then neither a null check nor a method
call on guest_ will avoid it.  We'll just get the UaF on this line instead, when
calling is_in_destruction.

An actual UaF on guest_ might suggest that |this| has been deleted, but we
probably would have crashed much earlier in this method if so.  Thus, I'm not
sure what to recommend.

+      guest_->SetChildFrameSurface(surface_id_, frame_size, scale_factor,
+                                   sequence);
+    }
   }
 
   cc::SurfaceFactory::DrawCallback ack_callback = base::Bind(
       &RenderWidgetHostViewChildFrame::SurfaceDrawn,
       RenderWidgetHostViewChildFrame::AsWeakPtr(), output_surface_id);
   ack_pending_count_++;
   // If this value grows very large, something is going wrong.
   DCHECK(ack_pending_count_ < 1000);
   surface_factory_->SubmitCompositorFrame(surface_id_, std::move(frame),
                                           ack_callback);
@@ skipping 344 matching lines @@
         gesture_event.data.scrollUpdate.inertialPhase ==
             blink::WebGestureEvent::MomentumPhase) {
       return;
     }
     host_->ForwardGestureEvent(gesture_event);
     return;
   }
 }
 
 }  // namespace content