Fix for 'Simple Adblock' extension crashes

content/renderer/render_widget.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/renderer/render_widget.h"
 
 #include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
@@ skipping 687 matching lines @@
 void RenderWidget::ResizeSynchronously(const gfx::Rect& new_position) {
   Resize(new_position.size(), new_position.size(), overdraw_bottom_height_,
          visible_viewport_size_, gfx::Rect(), is_fullscreen_, NO_RESIZE_ACK);
   view_screen_rect_ = new_position;
   window_screen_rect_ = new_position;
   if (!did_show_)
     initial_pos_ = new_position;
 }
 
 void RenderWidget::OnClose() {
-  if (closing_)
+  if (closing_) {
+    RenderThread::Get()->Send(new ViewHostMsg_Close_ACK(routing_id_));

[jsbell, 2014/06/03 16:56:39]

Since I'm new to the RenderViewImpl/RenderWidget code, can you explain why the
ACK is only sent out if OnClose on the second (or later) call?

[michaeln, 2014/06/03 18:58:09]

Its not, the ACK is normally sent in RenderView::Close. I've added it here too
because i noticed the odd early exit and wanted to send the ACK in more cases.
This still doesn't cover all cases because iff the View/Widget has been deleted
and the route removed, the Close msg will go unACK'd.

Since I'm new to the RenderViewImpl/RenderWidget too, I don't know the
circumstances in which closing_would be true or the view/widget would be deleted
prior to the Close msg arriving. Notice on line 718 the route id is removed,
given that line i don't see how the early exit makes sense. Also this is a
message handler that receives a routed message, so the test for routing_id_
against MSG_ROUTING_NONE looks odd to me.

Give those mysterious things, maybe i should not piggy pack an ack on the Close
msg, and instead define a new pair of ping/pong msgs to know when its safe to
release the namespaces. wdyt?

[michaeln, 2014/06/03 23:16:57]

I've adjusted this so both callsites for sending the ack are in RenderViewImpl.

[jsbell, 2014/06/03 23:29:30]

Ah yes. Definitely need someone more familiar with the code. But the latest
patch makes this concern mostly go away.

     return;
+  }
   closing_ = true;
 
   // Browser correspondence is no longer needed at this point.
   if (routing_id_ != MSG_ROUTING_NONE) {
     if (RenderThreadImpl::current())
       RenderThreadImpl::current()->WidgetDestroyed();
     RenderThread::Get()->RemoveRoute(routing_id_);
     SetHidden(false);
   }
 
@@ skipping 1412 matching lines @@
 void RenderWidget::RegisterVideoHoleFrame(RenderFrameImpl* frame) {
   video_hole_frames_.AddObserver(frame);
 }
 
 void RenderWidget::UnregisterVideoHoleFrame(RenderFrameImpl* frame) {
   video_hole_frames_.RemoveObserver(frame);
 }
 #endif  // defined(VIDEO_HOLE)
 
 }  // namespace content