[protector] Homepage UI polishing.

chrome/browser/protector/homepage_change_unittest.cc

 // 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.
 
 #include "base/memory/scoped_ptr.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/prefs/pref_service.h"
 #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 {
 
 namespace {
 
 std::string kHomepage1 = "http://google.com/";
 std::string kHomepage2 = "http://example.com/";
 
 }
 
@@ skipping 30 matching lines @@
       CreateHomepageChange(kHomepage1, false, true, kHomepage2, false, true));
   ASSERT_TRUE(change->Init(&profile_));
   // Setting is initially reverted to backup.
   EXPECT_EQ(kHomepage2, prefs_->GetString(prefs::kHomePage));
 
   change->Discard(NULL);  // |browser| is unused.
   // Nothing changed by Discard.
   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