Prevent a crash in TaskManagerWindowController by nil-ing the dataSource and delegate in dealloc.

Prevent a crash in TaskManagerWindowController by nil-ing the dataSource and delegate in dealloc.

BUG=61669
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=64907

[Nico, 2010-11-03 03:18:19]

LG

You don't happen to know what in appkit is keeping this reference around? :-)

On Tue, Nov 2, 2010 at 8:55 PM,  <rsesek@chromium.org> wrote:
> Reviewers: Nico,
>
> Description:
> Prevent a crash in TaskManagerWindowController by nil-ing the dataSource and
> delegate in dealloc.
>
> BUG=61669
> TEST=none
>
> Please review this at http://codereview.chromium.org/4234005/show
>
> SVN Base: svn://svn.chromium.org/chrome/trunk/src
>
> Affected files:
>  M chrome/browser/cocoa/task_manager_mac.mm
>
>
> Index: chrome/browser/cocoa/task_manager_mac.mm
> diff --git a/chrome/browser/cocoa/task_manager_mac.mm
> b/chrome/browser/cocoa/task_manager_mac.mm
> index
>
b4cee6673715ea86fd96224e36db1c591dac01a3..65241d2e604142c313e622ea33fe690c5df3f884
> 100644
> --- a/chrome/browser/cocoa/task_manager_mac.mm
> +++ b/chrome/browser/cocoa/task_manager_mac.mm
> @@ -195,6 +195,12 @@ class SortHelper {
>   [tableView_ sizeToFit];
>  }
>
> +- (void)dealloc {
> +  [tableView_ setDelegate:nil];
> +  [tableView_ setDataSource:nil];
> +  [super dealloc];
> +}
> +
>  // Adds a column which has the given string id as title. |isVisible|
> specifies
>  // if the column is initially visible.
>  - (NSTableColumn*)addColumnWithId:(int)columnId visible:(BOOL)isVisible {
>
>
>

[Scott Hess - ex-Googler, 2010-11-03 05:15:35]

LGTM2.

I think that the problem is the [self autorelease] in -windowWillClose: may be
out-of-order WRT when the window (and contents) get released.  There are cases
where windows process needs-display after close.  I don't fully understand why
this happens, but I've been bitten by it multiple times (in other contexts).

I'm pretty sure delegate/datasource are not weak references.

-scott



On 2010/11/03 03:18:19, Nico wrote:
> LG
> 
> You don't happen to know what in appkit is keeping this reference around? :-)
> 
> On Tue, Nov 2, 2010 at 8:55 PM,  <mailto:rsesek@chromium.org> wrote:
> > Reviewers: Nico,
> >
> > Description:
> > Prevent a crash in TaskManagerWindowController by nil-ing the dataSource and
> > delegate in dealloc.
> >
> > BUG=61669
> > TEST=none
> >
> > Please review this at http://codereview.chromium.org/4234005/show
> >
> > SVN Base: svn://svn.chromium.org/chrome/trunk/src
> >
> > Affected files:
> > &nbsp;M chrome/browser/cocoa/task_manager_mac.mm
> >
> >
> > Index: chrome/browser/cocoa/task_manager_mac.mm
> > diff --git a/chrome/browser/cocoa/task_manager_mac.mm
> > b/chrome/browser/cocoa/task_manager_mac.mm
> > index
> >
>
b4cee6673715ea86fd96224e36db1c591dac01a3..65241d2e604142c313e622ea33fe690c5df3f884
> > 100644
> > --- a/chrome/browser/cocoa/task_manager_mac.mm
> > +++ b/chrome/browser/cocoa/task_manager_mac.mm
> > @@ -195,6 +195,12 @@ class SortHelper {
> > &nbsp; [tableView_ sizeToFit];
> > &nbsp;}
> >
> > +- (void)dealloc {
> > + &nbsp;[tableView_ setDelegate:nil];
> > + &nbsp;[tableView_ setDataSource:nil];
> > + &nbsp;[super dealloc];
> > +}
> > +
> > &nbsp;// Adds a column which has the given string id as title. |isVisible|
> > specifies
> > &nbsp;// if the column is initially visible.
> > &nbsp;- (NSTableColumn*)addColumnWithId:(int)columnId
visible:(BOOL)isVisible {
> >
> >
> >
>