Handle empty string in GetSavePasswordDialogTitleTextAndLinkRange gracefully

chrome/browser/ui/passwords/manage_passwords_view_utils_unittest.cc

Index: chrome/browser/ui/passwords/manage_passwords_view_utils_unittest.cc
diff --git a/chrome/browser/ui/passwords/manage_passwords_view_utils_unittest.cc b/chrome/browser/ui/passwords/manage_passwords_view_utils_unittest.cc
index bbc2a7dc44caff80e7eb74957a6d0d955942414b..78293e94f6beefcfdf689f9b5498cef76cb45c5d 100644
--- a/chrome/browser/ui/passwords/manage_passwords_view_utils_unittest.cc
+++ b/chrome/browser/ui/passwords/manage_passwords_view_utils_unittest.cc
@@ -7,17 +7,60 @@
 #include <stddef.h>
 
 #include "base/macros.h"
+#include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
+#include "chrome/browser/browser_process.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/strings/grit/components_strings.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/base/l10n/l10n_util.h"
+#include "ui/base/resource/resource_bundle.h"
 #include "ui/gfx/range/range.h"
 #include "url/gurl.h"
 
 namespace {
 
+// ScopedResourceOverride allows overriding localised strings in the shared
+// instance of the resource bundle, while restoring the bundle state on
+// destruction.
+class ScopedResourceOverride {
+ public:
+  ScopedResourceOverride()
+      : had_shared_instance_(ui::ResourceBundle::HasSharedInstance()),
+        bundle_(GetOrCreateSharedInstance()),
+        app_locale_(g_browser_process->GetApplicationLocale()) {}
+
+  ~ScopedResourceOverride() {
+    if (had_shared_instance_) {
+      // Reloading the resources will discard all overrides.
+      bundle_.ReloadLocaleResources(app_locale_);
+    } else {
+      ui::ResourceBundle::CleanupSharedInstance();
+    }
+  }
+
+  void OverrideLocaleStringResource(int string_id, const base::string16& str) {
+    bundle_.OverrideLocaleStringResource(string_id, str);
+  }
+
+ private:
+  // Returns the shared resource bundle. Creates one if there was none.
+  static ui::ResourceBundle& GetOrCreateSharedInstance() {
+    if (!ui::ResourceBundle::HasSharedInstance()) {
+      ui::ResourceBundle::InitSharedInstanceWithLocale(
+          "en", nullptr, ui::ResourceBundle::LOAD_COMMON_RESOURCES);
+    }
+    return ui::ResourceBundle::GetSharedInstance();
+  }
+
+  const bool had_shared_instance_;  // Was there a shared bundle before?
+  ui::ResourceBundle& bundle_;  // The shared bundle.
+  const std::string app_locale_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedResourceOverride);
+};
+
 const struct {
   const char* const user_visible_url;
   const char* const form_origin_url;
@@ -124,6 +167,39 @@ TEST(ManagePasswordsViewUtilTest, GetSavePasswordDialogTitleTextAndLinkRange) {
   }
 }
 
+// Check that empty localised strings do not cause a crash.
+TEST(ManagePasswordsViewUtilTest,
+     GetSavePasswordDialogTitleTextAndLinkRange_EmptyStrings) {
+  ScopedResourceOverride resource_override;
+
+  // Ensure that the resource bundle returns an empty string for the UI.
+  resource_override.OverrideLocaleStringResource(IDS_SAVE_PASSWORD,
+                                                 base::string16());
+
+  base::string16 title;
+  gfx::Range title_link_range;
+  const GURL kExample("http://example.org");
+  const bool kBrandingEnabled = true;
+  // The arguments passed below have this importance for the codepath:
+  // * The first two URLs need to be the same, otherwise
+  //   IDS_SAVE_PASSWORD_DIFFERENT_DOMAINS_TITLE will be used instead of
+  //   IDS_SAVE_PASSWORD overridden above.
+  // * |kBrandingEnabled| needs to be true, otherwise the code won't try to
+  //   dereference placeholder offsets from the localised string, which
+  //   triggers the crash in http://crbug.com/658902.
+  // * SAVE_PASSWORD dialog type needs to be passed to match the
+  //   IDS_SAVE_PASSWORD overridden above.
+  GetSavePasswordDialogTitleTextAndLinkRange(
+      kExample, kExample, kBrandingEnabled, PasswordTitleType::SAVE_PASSWORD,
+      &title, &title_link_range);
+  // Verify that the test did not pass just because
+  // GetSavePasswordDialogTitleTextAndLinkRange changed the resource IDs it uses
+  // (and hence did not get the overridden empty string). If the empty localised
+  // string was used, the title and the range will be empty as well.
+  EXPECT_TRUE(title_link_range.is_empty());
+  EXPECT_TRUE(title.empty()) << "|title| is non-empty: " << title;

[vasilii, 2016/11/25 10:47:24]

You can say EXPECT_THAT(title, IsEmpty()); to get the same result.

[vabr (Chromium), 2016/11/25 11:22:53]

Done.

+}
+
 TEST(ManagePasswordsViewUtilTest, GetManagePasswordsDialogTitleText) {
   for (size_t i = 0; i < arraysize(kDomainsTestCases); ++i) {
     SCOPED_TRACE(testing::Message() << "user_visible_url = "