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

chrome/browser/protector/session_startup_change.cc

Index: chrome/browser/protector/session_startup_change.cc
diff --git a/chrome/browser/protector/session_startup_change.cc b/chrome/browser/protector/session_startup_change.cc
index c1d6b191c8e5c4b1c1e7eb9264830b261f87464e..b2644fb81bec7f07457d2df018715a4f9ecf8f47 100644
--- a/chrome/browser/protector/session_startup_change.cc
+++ b/chrome/browser/protector/session_startup_change.cc
@@ -23,6 +23,13 @@
 
 namespace protector {
 
+namespace {
+
+// Priority for the display name.
+const size_t kNamePriority = 50U;

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

Maybe we should put the priorities in a single file so it's easy to figure out
which change has a greater one?
Otherwise I have to look up all priorities when adding a new change type to see
if the priority for it is correctly aligned with the others.

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

Done.

+
+}  // namespace
+
 // Session startup settings change tracked by Protector.
 class SessionStartupChange : public BasePrefsChange {
  public:
@@ -43,10 +50,16 @@ class SessionStartupChange : public BasePrefsChange {
   virtual string16 GetBubbleMessage() const OVERRIDE;
   virtual string16 GetApplyButtonText() const OVERRIDE;
   virtual string16 GetDiscardButtonText() const OVERRIDE;
+  virtual DisplayName GetApplyDisplayName() const OVERRIDE;
+  virtual std::string GetApplyKeyword() const OVERRIDE;
 
  private:
   virtual ~SessionStartupChange();
 
+  // Returns the first URL among new startup pages. Returns an empty URL if
+  // there are no new startup pages.
+  GURL GetFirstNewURL() const;

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

What if the first new URL is the same as the old one?
I.e. third party adds their homepage as the last one?

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

I considered that a non-common use case but you're right, ideally we should
compare new and old URLs and pick the first in new that wasn't present in old.
Added a TODO.

+
   // Opens all tabs in |tabs| and makes them pinned.
   void OpenPinnedTabs(Browser* browser, const PinnedTabCodec::Tabs& tabs);
 
@@ -139,16 +152,8 @@ string16 SessionStartupChange::GetApplyButtonText() const {
   if (new_startup_pref_.type == SessionStartupPref::LAST)
       return l10n_util::GetStringUTF16(IDS_CHANGE_STARTUP_SETTINGS_RESTORE);
 
-  string16 first_url;
-  if (new_startup_pref_.type == SessionStartupPref::URLS &&
-      !new_startup_pref_.urls.empty()) {
-    // Display the domain name of the first statrup URL.
-    first_url = UTF8ToUTF16(new_startup_pref_.urls[0].host());
-  } else if (!new_pinned_tabs_.empty()) {
-    // Start with NTP or no URLs (basically the same): display the domain name
-    // of the first pinned tab, if any.
-    first_url = UTF8ToUTF16(new_pinned_tabs_[0].url.host());
-  }
+  // Display the domain name of the first startup/pinned URL.
+  string16 first_url = UTF8ToUTF16(GetFirstNewURL().host());

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

so is it first_url or first_url_host?

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

Fixed.

   return first_url.empty() ?
       l10n_util::GetStringUTF16(IDS_CHANGE_STARTUP_SETTINGS_NTP) :
       l10n_util::GetStringFUTF16(IDS_CHANGE_STARTUP_SETTINGS_URLS,
@@ -159,6 +164,27 @@ string16 SessionStartupChange::GetDiscardButtonText() const {
   return l10n_util::GetStringUTF16(IDS_KEEP_SETTING);
 }
 
+BaseSettingChange::DisplayName
+SessionStartupChange::GetApplyDisplayName() const {
+  string16 first_url = UTF8ToUTF16(GetFirstNewURL().host());
+  return first_url.empty() ? DisplayName(kDefaultNamePriority, string16()) :
+      DisplayName(kNamePriority, first_url);
+}
+
+std::string SessionStartupChange::GetApplyKeyword() const {
+  return GetFirstNewURL().host();
+}
+
+GURL SessionStartupChange::GetFirstNewURL() const {

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

I guess it could return .host() since it's what every caller is calling
afterwards.

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

Done.

+  if (new_startup_pref_.type == SessionStartupPref::URLS &&
+      !new_startup_pref_.urls.empty()) {
+    return new_startup_pref_.urls[0];
+  }
+  if (!new_pinned_tabs_.empty())
+    return new_pinned_tabs_[0].url;
+  return GURL();
+}
+
 void SessionStartupChange::OpenPinnedTabs(
     Browser* browser,
     const PinnedTabCodec::Tabs& tabs) {