Protector strings and bubble/SettingsChange code refactoring.

chrome/browser/protector/settings_change_global_error.cc

Index: chrome/browser/protector/settings_change_global_error.cc
diff --git a/chrome/browser/protector/settings_change_global_error.cc b/chrome/browser/protector/settings_change_global_error.cc
index 9909cb31e029a522ed8078c09b4fd0fc8ef30ae3..4a7583c2aa574a8be6f90689d6709a04f24e4c92 100644
--- a/chrome/browser/protector/settings_change_global_error.cc
+++ b/chrome/browser/protector/settings_change_global_error.cc
@@ -15,9 +15,6 @@
 #include "chrome/browser/ui/global_error_service.h"
 #include "chrome/browser/ui/global_error_service_factory.h"
 #include "content/public/browser/browser_thread.h"
-#include "grit/chromium_strings.h"
-#include "grit/generated_resources.h"
-#include "ui/base/l10n/l10n_util.h"
 
 using content::BrowserThread;
 
@@ -28,58 +25,21 @@ namespace {
 // Timeout before the global error is removed (wrench menu item disappears).
 const int kMenuItemDisplayPeriodMs = 10*60*1000;  // 10 min
 
-// IDs of menu item labels.
-const int kMenuItemLabelIDs[] = {
-  IDS_SEARCH_ENGINE_CHANGE_WRENCH_MENU_ITEM,
-  IDS_HOMEPAGE_CHANGE_WRENCH_MENU_ITEM
-};
-
-// IDs of bubble title messages.
-const int kBubbleTitleIDs[] = {
-  IDS_SEARCH_ENGINE_CHANGE_BUBBLE_TITLE,
-  IDS_HOMEPAGE_CHANGE_BUBBLE_TITLE
-};
-
-// IDs of bubble text messages.
-const int kBubbleMessageIDs[] = {
-  IDS_SEARCH_ENGINE_CHANGE_BUBBLE_TEXT,
-  IDS_HOMEPAGE_CHANGE_BUBBLE_TEXT
-};
-
-// IDs of bubble text messages when the old setting is unknown.
-const int kBubbleMessageOldUnknownIDs[] = {
-  IDS_SEARCH_ENGINE_CHANGE_UNKNOWN_BUBBLE_TEXT,
-  IDS_HOMEPAGE_CHANGE_UNKNOWN_BUBBLE_TEXT
-};
-
-// IDs of "Keep Setting" button titles.
-const int kBubbleKeepSettingIDs[] = {
-  IDS_SEARCH_ENGINE_CHANGE_RESTORE,
-  IDS_HOMEPAGE_CHANGE_RESTORE
-};
-
-// IDs of "Change Setting" button titles.
-const int kBubbleChangeSettingIDs[] = {
-  IDS_SEARCH_ENGINE_CHANGE_APPLY,
-  IDS_HOMEPAGE_CHANGE_APPLY
-};
-
 }  // namespace
 
 SettingsChangeGlobalError::SettingsChangeGlobalError(
-    const SettingChangeVector& changes,
+    BaseSettingChange* change,
     SettingsChangeGlobalErrorDelegate* delegate)
-    : changes_(changes),
+    : change_(change),
       delegate_(delegate),
       profile_(NULL),
       browser_(NULL),
       closed_by_button_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
-  DCHECK(changes.size() > 0);
+  DCHECK(delegate_);
 }
 
 SettingsChangeGlobalError::~SettingsChangeGlobalError() {
-  STLDeleteElements(&changes_);
 }
 
 bool SettingsChangeGlobalError::HasBadge() {
@@ -94,12 +54,8 @@ int SettingsChangeGlobalError::MenuItemCommandID() {
   return IDC_SHOW_SETTINGS_CHANGES;
 }
 
-// TODO(ivankr): Currently the menu item/bubble only displays a warning about
-// the first change. We want to fix this so that a single menu item/bubble
-// can display warning about multiple changes.
-
 string16 SettingsChangeGlobalError::MenuItemLabel() {
-  return l10n_util::GetStringUTF16(kMenuItemLabelIDs[changes_.front()->type()]);
+  return change_->GetBubbleTitle();
 }
 
 void SettingsChangeGlobalError::ExecuteMenuItem(Browser* browser) {
@@ -114,58 +70,37 @@ bool SettingsChangeGlobalError::HasBubbleView() {
 }
 
 string16 SettingsChangeGlobalError::GetBubbleViewTitle() {
-  return l10n_util::GetStringUTF16(kBubbleTitleIDs[changes_.front()->type()]);
+  return change_->GetBubbleTitle();
 }
 
 string16 SettingsChangeGlobalError::GetBubbleViewMessage() {
-  SettingChange* change = changes_.front();
-  const string16& old_setting = change->GetOldSetting();
-  if (old_setting.empty()) {
-    return l10n_util::GetStringFUTF16(
-        kBubbleMessageOldUnknownIDs[change->type()],
-        change->GetNewSetting());
-  } else {
-    return l10n_util::GetStringFUTF16(
-        kBubbleMessageIDs[change->type()],
-        old_setting,
-        change->GetNewSetting());
-  }
+  return change_->GetBubbleMessage();
 }
 
+// The Accept and Revert buttons are swapped like the 'server' and 'client'
+// concepts in X11. Accept button (the default one) discards changes
+// (keeps using previous setting) while cancel button applies changes
+// (switches to the new setting). This is sick and blows my mind. - ivankr
+
 string16 SettingsChangeGlobalError::GetBubbleViewAcceptButtonLabel() {
-  SettingChange* change = changes_.front();
-  string16 old_setting = change->GetOldSetting();
-  if (old_setting.empty()) {
-    return l10n_util::GetStringUTF16(IDS_SETTINGS_CHANGE_OPEN_SETTINGS);
-  } else {
-    return l10n_util::GetStringFUTF16(
-        kBubbleKeepSettingIDs[change->type()],
-        old_setting);
-  }
+  return change_->GetDiscardButtonText();
 }
 
 string16 SettingsChangeGlobalError::GetBubbleViewCancelButtonLabel() {
-  SettingChange* change = changes_.front();
-  return l10n_util::GetStringFUTF16(kBubbleChangeSettingIDs[change->type()],
-                                    change->GetNewSetting());
+  return change_->GetApplyButtonText();
 }
 
 void SettingsChangeGlobalError::BubbleViewAcceptButtonPressed() {
   closed_by_button_ = true;
-  DCHECK(delegate_);
-  VLOG(1) << "Discard changes";
-  delegate_->OnDiscardChanges();
+  delegate_->OnDiscardChange();
 }
 
 void SettingsChangeGlobalError::BubbleViewCancelButtonPressed() {
   closed_by_button_ = true;
-  DCHECK(delegate_);
-  VLOG(1) << "Apply changes";
-  delegate_->OnApplyChanges();
+  delegate_->OnApplyChange();
 }
 
 void SettingsChangeGlobalError::RemoveFromProfile() {
-  DCHECK(delegate_);
   if (profile_)
     GlobalErrorServiceFactory::GetForProfile(profile_)->RemoveGlobalError(this);
   if (!closed_by_button_)