Fix a couple of bugs in the "custom home pages" list in the Preferences window.

1. It was possible to add empty rows to the list by repeatedly clicking the + button.
2. There was a crash when editing an existing item, clearing the text and pressing enter. CustomHomePagesEntry::setURL didn't handle a nil NSString passed to it in this condition.

I added a controlTextDidEndEditing method to PreferencesWindowController that forces a revalidation of the contents of the model backing the TableView.

http://crbug.com/19555
TEST=Click the + button below the custom home pages table view in the Basics page of Preferences. You should get an active edit but not add the item if you don't type anything. Also, try adding a few valid URLs, then click one to edit and delete the URL, then press enter. It should be removed from the table. See also attached unit test.
Review URL: http://codereview.chromium.org/174173

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@24635 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 53 insertions, 1 deletions

diff --git a/chrome/browser/cocoa/custom_home_pages_model.h b/chrome/browser/cocoa/custom_home_pages_model.h
index ba56e5f..259a891 100644
--- a/chrome/browser/cocoa/custom_home_pages_model.h
+++ b/chrome/browser/cocoa/custom_home_pages_model.h
@@ -35,6 +35,11 @@ class Profile;
 - (std::vector<GURL>)URLs;
 - (void)setURLs:(const std::vector<GURL>&)urls;
 
+// Validates the set of URLs stored in the model. The user may have input bad
+// data. This function removes invalid entries from the model, which will result
+// in anyone observing being updated.
+- (void)validateURLs;
+
 // For binding |customHomePages| to a mutable array controller.
 - (NSUInteger)countOfCustomHomePages;
 - (id)objectInCustomHomePagesAtIndex:(NSUInteger)index;
@@ -42,6 +47,14 @@ class Profile;
 - (void)removeObjectFromCustomHomePagesAtIndex:(NSUInteger)index;
 @end
 
+@interface CustomHomePagesModel(InternalOrTestingAPI)
+
+// Clears the URL string at the specified index. This constitutes bad data. The
+// validator should scrub the entry from the list the next time it is run.
+- (void) setURLStringEmptyAt:(NSUInteger)index;
+
+@end
+
 // A notification that fires when the URL of one of the entries changes.
 // Prevents interested parties from having to observe all model objects in order
 // to persist changes to a single entry. Changes to the number of items in the
diff --git a/chrome/browser/cocoa/custom_home_pages_model.mm b/chrome/browser/cocoa/custom_home_pages_model.mm
index 1204ac6..4232e4d 100644
--- a/chrome/browser/cocoa/custom_home_pages_model.mm
+++ b/chrome/browser/cocoa/custom_home_pages_model.mm
@@ -57,7 +57,6 @@ NSString* const kHomepageEntryChangedNotification =
 // Get/set the urls the model currently contains as a group. These will weed
 // out any URLs that are empty and not add them to the model. As a result,
 // the next time they're persisted to the prefs backend, they'll disappear.
-
 - (std::vector<GURL>)URLs {
   std::vector<GURL> urls;
   for (CustomHomePageEntry* entry in entries_.get()) {
@@ -85,6 +84,16 @@ NSString* const kHomepageEntryChangedNotification =
   [self didChangeValueForKey:@"customHomePages"];
 }
 
+- (void)validateURLs {
+  [self setURLs:[self URLs]];
+}
+
+- (void)setURLStringEmptyAt:(NSUInteger)index {
+  // This replaces the data at |index| with an empty (invalid) URL string.
+  CustomHomePageEntry* entry = [entries_ objectAtIndex:index];
+  [entry setURL:[NSString stringWithString:@""]];
+}
+
 @end
 
 //---------------------------------------------------------------------------
@@ -92,6 +101,10 @@ NSString* const kHomepageEntryChangedNotification =
 @implementation CustomHomePageEntry
 
 - (void)setURL:(NSString*)url {
+  // |url| can be nil if the user cleared the text from the edit field.
+  if (!url)
+    url = [NSString stringWithString:@""];
+
   // Make sure the url is valid before setting it by fixing it up.
   std::string urlToFix(base::SysNSStringToUTF8(url));
   urlToFix = URLFixerUpper::FixupURL(urlToFix, "");
diff --git a/chrome/browser/cocoa/preferences_window_controller.mm b/chrome/browser/cocoa/preferences_window_controller.mm
index 372a4c6..400c6cd 100644
--- a/chrome/browser/cocoa/preferences_window_controller.mm
+++ b/chrome/browser/cocoa/preferences_window_controller.mm
@@ -938,4 +938,8 @@ const int kDisabledIndex = 1;
                     object:self];
 }
 
+- (void)controlTextDidEndEditing:(NSNotification*)notification {
+  [customPagesSource_ validateURLs];
+}
+
 @end
diff --git a/chrome/browser/cocoa/preferences_window_controller_unittest.mm b/chrome/browser/cocoa/preferences_window_controller_unittest.mm
index 1241100..8040e56 100644
--- a/chrome/browser/cocoa/preferences_window_controller_unittest.mm
+++ b/chrome/browser/cocoa/preferences_window_controller_unittest.mm
@@ -8,6 +8,7 @@
 #import "chrome/browser/cocoa/preferences_window_controller.h"
 #include "chrome/browser/cocoa/browser_test_helper.h"
 #include "chrome/browser/cocoa/cocoa_test_helper.h"
+#import "chrome/browser/cocoa/custom_home_pages_model.h"
 #include "chrome/common/pref_names.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
@@ -69,4 +70,25 @@ TEST_F(PrefsControllerTest, ShowAndClose) {
 #endif
 }
 
+TEST_F(PrefsControllerTest, ValidateCustomHomePagesTable) {
+  // First, insert two valid URLs into the CustomHomePagesModel.
+  GURL url1("http://www.google.com/");
+  GURL url2("http://maps.google.com/");
+  std::vector<GURL> urls;
+  urls.push_back(url1);
+  urls.push_back(url2);
+  [[pref_controller_ customPagesSource] setURLs:urls];
+  EXPECT_EQ(2U, [[pref_controller_ customPagesSource] countOfCustomHomePages]);
+
+  // Now insert a bad (empty) URL into the model.
+  [[pref_controller_ customPagesSource] setURLStringEmptyAt:1];
+
+  // Send a notification to simulate the end of editing on a cell in the table
+  // which should trigger validation.
+  [pref_controller_ controlTextDidEndEditing:[NSNotification
+      notificationWithName:NSControlTextDidEndEditingNotification
+                    object:nil]];
+  EXPECT_EQ(1U, [[pref_controller_ customPagesSource] countOfCustomHomePages]);
+}
+
 }  // namespace