Protect against NULL PasswordStore

chrome/browser/ui/webui/options/password_manager_handler.cc

 // Copyright (c) 2011 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 "chrome/browser/ui/webui/options/password_manager_handler.h"
 
 #include "base/callback.h"
 #include "base/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/google/google_util.h"
 #include "chrome/browser/prefs/pref_service.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
 #include "content/common/notification_details.h"
 #include "content/common/notification_source.h"
 #include "grit/chromium_strings.h"
 #include "grit/generated_resources.h"
 #include "net/base/net_util.h"
 #include "ui/base/l10n/l10n_util.h"
 #include "webkit/glue/password_form.h"
 
 PasswordManagerHandler::PasswordManagerHandler()
     : ALLOW_THIS_IN_INITIALIZER_LIST(populater_(this)),
       ALLOW_THIS_IN_INITIALIZER_LIST(exception_populater_(this)) {
 }
 
 PasswordManagerHandler::~PasswordManagerHandler() {
-  GetPasswordStore()->RemoveObserver(this);
+  PasswordStore* store = GetPasswordStore();
+  if (store)
+    store->RemoveObserver(this);
 }
 
 void PasswordManagerHandler::GetLocalizedValues(
     DictionaryValue* localized_strings) {
   DCHECK(localized_strings);
 
   static const OptionsStringResource resources[] = {
     { "savedPasswordsTitle",
       IDS_PASSWORDS_SHOW_PASSWORDS_TAB_TITLE },
     { "passwordExceptionsTitle",
@@ skipping 26 matching lines @@
 
   localized_strings->SetString("passwordManagerLearnMoreURL",
       google_util::AppendGoogleLocaleParam(
           GURL(chrome::kPasswordManagerLearnMoreURL)).spec());
 }
 
 void PasswordManagerHandler::Initialize() {
   show_passwords_.Init(prefs::kPasswordManagerAllowShowPasswords,
                        web_ui_->GetProfile()->GetPrefs(), this);
   // We should not cache web_ui_->GetProfile(). See crosbug.com/6304.
-  GetPasswordStore()->AddObserver(this);
+  PasswordStore* store = GetPasswordStore();
+  if (store)
+    store->AddObserver(this);
+  else
+    NOTREACHED();

[James Hawkins, 2011/05/18 17:52:27]

I'm not a fan of the NOTREACHEDs. This will not hit in the wild (it's
essentially a DCHECK), so it's unlikely we'll get any data from it. We've
previously said there are reasons why the password store may not exist (likely
to corrupt profiles), so handling this as a potential case is the right thing to
do (and not pretend like it's unexpected).

 }
 
 void PasswordManagerHandler::RegisterMessages() {
   DCHECK(web_ui_);
 
   web_ui_->RegisterMessageCallback("updatePasswordLists",
       NewCallback(this, &PasswordManagerHandler::UpdatePasswordLists));
   web_ui_->RegisterMessageCallback("removeSavedPassword",
       NewCallback(this, &PasswordManagerHandler::RemoveSavedPassword));
   web_ui_->RegisterMessageCallback("removePasswordException",
@@ skipping 30 matching lines @@
   password_list_.reset();
   password_exception_list_.reset();
 
   languages_ =
       web_ui_->GetProfile()->GetPrefs()->GetString(prefs::kAcceptLanguages);
   populater_.Populate();
   exception_populater_.Populate();
 }
 
 void PasswordManagerHandler::RemoveSavedPassword(const ListValue* args) {
+  PasswordStore* store = GetPasswordStore();
+  if (!store) {
+    NOTREACHED();
+    return;
+  }
   std::string string_value = UTF16ToUTF8(ExtractStringValue(args));
   int index;
   base::StringToInt(string_value, &index);
-  GetPasswordStore()->RemoveLogin(*password_list_[index]);
+  store->RemoveLogin(*password_list_[index]);
 }
 
 void PasswordManagerHandler::RemovePasswordException(
     const ListValue* args) {
+  PasswordStore* store = GetPasswordStore();
+  if (!store) {
+    NOTREACHED();
+    return;
+  }
   std::string string_value = UTF16ToUTF8(ExtractStringValue(args));
   int index;
   base::StringToInt(string_value, &index);
 
-  GetPasswordStore()->RemoveLogin(*password_exception_list_[index]);
+  store->RemoveLogin(*password_exception_list_[index]);
 }
 
 void PasswordManagerHandler::RemoveAllSavedPasswords(
     const ListValue* args) {
   // TODO(jhawkins): This will cause a list refresh for every password in the
   // list. Add PasswordStore::RemoveAllLogins().
   PasswordStore* store = GetPasswordStore();
+  if (!store) {
+    NOTREACHED();
+    return;
+  }
   for (size_t i = 0; i < password_list_.size(); ++i)
     store->RemoveLogin(*password_list_[i]);
 }
 
 void PasswordManagerHandler::RemoveAllPasswordExceptions(
     const ListValue* args) {
   PasswordStore* store = GetPasswordStore();
+  if (!store) {
+    NOTREACHED();
+    return;
+  }
   for (size_t i = 0; i < password_exception_list_.size(); ++i)
     store->RemoveLogin(*password_exception_list_[i]);
 }
 
 void PasswordManagerHandler::SetPasswordList() {
   ListValue entries;
   bool show_passwords = *show_passwords_;
   string16 empty;
   for (size_t i = 0; i < password_list_.size(); ++i) {
     ListValue* entry = new ListValue();
@@ skipping 78 matching lines @@
     OnPasswordStoreRequestDone(
         CancelableRequestProvider::Handle handle,
         const std::vector<webkit_glue::PasswordForm*>& result) {
   DCHECK_EQ(pending_login_query_, handle);
   pending_login_query_ = 0;
   page_->password_exception_list_.reset();
   page_->password_exception_list_.insert(page_->password_exception_list_.end(),
                                          result.begin(), result.end());
   page_->SetPasswordExceptionList();
 }