Fix the issue in 'Clear browsing data' dialog with Cancel vs Close button.

We should *only* save the state of the checkboxes when the user click on the button to clear the data,
otherwise we shouldn't change the preferences of the checkboxes.

Along the way, rename the button from 'Close' to 'Cancel'.

BUG=35098
TEST=open the dialog, see the current state of the checkboxes, click 'Cancel'.
reopen the dialog, the state should be the same. Now uncheck some checkbox,
click 'Cancel', reopen the dialog the unchecked checkbox should be checked.
Open the dialog again, uncheck a checkbox and click 'Clear browsing data',
reopen the dialog, that checkbox should be *unchecked*.

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

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

2 files changed, 38 insertions, 53 deletions

diff --git a/chrome/browser/gtk/clear_browsing_data_dialog_gtk.cc b/chrome/browser/gtk/clear_browsing_data_dialog_gtk.cc
index 9549319..2607359 100644
--- a/chrome/browser/gtk/clear_browsing_data_dialog_gtk.cc
+++ b/chrome/browser/gtk/clear_browsing_data_dialog_gtk.cc
@@ -45,9 +45,9 @@ ClearBrowsingDataDialogGtk::ClearBrowsingDataDialogGtk(GtkWindow* parent,
       (GtkDialogFlags) (GTK_DIALOG_MODAL | GTK_DIALOG_NO_SEPARATOR),
       NULL);
 
-  GtkWidget* close_button = gtk_dialog_add_button(GTK_DIALOG(dialog_),
-      GTK_STOCK_CLOSE, GTK_RESPONSE_REJECT);
-  gtk_widget_grab_focus(close_button);
+  GtkWidget* cancel_button = gtk_dialog_add_button(GTK_DIALOG(dialog_),
+      GTK_STOCK_CANCEL, GTK_RESPONSE_REJECT);
+  gtk_widget_grab_focus(cancel_button);
 
   accessible_widget_helper_.reset(new AccessibleWidgetHelper(dialog_, profile));
   accessible_widget_helper_->SendOpenWindowNotification(dialog_name);
