Invalid URLs are no longer mangled when reopening the Options window.

If the homepage URL is changed to an invalid URL, the homepage preference is swapped to a blank URL and NTP is shown.

BUG=40996
TEST=Set homepage to http://www.google.com, close Chromium, restart Chromium and see homepage set to http://www.google.com. Set homepage to http://, close Chromium, restart Chromium and see homepage set to New Tab Page.

Patch by Jared Wein <weinjared@gmail.com>

Review URL: http://codereview.chromium.org/2102019

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

6 files changed, 32 insertions, 30 deletions

diff --git a/AUTHORS b/AUTHORS
index 59a22ed..76c0be9 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -76,3 +76,4 @@ Ryan Sleevi <ryan.sleevi@gmail.com>
 Satoshi Matsuzaki <satoshi.matsuzaki@gmail.com>
 Benjamin Jemlich <pcgod99@gmail.com>
 Ningxin Hu <ningxin.hu@intel.com>
+Jared Wein <weinjared@gmail.com>
diff --git a/chrome/browser/cocoa/preferences_window_controller.mm b/chrome/browser/cocoa/preferences_window_controller.mm
index 4410db6..586860c 100644
--- a/chrome/browser/cocoa/preferences_window_controller.mm
+++ b/chrome/browser/cocoa/preferences_window_controller.mm
@@ -796,18 +796,19 @@ class PrefObserverBridge : public NotificationObserver,
 // Basics panel
 
 // Sets the home page preferences for kNewTabPageIsHomePage and kHomePage. If a
