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

chrome/browser/protector/base_setting_change.cc

Index: chrome/browser/protector/base_setting_change.cc
diff --git a/chrome/browser/protector/base_setting_change.cc b/chrome/browser/protector/base_setting_change.cc
index 805d48b96f17813477f95bb27031e170361e0c62..04aae0abe7a518930adad3ca5ce489149b905785 100644
--- a/chrome/browser/protector/base_setting_change.cc
+++ b/chrome/browser/protector/base_setting_change.cc
@@ -3,11 +3,15 @@
 // found in the LICENSE file.
 
 #include "chrome/browser/protector/base_setting_change.h"
+#include "chrome/browser/protector/composite_settings_change.h"
 
 #include "base/logging.h"
 
 namespace protector {
 
+// static
+const size_t BaseSettingChange::kDefaultNamePriority = 0U;
+
 BaseSettingChange::BaseSettingChange()
     : profile_(NULL) {
 }
@@ -15,6 +19,20 @@ BaseSettingChange::BaseSettingChange()
 BaseSettingChange::~BaseSettingChange() {
 }
 
+CompositeSettingsChange* BaseSettingChange::MergeWith(
+    BaseSettingChange* other) {
+  CompositeSettingsChange* composite_change = new CompositeSettingsChange;

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

nit: I think we should use () always for constructor calls.

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

Done.

+  CHECK(composite_change->Init(profile_));
+  composite_change->MergeWith(this);
+  composite_change->MergeWith(other);
+  return composite_change;
+}
+
+bool BaseSettingChange::Contains(const BaseSettingChange* other) const {
+  // BaseSettingChange can only contain itself.
+  return this == other;
+}
+
 bool BaseSettingChange::Init(Profile* profile) {
   DCHECK(profile);
   profile_ = profile;
@@ -30,4 +48,13 @@ void BaseSettingChange::Discard(Browser* browser) {
 void BaseSettingChange::Timeout() {
 }
 
+BaseSettingChange::DisplayName BaseSettingChange::GetApplyDisplayName() const {
+  return DisplayName(kDefaultNamePriority, string16());
+}
+
+std::string BaseSettingChange::GetApplyKeyword() const {
+  // By default change should not be collapsed with others.
+  return std::string();

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

Maybe this property should be returned by clearer boolean method? I.e.
IsCollapsible or smth like that?

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

In most cases, the IsCollapsible will have the same code as GetApplyKeyword,
just with an .empty() check on end.

[whywhat, 2012/04/10 09:55:44]

So? I see nothing wrong with that :)

[Ivan Korotkov, 2012/04/10 11:19:49]

Er, well, code duplication is bad isn't it? :)
If in future there happen to be changes that have no specific keyword but still
can be merged into a "whatever-change" and, OTOH, changes that absolutely cannot
be merged, I'll add something like CanBeMerged().

[whywhat, 2012/04/10 11:22:08]

There would be none if IsCollapsible() would be implemented like { return
!GetApplyKeyword().empty() } and the nuance of empty keyword meaning something
special would be hidden.

[Ivan Korotkov, 2012/04/10 11:33:17]

Ok, you've convinced me :)

+}
+
 }  // namespace protector