Revert 110684 - Protector strings and bubble/SettingsChange code refactoring.

BUG=102765,103317
TEST=Manual

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

TBR=ivankr@chromium.org
Review URL: http://codereview.chromium.org/8586050

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

chrome/app/generated_resources.grd

@@ -1197,53 +1197,53 @@ are declared in build/common.gypi.
         </message>
       </if>
 
-      <!-- Protector error messages -->
-      <message name="IDS_SEARCH_ENGINE_CHANGE_TITLE" desc="The title of the bubble and the wrench menu item with a search engine change notification.">
-        Attempted search engine change
+      <!-- Settings change error messages -->
+      <message name="IDS_SEARCH_ENGINE_CHANGE_WRENCH_MENU_ITEM" desc="The wrench menu item indicating a search engine change.">
+        Verify search engine change
       </message>
-      <message name="IDS_SEARCH_ENGINE_CHANGE_MESSAGE" desc="The text of the search engine change notification.">
-        Arrr! Something tried to commandeer <ph name="PRODUCT_NAME">$1<ex>Google Chrome</ex></ph>'s default search engine!
+      <message name="IDS_SEARCH_ENGINE_CHANGE_BUBBLE_TITLE" desc="The title of the bubble view with a search engine change notification.">
+        Verify Search Engine Change
       </message>
-      <message name="IDS_SEARCH_ENGINE_CHANGE_NO_BACKUP_MESSAGE" desc="The text of the search engine change notification when the previous setting is lost.">
-        Arrr! Something tried to commandeer <ph name="PRODUCT_NAME">$1<ex>Google Chrome</ex></ph>'s default search engine! We weren't sure what to do, so we set it to <ph name="PREPOPULATED_ENGINE">$2<ex>DefaultSearchEngine</ex></ph>.
+      <message name="IDS_SEARCH_ENGINE_CHANGE_BUBBLE_TEXT" desc="The text of the bubble view with a search engine change notification.">
+        A program on your computer tried to change your search engine from <ph name="PREV_ENGINE">$1<ex>OldSearchEngine</ex></ph> to <ph name="NEW_ENGINE">$2<ex>NewSearchEngine</ex></ph>. Would you like to keep using <ph name="PREV_ENGINE">$1<ex>OldSearchEngine</ex></ph>?
       </message>
-      <message name="IDS_KEEP_SEARCH_ENGINE" desc="The title of the button that confirms staying with the previous search engine setting.">
-        Keep using <ph name="PREV_ENGINE">$1<ex>PreviousSearchEngine</ex></ph>
+      <message name="IDS_SEARCH_ENGINE_CHANGE_UNKNOWN_BUBBLE_TEXT" desc="The text of the bubble view with a search engine change notification when the previous search engine is unknown.">
+        A program on your computer tried to change your search engine to <ph name="NEW_ENGINE">$1<ex>NewSearchEngine</ex></ph> and your previous setting was deleted. Please check your settings to make sure everything is configured the way you like.
       </message>
-      <message name="IDS_CHANGE_SEARCH_ENGINE" desc="The title of the button that accepts new default search engine setting.">
-        Change to <ph name="NEW_ENGINE">$1<ex>NewSearchEngine</ex></ph>
-      </message>
-      <message name="IDS_CHANGE_SEARCH_ENGINE_NO_NAME" desc="The title of the button that accepts new default search engine setting when the search engine name is too long.">
-        Change search engine
+      <message name="IDS_SEARCH_ENGINE_CHANGE_RESTORE" desc="The title of the button that restores previous default search engine setting.">
+        Keep using <ph name="PREV_ENGINE">$1<ex>OldSearchEngine</ex></ph>
       </message>
-      <message name="IDS_SELECT_SEARCH_ENGINE" desc="The title of the button that opens a settings page, allowing user to select the default search engine to use.">
-        Select search engine...
+      <message name="IDS_SEARCH_ENGINE_CHANGE_APPLY" desc="The title of the button that accepts new default search engine setting.">
+        Change to <ph name="NEW_ENGINE">$1<ex>NewSearchEngine</ex></ph>
       </message>
 
-      <message name="IDS_HOMEPAGE_CHANGE_TITLE" desc="The title of the bubble and the wrench menu item with a home page change notification.">
-        Attempted home page change
+      <message name="IDS_HOMEPAGE_CHANGE_WRENCH_MENU_ITEM" desc="The wrench menu item indicating a home page change.">
+        Verify home page change
       </message>
