[protector] Homepage UI polishing.

chrome/browser/protector/homepage_change_unittest.cc

Index: chrome/browser/protector/homepage_change_unittest.cc
diff --git a/chrome/browser/protector/homepage_change_unittest.cc b/chrome/browser/protector/homepage_change_unittest.cc
index cfea686369125679e6ec2e0f8d24119bf31d0231..1c56a1c69e11111a9ad0e93e97d4e480e3b5b1d4 100644
--- a/chrome/browser/protector/homepage_change_unittest.cc
+++ b/chrome/browser/protector/homepage_change_unittest.cc
@@ -9,7 +9,9 @@
 #include "chrome/browser/protector/base_setting_change.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/test/base/testing_profile.h"
+#include "grit/generated_resources.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "ui/base/l10n/l10n_util.h"
 
 namespace protector {
 
@@ -60,4 +62,43 @@ TEST_F(HomepageChangeTest, InitAndDiscard) {
   EXPECT_EQ(kHomepage2, prefs_->GetString(prefs::kHomePage));
 }
 
+TEST_F(HomepageChangeTest, DiscardButtonCaptions) {
+  // NTP -> NTP.
+  scoped_ptr<BaseSettingChange> change(
+      CreateHomepageChange(kHomepage1, true, true, kHomepage2, true, true));
+  ASSERT_TRUE(change->Init(&profile_));
+  string16 generic_caption(l10n_util::GetStringUTF16(IDS_KEEP_SETTING));
+  EXPECT_EQ(generic_caption, change->GetDiscardButtonText());
+
+  // NTP -> NTP.
+  change.reset(
+      CreateHomepageChange("", false, true, kHomepage2, true, true));
+  ASSERT_TRUE(change->Init(&profile_));
+  EXPECT_EQ(generic_caption, change->GetDiscardButtonText());
+
+  // NTP -> NTP.

[whywhat, 2012/04/29 16:24:15]

The comments before the last three sections are identical, but code is
different. What's the truth?
Also, consider splitting test cases into several to make it easier to understand
which case was broken (it still could be done by line number now).

[Ivan Korotkov, 2012/05/02 11:17:26]

You're right, they'd better be separated.
NTP can be represented either as empty homepage URL or any URL with
"homepage_is_ntp = true", hence different code sections with the same comment.

[whywhat, 2012/05/02 11:23:48]

My point is that the comments should explain this difference between the
sections.

[Ivan Korotkov, 2012/05/02 11:29:40]

Done.

+  change.reset(
+      CreateHomepageChange(kHomepage1, true, true, "", false, true));
+  ASSERT_TRUE(change->Init(&profile_));
+  EXPECT_EQ(generic_caption, change->GetDiscardButtonText());
+
+  // NTP -> kHomepage2.
+  change.reset(
+      CreateHomepageChange(kHomepage1, true, true, kHomepage2, false, true));
+  ASSERT_TRUE(change->Init(&profile_));
+  EXPECT_NE(generic_caption, change->GetDiscardButtonText());
+
+  // kHomepage1 -> NTP.
+  change.reset(
+      CreateHomepageChange(kHomepage1, false, true, kHomepage2, true, true));
+  ASSERT_TRUE(change->Init(&profile_));
+  EXPECT_NE(generic_caption, change->GetDiscardButtonText());
+
+  // kHomepage1 -> kHomepage1.
+  change.reset(
+      CreateHomepageChange(kHomepage1, false, true, kHomepage1, false, true));
+  ASSERT_TRUE(change->Init(&profile_));
+  EXPECT_EQ(generic_caption, change->GetDiscardButtonText());
+}
+
 }  // namespace protector