[Mac] Fix keyword editor-related crashes

chrome/browser/cocoa/keyword_editor_cocoa_controller.mm

Index: chrome/browser/cocoa/keyword_editor_cocoa_controller.mm
diff --git a/chrome/browser/cocoa/keyword_editor_cocoa_controller.mm b/chrome/browser/cocoa/keyword_editor_cocoa_controller.mm
index 4198cf0a25299a59e733ea8749671999db875ecf..184e5e3dfcbdcd668a1296f2662f76fbac2fd77d 100644
--- a/chrome/browser/cocoa/keyword_editor_cocoa_controller.mm
+++ b/chrome/browser/cocoa/keyword_editor_cocoa_controller.mm
@@ -69,7 +69,9 @@ void KeywordEditorModelObserver::OnItemsChanged(int start, int length) {
 }
 
 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.
   }
@@ -128,6 +130,8 @@ void KeywordEditorModelObserver::InvalidateIconCache(int start, int length) {
 - (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];
 }
 
@@ -169,6 +173,7 @@ void KeywordEditorModelObserver::InvalidateIconCache(int start, int length) {
 
 - (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 {