-      <message name="IDS_HOMEPAGE_CHANGE_BUBBLE_MESSAGE" desc="The text of the home page change notification.">
-        Arrr! Something tried to commandeer <ph name="PRODUCT_NAME">$1<ex>Google Chrome</ex></ph> home page!
+      <message name="IDS_HOMEPAGE_CHANGE_BUBBLE_TITLE" desc="The title of the bubble view with a home page change notification.">
+        Verify Home Page Change
       </message>
-      <message name="IDS_HOMEPAGE_CHANGE_BUBBLE_NO_BACKUP_MESSAGE" desc="The text of the home page change notification when the previous setting is lost.">
-        Arrr! Something tried to commandeer <ph name="PRODUCT_NAME">$1<ex>Google Chrome</ex></ph> home page! We weren't sure what to do, so we set it to the New Tab page.
+      <message name="IDS_HOMEPAGE_CHANGE_BUBBLE_TEXT" desc="The text of the bubble view with a home page change notification.">
+        A program on your computer tried to change your home page from <ph name="PREV_HOMEPAGE">$1<ex>http://example.net/</ex></ph> to <ph name="NEW_HOMEPAGE">$2<ex>http://example.com/</ex></ph>. Would you like to keep using <ph name="PREV_HOMEPAGE">$1<ex>http://example.net/</ex></ph>?
       </message>
-      <message name="IDS_KEEP_HOMEPAGE" desc="The title of the button that confirms staying with the previous home page setting.">
-        Keep using <ph name="PREV_HOMEPAGE">$1<ex>My Awesome Blog</ex></ph>
+      <message name="IDS_HOMEPAGE_CHANGE_UNKNOWN_BUBBLE_TEXT" desc="The text of the bubble view with a home page change notification when the previous home page is unknown.">
+        A program on your computer tried to change your home page to <ph name="NEW_HOMEPAGE">$1<ex>http://example.com/</ex></ph> and your previous setting was deleted. Please check your settings to make sure everything is configured the way you like.
       </message>
-      <message name="IDS_CHANGE_HOMEPAGE" desc="The title of the button that accepts new home page setting.">
-        Change to <ph name="NEW_HOMEPAGE">$1<ex>My Awesome Blog</ex></ph>
+      <message name="IDS_HOMEPAGE_CHANGE_RESTORE" desc="The title of the button that restores previous home page setting.">
+        Keep using <ph name="PREV_HOMEPAGE">$1<ex>http://example.net/</ex></ph>
       </message>
-      <message name="IDS_CHANGE_HOMEPAGE_NO_NAME" desc="The title of the button that accepts new home page setting when the home page title is too long.">
-        Change home page
-      </message>
-      <message name="IDS_SELECT_HOMEPAGE" desc="The title of the button that opens a settings page, allowing user to select the home page to use.">
-        Select home page...
+      <message name="IDS_HOMEPAGE_CHANGE_APPLY" desc="The title of the button that accepts new home page setting.">
+        Change to <ph name="NEW_HOMEPAGE">$1<ex>http://example.com/</ex></ph>
       </message>
 
-      <message name="IDS_KEEP_SETTING" desc="The title of the button that confirms staying with the previous setting.">
-        Keep existing setting
+      <message name="IDS_SETTINGS_CHANGE_OPEN_SETTINGS" desc="The title of the button that takes user to the settings page to restore settings.">
+        Go to settings
+      </message>
+      <message name="IDS_SETTINGS_CHANGE_RESTORE" desc="The title of the button that restores previous settings.">
+        Keep previous settings
+      </message>
+      <message name="IDS_SETTINGS_CHANGE_APPLY" desc="The title of the button that accepts new settings.">
+        Accept changes
       </message>
 
       <!-- Keywords -->

chrome/browser/protector/default_search_provider_change.cc

@@ -11,57 +11,33 @@
 #include "chrome/browser/search_engines/template_url_service.h"
 #include "chrome/browser/webdata/keyword_table.h"
 #include "chrome/common/url_constants.h"
-#include "grit/chromium_strings.h"
-#include "grit/generated_resources.h"
 #include "googleurl/src/gurl.h"