-// blank string is passed in we revert to using NewTab page as the Home page.
-// When setting the Home Page to NewTab page, we preserve the old value of
-// kHomePage (we don't overwrite it). Note: using SetValue() causes the
-// observers not to fire, which is actually a good thing as we could end up in a
-// state where setting the homepage to an empty url would automatically reset
-// the prefs back to using the NTP, so we'd be never be able to change it.
-- (void)setHomepage:(const std::string&)homepage {
-  if (homepage.empty() || homepage == GetNewTabUIURLString()) {
+// blank or null-host URL is passed in we revert to using NewTab page
+// as the Home page. Note: using SetValue() causes the observers not to fire,
+// which is actually a good thing as we could end up in a state where setting
+// the homepage to an empty url would automatically reset the prefs back to
+// using the NTP, so we'd be never be able to change it.
+- (void)setHomepage:(const GURL&)homepage {
+  if (!homepage.is_valid() || homepage.spec() == GetNewTabUIURLString()) {
     newTabPageIsHomePage_.SetValue(true);
+    if (!homepage.has_host())
+      homepage_.SetValue(std::wstring());
   } else {
     newTabPageIsHomePage_.SetValue(false);
-    homepage_.SetValue(UTF8ToWide(homepage));
+    homepage_.SetValue(UTF8ToWide(homepage.spec()));
   }
 }
 
@@ -1013,9 +1014,7 @@ enum { kHomepageNewTabPage, kHomepageURL };
   // to something valid ("http://google.com").
   std::string unfixedURL = urlString ? base::SysNSStringToUTF8(urlString) :
                                        chrome::kChromeUINewTabURL;
-  std::string fixedURL = URLFixerUpper::FixupURL(unfixedURL, std::string());
-  if (GURL(fixedURL).is_valid())
-    [self setHomepage:fixedURL];
+  [self setHomepage:GURL(URLFixerUpper::FixupURL(unfixedURL, std::string()))];
 }
 
 // Returns whether the home button should be checked based on the preference.
diff --git a/chrome/browser/gtk/options/general_page_gtk.cc b/chrome/browser/gtk/options/general_page_gtk.cc
index 836f2a7..8a25f01 100644
--- a/chrome/browser/gtk/options/general_page_gtk.cc
+++ b/chrome/browser/gtk/options/general_page_gtk.cc
@@ -628,6 +628,8 @@ void GeneralPageGtk::EnableDefaultSearchEngineComboBox(bool enable) {
 void GeneralPageGtk::SetHomepage(const GURL& homepage) {
   if (!homepage.is_valid() || homepage.spec() == chrome::kChromeUINewTabURL) {
     new_tab_page_is_home_page_.SetValue(true);
+    if (!homepage.has_host())
+      homepage_.SetValue(std::wstring());
   } else {
     new_tab_page_is_home_page_.SetValue(false);
     homepage_.SetValue(UTF8ToWide(homepage.spec()));
diff --git a/chrome/browser/gtk/options/general_page_gtk.h b/chrome/browser/gtk/options/general_page_gtk.h
index e7139163..10ec4c8 100644
--- a/chrome/browser/gtk/options/general_page_gtk.h
+++ b/chrome/browser/gtk/options/general_page_gtk.h
@@ -67,9 +67,8 @@ class GeneralPageGtk : public OptionsPageBase,
   void EnableDefaultSearchEngineComboBox(bool enable);
 
   // Sets the home page preferences for kNewTabPageIsHomePage and kHomePage.
-  // If a blank string is passed in we revert to using NewTab page as the Home
-  // page. When setting the Home Page to NewTab page, we preserve the old value
-  // of kHomePage (we don't overwrite it).
+  // If an invalid URL is passed in we revert to using NewTab page as the Home
+  // page.
   void SetHomepage(const GURL& homepage);
 
   // Sets the home page pref using the value in the entry box
diff --git a/chrome/browser/views/options/general_page_view.cc b/chrome/browser/views/options/general_page_view.cc
index cdc85be..9cbcc5f 100644
--- a/chrome/browser/views/options/general_page_view.cc
+++ b/chrome/browser/views/options/general_page_view.cc
@@ -232,12 +232,12 @@ void GeneralPageView::ButtonPressed(
   } else if (sender == homepage_use_newtab_radio_) {
     UserMetricsRecordAction(UserMetricsAction("Options_Homepage_UseNewTab"),
                             profile()->GetPrefs());
-    SetHomepage(GetNewTabUIURLString());
+    SetHomepage(GURL());
     EnableHomepageURLField(false);
   } else if (sender == homepage_use_url_radio_) {
     UserMetricsRecordAction(UserMetricsAction("Options_Homepage_UseURL"),
                             profile()->GetPrefs());
-    SetHomepage(homepage_use_url_textfield_->text());
+    SetHomepage(GURL(homepage_use_url_textfield_->text()));
     EnableHomepageURLField(true);
   } else if (sender == homepage_show_home_button_checkbox_) {
     bool show_button = homepage_show_home_button_checkbox_->checked();
@@ -285,11 +285,10 @@ void GeneralPageView::ContentsChanged(views::Textfield* sender,
   if (sender == homepage_use_url_textfield_) {
     // If the text field contains a valid URL, sync it to prefs. We run it
     // through the fixer upper to allow input like "google.com" to be converted
-    // to something valid ("http://google.com").
-    std::string url_string = URLFixerUpper::FixupURL(
-        UTF16ToUTF8(homepage_use_url_textfield_->text()), std::string());
-    if (GURL(url_string).is_valid())
-      SetHomepage(UTF8ToWide(url_string));
+    // to something valid ("http://google.com"). If the field contains an
+    // empty or null-host URL, a blank homepage is synced to prefs.
+    SetHomepage(GURL(URLFixerUpper::FixupURL(
+        UTF16ToUTF8(homepage_use_url_textfield_->text()), std::string())));
   }
 }
 
@@ -745,12 +744,15 @@ void GeneralPageView::AddBookmark(UrlPicker* dialog,
   SaveStartupPref();
 }
 
-void GeneralPageView::SetHomepage(const std::wstring& homepage) {
-  if (homepage.empty() || homepage == GetNewTabUIURLString()) {
+void GeneralPageView::SetHomepage(const GURL& homepage) {
+  if (!homepage.is_valid() ||
+      UTF8ToWide(homepage.spec()) == GetNewTabUIURLString()) {
     new_tab_page_is_home_page_.SetValue(true);
+    if (!homepage.has_host())
+      homepage_.SetValue(std::wstring());
   } else {
     new_tab_page_is_home_page_.SetValue(false);
-    homepage_.SetValue(homepage);
+    homepage_.SetValue(UTF8ToWide(homepage.spec()));
   }
 }
 
diff --git a/chrome/browser/views/options/general_page_view.h b/chrome/browser/views/options/general_page_view.h
index 32372b4..4ef4020 100644
--- a/chrome/browser/views/options/general_page_view.h
+++ b/chrome/browser/views/options/general_page_view.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 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.
 
@@ -105,10 +105,9 @@ class GeneralPageView : public OptionsPageView,
                            const GURL& url);
 
   // Sets the home page preferences for kNewTabPageIsHomePage and kHomePage.
-  // If a blank string is passed in we revert to using NewTab page as the Home
-  // page. When setting the Home Page to NewTab page, we preserve the old value
-  // of kHomePage (we don't overwrite it).
-  void SetHomepage(const std::wstring& homepage);
+  // If an empty or null-host GURL is passed in we revert to using NewTab
+  // page as the Homepage.
+  void SetHomepage(const GURL& homepage);
 
   // Invoked when the selection of the table view changes. Updates the enabled
   // property of the remove button.