[Mac] Fix keyword editor-related crashes

chrome/browser/cocoa/keyword_editor_cocoa_controller.mm

 // Copyright (c) 2009 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 <Cocoa/Cocoa.h>
 
 #import "base/mac_util.h"
 #include "base/sys_string_conversions.h"
 #include "chrome/browser/browser_process.h"
 #import "chrome/browser/cocoa/edit_search_engine_cocoa_controller.h"
@@ skipping 51 matching lines @@
   [iconImages_ setCount:count];
   [controller_ modelChanged];
 }
 
 void KeywordEditorModelObserver::OnItemsChanged(int start, int length) {
   InvalidateIconCache(start, length);
   [controller_ modelChanged];
 }
 
 void KeywordEditorModelObserver::OnItemsAdded(int start, int length) {
-  DCHECK_LE(start, static_cast<int>([iconImages_ count]));
+  const NSUInteger count([iconImages_ count]);
+  DCHECK_LE(start, static_cast<int>(count));
+  [iconImages_ setCount:count + length];

[Scott Hess - ex-Googler, 2009/10/09 02:20:43]

This will add length items to iconImages_ and the loop will add length items to
iconImages_.

One option would be capture the count before, [iconImages addPointer:NULL]
before, and then [iconImages_ setCount:count + length] after the loop to trim it
back.  Make sure you leave a comment about having to work around
-insertPointer:atIndex:.

[pink (ping after 24hrs), 2009/10/09 14:50:38]

i don't follow why you need to do this.

[Scott Hess - ex-Googler, 2009/10/09 14:54:16]

-[NSPointerArray insertPointer:atIndex:] requires the index to be strictly less
than the count.  If you try to insert _at_ the count, it throws an exception. 
Though documented, I would call that a bug in NSPointerArray.

   for (int i = 0; i < length; ++i) {
     [iconImages_ insertPointer:NULL atIndex:start];  // Values slide down.
   }
   [controller_ modelChanged];
 }
 
 void KeywordEditorModelObserver::OnItemsRemoved(int start, int length) {
   DCHECK_LE(start + length, static_cast<int>([iconImages_ count]));
   for (int i = 0; i < length; ++i) {
     [iconImages_ removePointerAtIndex:start];  // Values slide up.
@@ skipping 38 matching lines @@
     observer_.reset(new KeywordEditorModelObserver(self));
     controller_->table_model()->SetObserver(observer_.get());
     controller_->url_model()->AddObserver(observer_.get());
   }
   return self;
 }
 
 - (void)dealloc {
   controller_->table_model()->SetObserver(NULL);
   controller_->url_model()->RemoveObserver(observer_.get());
+  [tableView_ setTarget:nil];

[Scott Hess - ex-Googler, 2009/10/09 02:20:43]

If you're going to clear target, clear dataSource, too.  That said, I'm not as
worried about tableView_ retaining leftover references, though, because we're
the window controller for its window, and straight Cocoa code should have all
the ref-counting right in the first place.  Where I worry about is the interface
objects between Cocoa and Chrome (to observer_.reset(), so that if something
delayed comes in from the table view, we don't call out to released C++ objects.

+  observer_.reset();
   [super dealloc];
 }
 
 - (void)awakeFromNib {
   // Make sure the button fits its label, but keep it the same height as the
   // other two buttons.
   [GTMUILocalizerAndLayoutTweaker sizeToFitView:makeDefaultButton_];
   NSSize size = [makeDefaultButton_ frame].size;
   size.height = NSHeight([addButton_ frame]);
   [makeDefaultButton_ setFrameSize:size];
@@ skipping 19 matching lines @@
 // The last page info window that was moved will determine the location of the
 // next new one.
 - (void)windowDidMove:(NSNotification*)notif {
   if (g_browser_process && g_browser_process->local_state()) {
     NSWindow* window = [self window];
     [window saveWindowPositionToPrefs:g_browser_process->local_state()
                              withPath:prefs::kKeywordEditorWindowPlacement];
   }
 }
 
 - (void)modelChanged {

[Scott Hess - ex-Googler, 2009/10/09 14:54:16]

To help catch out-of-sync issues (which is what happened due to the exception),
it might not hurt to have this (or other appropriate central location) do
DCHECK_EQ(row_count(), [iconImages_ count]), because at this point they should
always be identical.

   [tableView_ reloadData];
+  [self adjustEditingButtons];
 }
 
 - (KeywordEditorController*)controller {
   return controller_.get();
 }
 
 - (void)sheetDidEnd:(NSWindow*)sheet
          returnCode:(NSInteger)code
             context:(void*)context {
   [sheet orderOut:self];
@@ skipping 132 matching lines @@
   if ([selection count] != 1) {
     [makeDefaultButton_ setEnabled:NO];
   } else {
     const TemplateURL& url =
         controller_->table_model()->GetTemplateURL([selection firstIndex]);
     [makeDefaultButton_ setEnabled:controller_->CanMakeDefault(&url)];
   }
 }
 
 @end