-#include "ui/base/l10n/l10n_util.h"
 
 namespace protector {
 
-namespace {
-
-// Maximum length of the search engine name to be displayed.
-const size_t kMaxDisplayedNameLength = 10;
-
-}  // namespace
-
 class DefaultSearchProviderChange : public SettingChange {
  public:
   DefaultSearchProviderChange(const TemplateURL* old_url,
                               const TemplateURL* new_url);
 
   // SettingChange overrides:
-  virtual bool Init(Protector* protector) OVERRIDE;
-  virtual void Apply(Protector* protector) OVERRIDE;
-  virtual void Discard(Protector* protector) OVERRIDE;
-  virtual string16 GetTitle() const OVERRIDE;
-  virtual string16 GetMessage() const OVERRIDE;
-  virtual string16 GetApplyButtonText() const OVERRIDE;
-  virtual string16 GetDiscardButtonText() const OVERRIDE;
+  virtual string16 GetOldSetting() const OVERRIDE;
+  virtual string16 GetNewSetting() const OVERRIDE;
+  virtual void Accept(Protector* protector) OVERRIDE;
+  virtual void Revert(Protector* protector) OVERRIDE;
+  virtual void DoDefault(Protector* protector) OVERRIDE;
 
  private:
   virtual ~DefaultSearchProviderChange();
 
   // Sets the given default search provider to profile that |protector| is
-  // guarding. Returns the |TemplateURL| instance the default search provider
-  // has been set to. If no search provider with |id| exists and
-  // |allow_fallback| is true, sets one of the prepoluated search providers.
-  const TemplateURL* SetDefaultSearchProvider(Protector* protector,
-                                              int64 id,
-                                              bool allow_fallback);
-
-  // Opens the Search engine settings page in a new tab.
-  void OpenSearchEngineSettings(Protector* protector);
+  // guarding.
+  void SetDefaultSearchProvider(Protector* protector, int64 id);
 
   int64 old_id_;
   int64 new_id_;
-  // ID of the search engine that we fall back to if the backup is lost.
-  int64 fallback_id_;
   string16 old_name_;
   string16 new_name_;
-  // Name of the search engine that we fall back to if the backup is lost.
-  string16 fallback_name_;
-  string16 product_name_;
 
   DISALLOW_COPY_AND_ASSIGN(DefaultSearchProviderChange);
 };
@@ -69,14 +45,12 @@ class DefaultSearchProviderChange : public SettingChange {
 DefaultSearchProviderChange::DefaultSearchProviderChange(
     const TemplateURL* old_url,
     const TemplateURL* new_url)
-    : old_id_(0),
-      new_id_(0),
-      fallback_id_(0),
-      product_name_(l10n_util::GetStringUTF16(IDS_PRODUCT_NAME)) {
-  if (new_url) {
+    : SettingChange(kSearchEngineChanged),
+      old_id_(0),
+      new_id_(0) {
+  DCHECK(new_url);
   new_id_ = new_url->id();
   new_name_ = new_url->short_name();
-  }
   if (old_url) {
     old_id_ = old_url->id();
     old_name_ = old_url->short_name();
@@ -86,124 +60,55 @@ DefaultSearchProviderChange::DefaultSearchProviderChange(
 DefaultSearchProviderChange::~DefaultSearchProviderChange() {
 }
 
-bool DefaultSearchProviderChange::Init(Protector* protector) {
-  // Initially reset the search engine to its previous setting.
-  const TemplateURL* current_url =
-      SetDefaultSearchProvider(protector, old_id_, true);
-  if (!current_url)
-    return false;
-  if (!old_id_ || current_url->id() != old_id_) {
-    // Old settings is lost or invalid, so we had to fall back to one of the
-    // prepopulated search engines.
-    fallback_id_ = current_url->id();
-    fallback_name_ = current_url->short_name();
-    VLOG(1) << "Fallback to " << fallback_name_;
-  }
-  return true;
+string16 DefaultSearchProviderChange::GetOldSetting() const {
+  return old_name_;
 }
 
-void DefaultSearchProviderChange::Apply(Protector* protector) {
-  // TODO(avayvod): Add histrogram.
-  if (!new_id_) {
-    // Open settings page in case the new setting is invalid.
-    OpenSearchEngineSettings(protector);
-  } else {
-    SetDefaultSearchProvider(protector, new_id_, false);
-  }
+string16 DefaultSearchProviderChange::GetNewSetting() const {
+  return new_name_;
 }
 
-void DefaultSearchProviderChange::Discard(Protector* protector) {
+void DefaultSearchProviderChange::Accept(Protector* protector) {
+  SetDefaultSearchProvider(protector, new_id_);
   // TODO(avayvod): Add histrogram.
-  if (!old_id_) {
-    // Open settings page in case the old setting is invalid.
-    OpenSearchEngineSettings(protector);
-  }
-  // Nothing to do otherwise since we have already set the search engine
-  // to |old_id_| in |Init|.
-}
-
-string16 DefaultSearchProviderChange::GetTitle() const {
-  return l10n_util::GetStringUTF16(IDS_SEARCH_ENGINE_CHANGE_TITLE);
 }
 
-string16 DefaultSearchProviderChange::GetMessage() const {
-  if (fallback_name_.empty())
-    return l10n_util::GetStringFUTF16(
-        IDS_SEARCH_ENGINE_CHANGE_MESSAGE, product_name_);
-  else
-    return l10n_util::GetStringFUTF16(
-        IDS_SEARCH_ENGINE_CHANGE_NO_BACKUP_MESSAGE,
-        product_name_, fallback_name_);
-}
-
-string16 DefaultSearchProviderChange::GetApplyButtonText() const {
-  if (new_id_) {
-    if (new_id_ == fallback_id_) {
-      // Old search engine is lost, the fallback search engine is the same as
-      // the new one so no need to show this button.
-      return string16();
-    }
-    if (new_name_.length() > kMaxDisplayedNameLength)
-      return l10n_util::GetStringUTF16(IDS_CHANGE_SEARCH_ENGINE_NO_NAME);
-    else
-      return l10n_util::GetStringFUTF16(IDS_CHANGE_SEARCH_ENGINE, new_name_);
-  } else if (old_id_) {
-    // New setting is lost, offer to go to settings.
-    return l10n_util::GetStringUTF16(IDS_SELECT_SEARCH_ENGINE);
-  } else {
-    // Both settings are lost: don't show this button.
-    return string16();
+void DefaultSearchProviderChange::Revert(Protector* protector) {
+  SetDefaultSearchProvider(protector, old_id_);
+  if (!old_id_) {
+    // Open settings page in case the original setting was lost.
+    protector->OpenTab(
+        GURL(std::string(chrome::kChromeUISettingsURL) +
+            chrome::kSearchEnginesSubPage));
   }
+  // TODO(avayvod): Add histrogram.
 }
 
-string16 DefaultSearchProviderChange::GetDiscardButtonText() const {
-  if (old_id_) {
-    if (new_name_.length() > kMaxDisplayedNameLength)
-      return l10n_util::GetStringUTF16(IDS_KEEP_SETTING);
-    else
-      return l10n_util::GetStringFUTF16(IDS_KEEP_SEARCH_ENGINE, old_name_);
-  } else {
-    // Old setting is lost, offer to go to settings.
-    return l10n_util::GetStringUTF16(IDS_SELECT_SEARCH_ENGINE);
-  }
+void DefaultSearchProviderChange::DoDefault(Protector* protector) {
+  SetDefaultSearchProvider(protector, old_id_);
+  // TODO(avayvod): Add histrogram.
 }
 
-const TemplateURL* DefaultSearchProviderChange::SetDefaultSearchProvider(
+void DefaultSearchProviderChange::SetDefaultSearchProvider(
     Protector* protector,
-    int64 id,
-    bool allow_fallback) {
+    int64 id) {
+  DCHECK(protector);
   TemplateURLService* url_service = protector->GetTemplateURLService();
   if (!url_service) {
-    NOTREACHED() << "Can't get TemplateURLService object.";
-    return NULL;
+    LOG(WARNING) << "Can't get TemplateURLService object.";
+    return;
   }
   const TemplateURL* url = NULL;
-  if (id) {
   const TemplateURLService::TemplateURLVector& urls =
       url_service->GetTemplateURLs();
-    for (size_t i = 0; i < urls.size(); ++i) {
+  for (size_t i = 0; i < urls.size(); ++i)
     if (urls[i]->id() == id) {
       url = urls[i];
       break;
     }
-    }
-  }
-  if (!url && allow_fallback) {
+  if (!url)
     url = url_service->FindNewDefaultSearchProvider();
-    DCHECK(url);
-  }
-  if (url) {
   url_service->SetDefaultSearchProvider(url);
-    VLOG(1) << "Default search provider set to: " << url->short_name();
-  }
-  return url;
-}
-
-void DefaultSearchProviderChange::OpenSearchEngineSettings(
-    Protector* protector) {
-  protector->OpenTab(
-      GURL(std::string(chrome::kChromeUISettingsURL) +
-           chrome::kSearchEnginesSubPage));
 }
 
 SettingChange* CreateDefaultSearchProviderChange(

chrome/browser/protector/protector.cc

@@ -4,7 +4,6 @@
 
 #include "chrome/browser/protector/protector.h"
 
-#include "base/bind.h"
 #include "base/logging.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/protector/settings_change_global_error.h"
@@ -42,43 +41,36 @@ TemplateURLService* Protector::GetTemplateURLService() {
 
 void Protector::ShowChange(SettingChange* change) {
   DCHECK(change);
-  BrowserThread::PostTask(
-      BrowserThread::UI, FROM_HERE,
-      base::Bind(&Protector::InitAndShowChange,
-                 base::Unretained(this), change));
-}
+  SettingChangeVector changes(1, change);
 
-void Protector::InitAndShowChange(SettingChange* change) {
-  VLOG(1) << "Init change";
-  if (!change->Init(this)) {
-    VLOG(1) << "Error while initializing, removing ourselves";
-    delete change;
-    OnRemovedFromProfile();
-    return;
-  }
-  error_.reset(new SettingsChangeGlobalError(change, this));
+  error_.reset(new SettingsChangeGlobalError(changes, this));
   error_->ShowForProfile(profile_);
 }
 
-void Protector::OnApplyChange() {
-  VLOG(1) << "Apply change";
-  error_->mutable_change()->Apply(this);
+void Protector::OnApplyChanges() {
+  OnChangesAction(&SettingChange::Accept);
 }
 
-void Protector::OnDiscardChange() {
-  VLOG(1) << "Discard change";
-  error_->mutable_change()->Discard(this);
+void Protector::OnDiscardChanges() {
+  OnChangesAction(&SettingChange::Revert);
 }
 
 void Protector::OnDecisionTimeout() {
-  // TODO(ivankr): Add histogram.
-  VLOG(1) << "Timeout";
+  OnChangesAction(&SettingChange::DoDefault);
 }
 
 void Protector::OnRemovedFromProfile() {
   BrowserThread::DeleteSoon(BrowserThread::UI, FROM_HERE, this);
 }
 
+void Protector::OnChangesAction(SettingChangeAction action) {
+  DCHECK(error_.get());
+  SettingChangeVector* changes = error_->mutable_changes();
+  for (SettingChangeVector::iterator it = changes->begin();
+       it != changes->end(); ++it)
+    ((*it)->*action)(this);
+}
+
 
 std::string SignSetting(const std::string& value) {
   crypto::HMAC hmac(crypto::HMAC::SHA256);

chrome/browser/protector/protector.h

@@ -21,8 +21,9 @@ namespace protector {
 
 class SettingsChangeGlobalError;
 
-// Presents a SettingChange to user and handles possible user actions.
-// Deletes itself after a user action is taken or timeout expires.
+// Accumulates settings changes and shows them altogether to user.
+// Deletes itself when changes are shown to the user and some action is taken
+// or timeout expires.
 class Protector : public SettingsChangeGlobalErrorDelegate {
  public:
   explicit Protector(Profile* profile);
@@ -36,12 +37,12 @@ class Protector : public SettingsChangeGlobalErrorDelegate {
   TemplateURLService* GetTemplateURLService();
 
   // Shows global error about the specified change. Ownership of the change
-  // is passed to the GlobalError object.
+  // is passed to the error object.
   void ShowChange(SettingChange* change);
 
   // SettingsChangeGlobalErrorDelegate implementation.
-  virtual void OnApplyChange() OVERRIDE;
-  virtual void OnDiscardChange() OVERRIDE;
+  virtual void OnApplyChanges() OVERRIDE;
+  virtual void OnDiscardChanges() OVERRIDE;
   virtual void OnDecisionTimeout() OVERRIDE;
   virtual void OnRemovedFromProfile() OVERRIDE;
 
@@ -51,10 +52,9 @@ class Protector : public SettingsChangeGlobalErrorDelegate {
   // The object can only be allocated and destroyed on heap.
   virtual ~Protector();
 
-  // Performs the initial action on settings change and shows it. This is run
-  // asynchronously on UI thread because |ShowChange| may be called in the
-  // middle of some operations on settings that have changed.
-  void InitAndShowChange(SettingChange* change);
+  // Common handler for error delegate handlers. Calls the specified method
+  // on each change we showed error for.
+  void OnChangesAction(SettingChangeAction action);
 
   // Pointer to error bubble controller. Indicates if we're showing change
   // notification to user. Owns itself.

chrome/browser/protector/setting_change.h

@@ -21,43 +21,49 @@ class Protector;
 // Base class for setting change tracked by Protector.
 class SettingChange {
  public:
-  SettingChange() {}
-  virtual ~SettingChange() {}
+  // IDs of changes Protector currently tracks.
+  enum Type {
+    // Default search engine has been changed.
+    kSearchEngineChanged,
 
-  // Applies initial actions to the setting if needed. Must be called before
-  // any other calls are made, including text getters. Returns true if
-  // initialization was successful.
-  virtual bool Init(Protector* protector) { return true; }
+    // Home page has been changed.
+    kHomePageChanged,
+  };
 
-  // Persists new setting if needed.
-  virtual void Apply(Protector* protector) {}
+  explicit SettingChange(Type type) : type_(type) {}
+  virtual ~SettingChange() {}
 
-  // Restores old setting if needed.
-  virtual void Discard(Protector* protector) {}
+  Type type() const { return type_; }
 
-  // Returns the wrench menu item and bubble title.
-  virtual string16 GetTitle() const = 0;
+  // Returns the old setting presentation to be shown to user.
+  // Returns empty string if the old setting is unavailable.
+  virtual string16 GetOldSetting() const = 0;
 
-  // Returns the bubble message text.
-  virtual string16 GetMessage() const = 0;
+  // Returns the new setting presentation to be shown to user.
+  virtual string16 GetNewSetting() const = 0;
 
-  // Returns text for the button to apply the change with |Apply|.
-  // Returns empty string if no apply button should be shown.
-  virtual string16 GetApplyButtonText() const = 0;
+  // Persists new setting if needed.
+  virtual void Accept(Protector* protector) {}
+
+  // Restores old setting value if needed.
+  virtual void Revert(Protector* protector) {}
 
-  // Returns text for the button to discard the change with |Discard|.
-  virtual string16 GetDiscardButtonText() const = 0;
+  // Called when user ignored the change.
+  virtual void DoDefault(Protector* protector) {}
 
  private:
+  // Type of the change. Used for strings lookup by UI.
+  // TODO(avayvod): Refactor string selection logic via polymorphism.
+  Type type_;
+
   DISALLOW_COPY_AND_ASSIGN(SettingChange);
 };
 
-// TODO(ivankr): CompositeSettingChange that incapsulates multiple
-// SettingChange instances.
+typedef std::vector<SettingChange*> SettingChangeVector;
+typedef void (SettingChange::*SettingChangeAction)(Protector*);
 
 // Allocates and initializes SettingChange implementation for default search
-// provider setting. Both |actual| and |backup| may be NULL if corresponding
-// values are unknown or invalid.
+// provider setting.
 SettingChange* CreateDefaultSearchProviderChange(
     const TemplateURL* actual,
     const TemplateURL* backup);

chrome/browser/protector/settings_change_global_error.cc

@@ -15,6 +15,9 @@
 #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;
 
@@ -25,21 +28,58 @@ 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(
-    SettingChange* change,
+    const SettingChangeVector& changes,
     SettingsChangeGlobalErrorDelegate* delegate)
-    : change_(change),
+    : changes_(changes),
       delegate_(delegate),
       profile_(NULL),
       browser_(NULL),
       closed_by_button_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)) {
-  DCHECK(delegate_);
+  DCHECK(changes.size() > 0);
 }
 
 SettingsChangeGlobalError::~SettingsChangeGlobalError() {
+  STLDeleteElements(&changes_);
 }
 
 bool SettingsChangeGlobalError::HasBadge() {
@@ -54,8 +94,12 @@ 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 change_->GetTitle();
+  return l10n_util::GetStringUTF16(kMenuItemLabelIDs[changes_.front()->type()]);
 }
 
 void SettingsChangeGlobalError::ExecuteMenuItem(Browser* browser) {
@@ -70,37 +114,58 @@ bool SettingsChangeGlobalError::HasBubbleView() {
 }
 
 string16 SettingsChangeGlobalError::GetBubbleViewTitle() {
-  return change_->GetTitle();
+  return l10n_util::GetStringUTF16(kBubbleTitleIDs[changes_.front()->type()]);
 }
 
 string16 SettingsChangeGlobalError::GetBubbleViewMessage() {
-  return change_->GetMessage();
+  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());
+  }
 }
 
-// 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() {
-  return change_->GetDiscardButtonText();
+  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);
+  }
 }
 
 string16 SettingsChangeGlobalError::GetBubbleViewCancelButtonLabel() {
-  return change_->GetApplyButtonText();
+  SettingChange* change = changes_.front();
+  return l10n_util::GetStringFUTF16(kBubbleChangeSettingIDs[change->type()],
+                                    change->GetNewSetting());
 }
 
 void SettingsChangeGlobalError::BubbleViewAcceptButtonPressed() {
   closed_by_button_ = true;
-  delegate_->OnDiscardChange();
+  DCHECK(delegate_);
+  VLOG(1) << "Discard changes";
+  delegate_->OnDiscardChanges();
 }
 
 void SettingsChangeGlobalError::BubbleViewCancelButtonPressed() {
   closed_by_button_ = true;
-  delegate_->OnApplyChange();
+  DCHECK(delegate_);
+  VLOG(1) << "Apply changes";
+  delegate_->OnApplyChanges();
 }
 
 void SettingsChangeGlobalError::RemoveFromProfile() {
+  DCHECK(delegate_);
   if (profile_)
     GlobalErrorServiceFactory::GetForProfile(profile_)->RemoveGlobalError(this);
   if (!closed_by_button_)

chrome/browser/protector/settings_change_global_error.h

@@ -8,7 +8,6 @@
 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
-#include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/protector/setting_change.h"
 #include "chrome/browser/ui/global_error.h"
@@ -23,9 +22,11 @@ class SettingsChangeGlobalErrorDelegate;
 // Global error about unwanted settings changes.
 class SettingsChangeGlobalError : public GlobalError {
  public:
-  // Creates new global error about setting changes |change| and takes
-  // ownership over it. Uses |delegate| to notify about user decision.
-  SettingsChangeGlobalError(SettingChange* change,
+  // Creates new global error about settings changes |changes|. Takes
+  // ownership over |changes| contents.
+  // Uses |delegate| to notify about user decision.
+  SettingsChangeGlobalError(
+      const SettingChangeVector& changes,
       SettingsChangeGlobalErrorDelegate* delegate);
   virtual ~SettingsChangeGlobalError();
 
@@ -36,7 +37,7 @@ class SettingsChangeGlobalError : public GlobalError {
   // Browser that the bubble has been last time shown for.
   Browser* browser() const { return browser_; }
 
-  SettingChange* mutable_change() { return change_.get(); }
+  SettingChangeVector* mutable_changes() { return &changes_; }
 
   // GlobalError implementation.
   virtual bool HasBadge() OVERRIDE;
@@ -64,8 +65,8 @@ class SettingsChangeGlobalError : public GlobalError {
   // Removes global error from its profile and deletes |this| later.
   void RemoveFromProfile();
 
-  // Change to show.
-  scoped_ptr<SettingChange> change_;
+  // List of changes to show.
+  SettingChangeVector changes_;
 
   // Delegate to notify about user actions.
   SettingsChangeGlobalErrorDelegate* delegate_;

chrome/browser/protector/settings_change_global_error_delegate.h

@@ -16,10 +16,10 @@ class SettingsChangeGlobalErrorDelegate {
   virtual ~SettingsChangeGlobalErrorDelegate() {}
 
   // Called if user clicks "Apply change" button.
-  virtual void OnApplyChange() = 0;
+  virtual void OnApplyChanges() = 0;
 
   // Called if user clicks "Discard change" button.
-  virtual void OnDiscardChange() = 0;
+  virtual void OnDiscardChanges() = 0;
 
   // Called if user clicked outside the bubble and timeout for its reshow
   // has passed.
@@ -32,3 +32,4 @@ class SettingsChangeGlobalErrorDelegate {
 }  // namespace protector
 
 #endif  // CHROME_BROWSER_PROTECTOR_SETTINGS_CHANGE_GLOBAL_ERROR_DELEGATE_H_
+