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

chrome/browser/protector/protector_service.cc

Index: chrome/browser/protector/protector_service.cc
diff --git a/chrome/browser/protector/protector_service.cc b/chrome/browser/protector/protector_service.cc
index ce8b20a85cb2e331655130d13bba30920718fad4..4b790a780ba8c917fab979c75058d26fd40c1e22 100644
--- a/chrome/browser/protector/protector_service.cc
+++ b/chrome/browser/protector/protector_service.cc
@@ -5,16 +5,34 @@
 #include "chrome/browser/protector/protector_service.h"
 
 #include "base/logging.h"
+#include "chrome/browser/google/google_util.h"
 #include "chrome/browser/prefs/pref_service.h"
-#include "chrome/browser/protector/settings_change_global_error.h"
+#include "chrome/browser/protector/composite_settings_change.h"
+#include "chrome/browser/protector/keys.h"
 #include "chrome/browser/protector/protected_prefs_watcher.h"
+#include "chrome/browser/protector/settings_change_global_error.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/notification_source.h"
+#include "net/base/registry_controlled_domain.h"
 
 namespace protector {
 
+namespace {
+
+// Returns true if changes with URLs |url1| and |url2| can be merged.
+bool CanMerge(const GURL& url1, const GURL& url2) {
+  VLOG(1) << "Checking if can merge " << url1.spec() << " with " << url2.spec();
+  // All Google URLs are considered the same one.
+  if (google_util::IsGoogleHostname(url1.host()))
+    return google_util::IsGoogleHostname(url2.host());
+  // Otherwise URLs must have the same domain.
+  return net::RegistryControlledDomainService::SameDomainOrHost(url1, url2);
+}
+
+}  // namespace
+
 ProtectorService::ProtectorService(Profile* profile)
     : profile_(profile),
       has_active_change_(false) {
@@ -28,20 +46,37 @@ ProtectorService::~ProtectorService() {
 
 void ProtectorService::ShowChange(BaseSettingChange* change) {
   DCHECK(change);
-  Item new_item;
-  new_item.change.reset(change);
+
   DVLOG(1) << "Init change";
   if (!change->Init(profile_)) {
     LOG(WARNING) << "Error while initializing, dismissing change";
+    delete change;
     return;
   }
-  SettingsChangeGlobalError* error =
-      new SettingsChangeGlobalError(change, this);
-  new_item.error.reset(error);
-  items_.push_back(new_item);
-  // Do not show the bubble immediately if another one is active.
-  error->AddToProfile(profile_, !has_active_change_);
-  has_active_change_ = true;
+
+  Item* item_to_merge_with = FindItemToMergeWith(change);
+  if (item_to_merge_with) {
+    // CompositeSettingsChange takes ownership of merged changes.
+    BaseSettingChange* existing_change = item_to_merge_with->change.release();
+    CompositeSettingsChange* merged_change = existing_change->MergeWith(change);
+    item_to_merge_with->change.reset(merged_change);
+    item_to_merge_with->was_merged = true;
+    if (item_to_merge_with->error->GetBubbleView())
+      item_to_merge_with->show_when_merged = true;
+    // Remove old GlobalError instance. Later in OnRemovedFromProfile() a new
+    // GlobalError instance will be created for the composite change.
+    item_to_merge_with->error->RemoveFromProfile();
+  } else {
+    Item new_item;
+    SettingsChangeGlobalError* error =
+        new SettingsChangeGlobalError(change, this);
+    new_item.error.reset(error);
+    new_item.change.reset(change);
+    items_.push_back(new_item);
+    // Do not show the bubble immediately if another one is active.
+    error->AddToProfile(profile_, !has_active_change_);
+    has_active_change_ = true;
+  }
 }
 
 bool ProtectorService::IsShowingChange() const {
@@ -61,8 +96,8 @@ void ProtectorService::DiscardChange(BaseSettingChange* change,
 }
 
 void ProtectorService::DismissChange(BaseSettingChange* change) {
-  std::vector<Item>::iterator item =
-      std::find_if(items_.begin(), items_.end(), MatchItemByChange(change));
+  Items::iterator item = std::find_if(items_.begin(), items_.end(),
+                                      MatchItemByChange(change));
   DCHECK(item != items_.end());
   item->error->RemoveFromProfile();
 }
@@ -76,6 +111,19 @@ ProtectedPrefsWatcher* ProtectorService::GetPrefsWatcher() {
   return prefs_watcher_.get();
 }
 
+ProtectorService::Item* ProtectorService::FindItemToMergeWith(
+    const BaseSettingChange* change) {
+  if (!change->CanBeMerged())
+    return NULL;
+  GURL url = change->GetNewSettingURL();
+  for (Items::iterator item = items_.begin(); item != items_.end(); item++) {
+    if (item->change->CanBeMerged() &&
+        CanMerge(url, item->change->GetNewSettingURL()))
+      return &*item;
+  }
+  return NULL;
+}
+
 void ProtectorService::Shutdown() {
   while (IsShowingChange())
     items_[0].error->RemoveFromProfile();
@@ -101,13 +149,27 @@ void ProtectorService::OnDecisionTimeout(SettingsChangeGlobalError* error) {
 }
 
 void ProtectorService::OnRemovedFromProfile(SettingsChangeGlobalError* error) {
-  std::vector<Item>::iterator item =
-      std::find_if(items_.begin(), items_.end(), MatchItemByError(error));
+  Items::iterator item = std::find_if(items_.begin(), items_.end(),
+                                      MatchItemByError(error));
   DCHECK(item != items_.end());
+
+  if (item->was_merged) {
+    bool show_new_error = !has_active_change_ || item->show_when_merged;
+    item->was_merged = false;
+    item->show_when_merged = false;
+    // Item was merged with another change instance and error has been removed,
+    // create a new one for the composite change.
+    item->error.reset(new SettingsChangeGlobalError(item->change.get(), this));
+    item->error->AddToProfile(profile_, show_new_error);
+    has_active_change_ = true;
+    return;
+  }
+
   items_.erase(item);
 
+  // If no other change is shown and there are changes that haven't been shown
+  // yet, show the first one.
   if (!has_active_change_) {
-    // If there are changes that haven't been shown yet, show the first one.
     for (item = items_.begin(); item != items_.end(); ++item) {
       if (!item->error->HasShownBubbleView()) {
         item->error->ShowBubble();
@@ -122,7 +184,9 @@ BaseSettingChange* ProtectorService::GetLastChange() {
   return items_.empty() ? NULL : items_.back().change.get();
 }
 
-ProtectorService::Item::Item() {
+ProtectorService::Item::Item()
+    : was_merged(false),
+      show_when_merged(false) {
 }
 
 ProtectorService::Item::~Item() {
@@ -134,7 +198,7 @@ ProtectorService::MatchItemByChange::MatchItemByChange(
 
 bool ProtectorService::MatchItemByChange::operator()(
     const ProtectorService::Item& item) {
-  return other_ == item.change.get();
+  return item.change->Contains(other_);
 }
 
 ProtectorService::MatchItemByError::MatchItemByError(