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

chrome/browser/protector/protector_service.h

 // 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.
 
 #ifndef CHROME_BROWSER_PROTECTOR_PROTECTOR_SERVICE_H_
 #define CHROME_BROWSER_PROTECTOR_PROTECTOR_SERVICE_H_
 #pragma once
 
 #include <vector>
 
@@ skipping 53 matching lines @@
 
   // Returns the most recent change instance or NULL if there are no changes.
   BaseSettingChange* GetLastChange();
 
   // Returns the Profile instance for this service.
   Profile* profile() { return profile_; }
 
  private:
   friend class ProtectorServiceTest;
 
   // Pair of error and corresponding change instance. linked_ptr is used because

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

It's not a pair anymore.
BTW, maybe Items should be std::vector<linked_ptr<Item>> so change here and
error could be just pointers or scoped_ptrs? 

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

Fixed comment.
It's easier actually to work with vector<Item> than with
vector<linked_ptr<Item>> and the performance difference shouldn't be big here.

   // Item instances are stored in a std::vector and must be copyable.
   struct Item {
     Item();
     ~Item();
     linked_ptr<BaseSettingChange> change;
     linked_ptr<SettingsChangeGlobalError> error;
+    // When true, this means |change| was merged with another instance and
+    // |error| is in process of being removed from GlobalErrorService.
+    bool is_merging;

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

Why merging, not merged as in comment?

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

Fixed

+    // Meaningful only when |merged| is true. In that case, true means that the
+    // new merged GlobalError instance will be immediately shown.
+    bool show_when_merged;

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

This makes three states represented with two booleans.
Maybe use an enum with three constants?

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

Hm, it doesn't look like a tri-state flag to merge into a single enum, I think.

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

Both booleans above define three states out of possible four as you state above
(show_when_merged is meaningful only when is_merged is true)

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

I meant that, enum would be good if those were 3 distinct states but (single,
merged and merged,needs show) don't look like distinct states, 2 of them are
nearly the same apart from the show bit.

   };
 
+  typedef std::vector<Item> Items;
+
   // Matches Item by |change| field.
   class MatchItemByChange {
    public:
     explicit MatchItemByChange(const BaseSettingChange* other);
 
     bool operator()(const Item& item);
 
    private:
     const BaseSettingChange* other_;
   };
 
   // Matches Item by |error| field.
   class MatchItemByError {
    public:
     explicit MatchItemByError(const SettingsChangeGlobalError* other);
 
     bool operator()(const Item& item);
 
    private:
     const SettingsChangeGlobalError* other_;
   };
 
+  // Returns an Item instance whose change can be merged with |change|, if any.

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

What if there're multiple items that can be merged?

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

Merging strategy is (and should be) transitive so it shouldn't happen :)

+  // Otherwise returns |NULL|.
+  Item* FindItemToMergeWith(const BaseSettingChange* change);
+
   // ProfileKeyedService implementation.
   virtual void Shutdown() OVERRIDE;
 
   // SettingsChangeGlobalErrorDelegate implementation.
   virtual void OnApplyChange(SettingsChangeGlobalError* error,
                              Browser* browser) OVERRIDE;
   virtual void OnDiscardChange(SettingsChangeGlobalError* error,
                                Browser* browser) OVERRIDE;
   virtual void OnDecisionTimeout(SettingsChangeGlobalError* error) OVERRIDE;
   virtual void OnRemovedFromProfile(SettingsChangeGlobalError* error) OVERRIDE;
 
   // Pointers to error bubble controllers and corresponding changes in the order
   // added.
-  std::vector<Item> items_;
+  Items items_;
 
   // Profile which settings we are protecting.
   Profile* profile_;
 
   // True if there is a change that has been shown and not yet accepted or
   // discarded by user.
   bool has_active_change_;
 
   // Observes changes to protected prefs and updates the backup.
   scoped_ptr<ProtectedPrefsWatcher> prefs_watcher_;
 
   DISALLOW_COPY_AND_ASSIGN(ProtectorService);
 };
 
 }  // namespace protector
 
 #endif  // CHROME_BROWSER_PROTECTOR_PROTECTOR_SERVICE_H_