Attempt at fixing possible crash in ModalHTMLDialogDelegate. It's

possible to get more than one NOTIFY_WEB_CONTENTS_DISCONNECTED. The
first time ModalHTMLDialogDelegate gets a
NOTIFY_WEB_CONTENTS_DISCONNECTED it sets the contents_ to NULL. This
is problematic because the destructor than removes the observer using
a source of NULL. Instead we should remove the observer immediately,
then NULL out the contents_.

BUG=4129
TEST=covered by QEMU

Review URL: http://codereview.chromium.org/11413

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@5575 0039d316-1c4b-4281-b951-d872f2087c98

1 files changed, 11 insertions, 4 deletions

diff --git a/chrome/browser/modal_html_dialog_delegate.cc b/chrome/browser/modal_html_dialog_delegate.cc
index a4aec61..46489cc 100644
--- a/chrome/browser/modal_html_dialog_delegate.cc
+++ b/chrome/browser/modal_html_dialog_delegate.cc
@@ -25,9 +25,7 @@ ModalHtmlDialogDelegate::ModalHtmlDialogDelegate(
 }
 
 ModalHtmlDialogDelegate::~ModalHtmlDialogDelegate() {
-  NotificationService::current()->
-      RemoveObserver(this, NOTIFY_WEB_CONTENTS_DISCONNECTED,
-      Source<WebContents>(contents_));
+  RemoveObserver();
 }
 
 void ModalHtmlDialogDelegate::Observe(NotificationType type,
@@ -35,7 +33,7 @@ void ModalHtmlDialogDelegate::Observe(NotificationType type,
                                       const NotificationDetails& details) {
   DCHECK(type == NOTIFY_WEB_CONTENTS_DISCONNECTED);
   DCHECK(Source<WebContents>(source).ptr() == contents_);
-  contents_ = NULL;  // No longer safe to access.
+  RemoveObserver();
 }
 
 bool ModalHtmlDialogDelegate::IsModal() const {
@@ -66,3 +64,12 @@ void ModalHtmlDialogDelegate::OnDialogClosed(const std::string& json_retval) {
   delete this;
 }
 
+void ModalHtmlDialogDelegate::RemoveObserver() {
+  if (!contents_)
+    return;
+
+  NotificationService::current()->
+      RemoveObserver(this, NOTIFY_WEB_CONTENTS_DISCONNECTED,
+      Source<WebContents>(contents_));
+  contents_ = NULL;  // No longer safe to access.
+}