[protector] Support for collapsing multiple changes into a single one.

chrome/browser/protector/composite_settings_change.cc

Index: chrome/browser/protector/composite_settings_change.cc
diff --git a/chrome/browser/protector/composite_settings_change.cc b/chrome/browser/protector/composite_settings_change.cc
new file mode 100644
index 0000000000000000000000000000000000000000..c3a2a77dbbf327ab495794904517c8f2a5c0f1a3
--- /dev/null
+++ b/chrome/browser/protector/composite_settings_change.cc
@@ -0,0 +1,101 @@
+// Copyright (c) 2012 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.
+
+#include "chrome/browser/protector/composite_settings_change.h"
+
+#include "grit/generated_resources.h"
+#include "ui/base/l10n/l10n_util.h"
+
+namespace protector {
+
+
+CompositeSettingsChange::CompositeSettingsChange() {
+}
+
+CompositeSettingsChange::~CompositeSettingsChange() {
+}
+
+CompositeSettingsChange* CompositeSettingsChange::MergeWith(
+    BaseSettingChange* other) {
+  DCHECK(profile());
+  changes_.push_back(other);

[whywhat, 2012/04/06 16:28:32]

Check for NULL?

[Ivan Korotkov, 2012/04/09 14:46:47]

Done.

+  apply_names_.push(other->GetApplyDisplayName());
+  return this;
+}
+
+bool CompositeSettingsChange::Contains(const BaseSettingChange* other) const {
+  return this == other ||
+      std::find(changes_.begin(), changes_.end(), other) != changes_.end();
+}
+
+void CompositeSettingsChange::Apply(Browser* browser) {
+  DVLOG(1) << "Apply all changes";
+  for (size_t i = 0; i < changes_.size(); ++i)
+    changes_[i]->Apply(browser);

[whywhat, 2012/04/06 16:28:32]

I think we should be careful about applying/discarding changes that interfer
(i.e. when different changes touch the same settings/backup). Do you think such
a thing might happen?

[Ivan Korotkov, 2012/04/09 14:46:47]

It shouldn't with the current *Change classes. The only change interfering with
others is BackupInvalid but it should never be added together with other Prefs
changes.

+}
+
+void CompositeSettingsChange::Discard(Browser* browser) {
+  DVLOG(1) << "Discard all changes";
+  for (size_t i = 0; i < changes_.size(); ++i)
+    changes_[i]->Discard(browser);
+}
+
+void CompositeSettingsChange::Timeout() {
+  DVLOG(1) << "Timeout all changes";
+  for (size_t i = 0; i < changes_.size(); ++i)
+    changes_[i]->Timeout();
+}
+
+int CompositeSettingsChange::GetBadgeIconID() const {
+  DCHECK(changes_.size());
+  // Use icons from the first change.
+  // TODO(ivankr): need something better, maybe a special icon.
+  return changes_[0]->GetBadgeIconID();
+}
+
+int CompositeSettingsChange::GetMenuItemIconID() const {
+  DCHECK(changes_.size());
+  return changes_[0]->GetMenuItemIconID();
+}
+
+int CompositeSettingsChange::GetBubbleIconID() const {
+  DCHECK(changes_.size());
+  return changes_[0]->GetBubbleIconID();
+}
+
+string16 CompositeSettingsChange::GetBubbleTitle() const {
+  return l10n_util::GetStringUTF16(IDS_SETTING_CHANGE_TITLE);
+}
+
+string16 CompositeSettingsChange::GetBubbleMessage() const {
+  // TODO(ivankr): indicate what kind of changes happened (i.e., "something
+  // tried to change your search engine, startup pages, etc.").
+  return l10n_util::GetStringUTF16(IDS_SETTING_CHANGE_BUBBLE_MESSAGE);
+}
+
+string16 CompositeSettingsChange::GetApplyButtonText() const {
+  DCHECK(apply_names_.size());
+  string16 apply_text = apply_names_.top().second;
+  return apply_text.empty() ?
+      l10n_util::GetStringUTF16(IDS_CHANGE_SETTING_NO_NAME) :
+      l10n_util::GetStringFUTF16(IDS_CHANGE_SETTING, apply_text);
+}
+
+string16 CompositeSettingsChange::GetDiscardButtonText() const {
+  return l10n_util::GetStringUTF16(IDS_KEEP_SETTING);
+}
+
+BaseSettingChange::DisplayName
+CompositeSettingsChange::GetApplyDisplayName() const {
+  // CompositeSettingsChange should never be put inside another one.

[whywhat, 2012/04/06 16:28:32]

Maybe this means the class hierarchy is wrong?
Maybe there should be CompositeSettingsChange and ComposableSettingChange and
MergeWith should receive Composable only?

[Ivan Korotkov, 2012/04/09 14:46:47]

I guess you're right but that sounds a little too much heavyweight for now :)

+  NOTREACHED();
+  return DisplayName(kDefaultNamePriority, string16());
+}
+
+std::string CompositeSettingsChange::GetApplyKeyword() const {
+  DCHECK(changes_.size());
+  return changes_[0]->GetApplyKeyword();
+}
+
+}  // namespace protector