@@ -188,6 +188,22 @@ ClearBrowsingDataDialogGtk::~ClearBrowsingDataDialogGtk() {
 void ClearBrowsingDataDialogGtk::OnDialogResponse(GtkWidget* widget,
                                                   int response) {
   if (response == GTK_RESPONSE_ACCEPT) {
+    PrefService* prefs = profile_->GetPrefs();
+    prefs->SetBoolean(prefs::kDeleteBrowsingHistory,
+                      IsChecked(del_history_checkbox_));
+    prefs->SetBoolean(prefs::kDeleteDownloadHistory,
+                      IsChecked(del_downloads_checkbox_));
+    prefs->SetBoolean(prefs::kDeleteCache,
+                      IsChecked(del_cache_checkbox_));
+    prefs->SetBoolean(prefs::kDeleteCookies,
+                      IsChecked(del_cookies_checkbox_));
+    prefs->SetBoolean(prefs::kDeletePasswords,
+                      IsChecked(del_passwords_checkbox_));
+    prefs->SetBoolean(prefs::kDeleteFormData,
+                      IsChecked(del_form_data_checkbox_));
+    prefs->SetInteger(prefs::kDeleteTimePeriod,
+        gtk_combo_box_get_active(GTK_COMBO_BOX(time_period_combobox_)));
+
     int period_selected = gtk_combo_box_get_active(
         GTK_COMBO_BOX(time_period_combobox_));
 
@@ -203,28 +219,6 @@ void ClearBrowsingDataDialogGtk::OnDialogResponse(GtkWidget* widget,
 }
 
 void ClearBrowsingDataDialogGtk::OnDialogWidgetClicked(GtkWidget* widget) {
-  if (widget == del_history_checkbox_) {
-    profile_->GetPrefs()->SetBoolean(prefs::kDeleteBrowsingHistory,
-                                     IsChecked(widget));
-  } else if (widget == del_downloads_checkbox_) {
-    profile_->GetPrefs()->SetBoolean(prefs::kDeleteDownloadHistory,
-                                     IsChecked(widget));
-  } else if (widget == del_cache_checkbox_) {
-    profile_->GetPrefs()->SetBoolean(prefs::kDeleteCache,
-                                     IsChecked(widget));
-  } else if (widget == del_cookies_checkbox_) {
-    profile_->GetPrefs()->SetBoolean(prefs::kDeleteCookies,
-                                     IsChecked(widget));
-  } else if (widget == del_passwords_checkbox_) {
-    profile_->GetPrefs()->SetBoolean(prefs::kDeletePasswords,
-                                     IsChecked(widget));
-  } else if (widget == del_form_data_checkbox_) {
-    profile_->GetPrefs()->SetBoolean(prefs::kDeleteFormData,
-                                     IsChecked(widget));
-  } else if (widget == time_period_combobox_) {
-    profile_->GetPrefs()->SetInteger(prefs::kDeleteTimePeriod,
-        gtk_combo_box_get_active(GTK_COMBO_BOX(widget)));
-  }
   UpdateDialogButtons();
 }
 
diff --git a/chrome/browser/views/clear_browsing_data.cc b/chrome/browser/views/clear_browsing_data.cc
index c283997..0d59298 100644
--- a/chrome/browser/views/clear_browsing_data.cc
+++ b/chrome/browser/views/clear_browsing_data.cc
@@ -241,13 +241,10 @@ int ClearBrowsingDataView::GetDefaultDialogButton() const {
 
 std::wstring ClearBrowsingDataView::GetDialogButtonLabel(
     MessageBoxFlags::DialogButton button) const {
-  if (button == MessageBoxFlags::DIALOGBUTTON_OK) {
-    return l10n_util::GetString(IDS_CLEAR_BROWSING_DATA_COMMIT);
-  } else if (button == MessageBoxFlags::DIALOGBUTTON_CANCEL) {
-    return l10n_util::GetString(IDS_CLOSE);
-  } else {
-    return std::wstring();
-  }
+  DCHECK((button == MessageBoxFlags::DIALOGBUTTON_OK) ||
+         (button == MessageBoxFlags::DIALOGBUTTON_CANCEL));
+  return l10n_util::GetString((button == MessageBoxFlags::DIALOGBUTTON_OK) ?
+      IDS_CLEAR_BROWSING_DATA_COMMIT : IDS_CANCEL);
 }
 
 bool ClearBrowsingDataView::IsDialogButtonEnabled(
@@ -296,6 +293,19 @@ bool ClearBrowsingDataView::Accept() {
     return false;
   }
 
+  PrefService* prefs = profile_->GetPrefs();
+  prefs->SetBoolean(prefs::kDeleteBrowsingHistory,
+                    del_history_checkbox_->checked());
+  prefs->SetBoolean(prefs::kDeleteDownloadHistory,
+                    del_downloads_checkbox_->checked());
+  prefs->SetBoolean(prefs::kDeleteCache,
+                    del_cache_checkbox_->checked());
+  prefs->SetBoolean(prefs::kDeleteCookies,
+                    del_cookies_checkbox_->checked());
+  prefs->SetBoolean(prefs::kDeletePasswords,
+                    del_passwords_checkbox_->checked());
+  prefs->SetBoolean(prefs::kDeleteFormData,
+                    del_form_data_checkbox_->checked());
   OnDelete();
   return false;  // We close the dialog in OnBrowsingDataRemoverDone().
 }
@@ -363,27 +373,8 @@ void ClearBrowsingDataView::ItemChanged(views::Combobox* sender,
 ////////////////////////////////////////////////////////////////////////////////
 // ClearBrowsingDataView, views::ButtonListener implementation:
 
-void ClearBrowsingDataView::ButtonPressed(
-    views::Button* sender, const views::Event& event) {
-  if (sender == del_history_checkbox_)
-    profile_->GetPrefs()->SetBoolean(prefs::kDeleteBrowsingHistory,
-        del_history_checkbox_->checked() ? true : false);
-  else if (sender == del_downloads_checkbox_)
-    profile_->GetPrefs()->SetBoolean(prefs::kDeleteDownloadHistory,
-        del_downloads_checkbox_->checked() ? true : false);
-  else if (sender == del_cache_checkbox_)
-    profile_->GetPrefs()->SetBoolean(prefs::kDeleteCache,
-        del_cache_checkbox_->checked() ? true : false);
-  else if (sender == del_cookies_checkbox_)
-    profile_->GetPrefs()->SetBoolean(prefs::kDeleteCookies,
-        del_cookies_checkbox_->checked() ? true : false);
-  else if (sender == del_passwords_checkbox_)
-    profile_->GetPrefs()->SetBoolean(prefs::kDeletePasswords,
-        del_passwords_checkbox_->checked() ? true : false);
-  else if (sender == del_form_data_checkbox_)
-    profile_->GetPrefs()->SetBoolean(prefs::kDeleteFormData,
-        del_form_data_checkbox_->checked() ? true : false);
-
+void ClearBrowsingDataView::ButtonPressed(views::Button* sender,
+                                          const views::Event& event) {
   // When no checkbox is checked we should not have the action button enabled.
   // This forces the button to evaluate what state they should be in.
   GetDialogClientView()->UpdateDialogButtons();