Address a crash under -[NSWindow close] via a WeakPtr PostTask from Browser::TabStripEmpty()

chrome/browser/ui/cocoa/browser_window_controller.mm

 // Copyright 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.
 
 #import "chrome/browser/ui/cocoa/browser_window_controller.h"
 
 #include <cmath>
 #include <numeric>
 #include <utility>
 
@@ skipping 512 matching lines @@
              afterDelay:0];
 }
 
 // Called when the window meets the criteria to be closed (ie,
 // |-windowShouldClose:| returns YES). We must be careful to preserve the
 // semantics of BrowserWindow::Close() and not call the Browser's dtor directly
 // from this method.
 - (void)windowWillClose:(NSNotification*)notification {
   DCHECK_EQ([notification object], [self window]);
   DCHECK(browser_->tab_strip_model()->empty());
+  windowShim_->OnWindowWillClose();
   [savedRegularWindow_ close];
   // We delete statusBubble here because we need to kill off the dependency
   // that its window has on our window before our window goes away.
   delete statusBubble_;
   statusBubble_ = NULL;
   // We can't actually use |-autorelease| here because there's an embedded
   // run loop in the |-performClose:| which contains its own autorelease pool.
   // Instead call it after a zero-length delay, which gets us back to the main
   // event loop.
   [self performSelector:@selector(autorelease)
              withObject:nil
              afterDelay:0];
 }

[Robert Sesek, 2016/06/07 16:43:35]

What about:

[[self window] performSelector:@selector(autorelease) withObject:nil
afterDelay:0];
[self setWindow:nil];

instead?

[tapted, 2016/06/08 00:00:29]

So with this approach, I get a "guaranteed" double-free when the window is
closed -- both under __CFRunLoopDoTimer. The backtrace from dealloc is from
_ZN12_GLOBAL__N_119AutoreleasePoolPage3popEPv and the zombie is from
CFRunLoopTimerInvalidate (not under
__CFRUNLOOP_IS_CALLING_OUT_TO_A_TIMER_CALLBACK_FUNCTION__).

So I think the autorelease is triggering a dealloc, then the strong reference in
the closure is trying to do another one.

I'm now thinking the part that is broken is the
performSelector:@selector(autorelease) bit. The runtime has no way of knowing a
delayed autorelease is hanging around when the autoreleasepool is drained, so
this seems fundamentally broken.

But if the closure has a strong reference it doesn't really matter what task we
post, so long as it isn't a release.  (but also if the closure has a strong
reference, why is the autorelease pool trying to dealloc it in the first place
:| ). 

[tapted, 2016/06/08 00:07:42]

Ah - derp. No this is actually just a boring old double-free. We need to [window
retain] before we can [window autorelease] :p. Browser windows don't use
setReleasedWhenClosed:NO, so the NSWindow releases itself shortly after this
method returns.

Now go trybots go!

[Robert Sesek, 2016/06/08 00:09:00]

That makes sense, actually. What I wrote would be an over-release because we
transfer ownership of the window to the controller when it's created
(https://cs.chromium.org/chromium/src/chrome/browser/ui/cocoa/tabs/tab_window_...).
So calling setWindow:nil will release that ref.

To correct that, I think it should be:

NSWindow* window = [[self window] retain];
[self setWindow:nil];
[window performSelector:@selector(autorelease) withObject:nil afterDelay:0];

 
 - (void)updateDevToolsForContents:(WebContents*)contents {
   BOOL layout_changed =
       [devToolsController_ updateDevToolsForWebContents:contents
                                             withProfile:browser_->profile()];
   if (layout_changed && [findBarCocoaController_ isFindBarVisible])
     [self layoutSubviews];
 }
 
 // Called when the user wants to close a window or from the shutdown process.
@@ skipping 1500 matching lines @@
 
 - (BOOL)isTabbedWindow {
   return browser_->is_type_tabbed();
 }
 
 - (NSRect)savedRegularWindowFrame {
   return savedRegularWindowFrame_;
 }
 
 @end  // @implementation BrowserWindowController(WindowType)