There is a race condition when the HtmlDialogView is closed which causes a crash while dereferencing an invalid

delegate_ (HtmlDialogUIDelegate*) member. I could not reproduce this consistently though. The inference is
as below:-

1. When the dialog is closed the HtmlDialogView::OnDialogClosed member function is invoked which calls the
   OnDialogClosed function on the delegate. This in turn causes the delegate to be destroyed.
2. It then sets the delegate to NULL and attempts to close the window.
3. Before the Close method is dispatched if the view attempts to Paint it causes a crash in the
   HtmlDialogView::GetWindowTitle function because of dereferencing a NULL delegate_.

Fix is to add corresponding NULL checks in the relevant functions.

This fixes http://b/issue?id=2138035, which was reported with ChromeFrame.

Bug=2138035


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

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

1 files changed, 25 insertions, 9 deletions

diff --git a/chrome/browser/views/html_dialog_view.cc b/chrome/browser/views/html_dialog_view.cc
index 56c152f..ea8476a 100644
--- a/chrome/browser/views/html_dialog_view.cc
+++ b/chrome/browser/views/html_dialog_view.cc
@@ -12,7 +12,7 @@
 
 namespace browser {
 
-// Declared in browser_dialogs.h so that others don't need to depend on our .h. 
+// Declared in browser_dialogs.h so that others don't need to depend on our .h.
 void ShowHtmlDialogView(gfx::NativeWindow parent, Browser* browser,
                         HtmlDialogUIDelegate* delegate) {
   HtmlDialogView* html_view = new HtmlDialogView(browser, delegate);
@@ -43,7 +43,8 @@ HtmlDialogView::~HtmlDialogView() {
 
 gfx::Size HtmlDialogView::GetPreferredSize() {
   gfx::Size out;
-  delegate_->GetDialogSize(&out);
+  if (delegate_)
+    delegate_->GetDialogSize(&out);
   return out;
 }
 
@@ -55,11 +56,17 @@ bool HtmlDialogView::CanResize() const {
 }
 
 bool HtmlDialogView::IsModal() const {
-  return delegate_->IsDialogModal();
+  if (delegate_)
+    return delegate_->IsDialogModal();
+  else
+    return false;
 }
 
 std::wstring HtmlDialogView::GetWindowTitle() const {
-  return delegate_->GetDialogTitle();
+  if (delegate_)
+    return delegate_->GetDialogTitle();
+  else
+    return std::wstring();
 }
 
 void HtmlDialogView::WindowClosing() {
@@ -90,25 +97,34 @@ std::wstring HtmlDialogView::GetDialogTitle() const {
 }
 
 GURL HtmlDialogView::GetDialogContentURL() const {
-  return delegate_->GetDialogContentURL();
+  if (delegate_)
+    return delegate_->GetDialogContentURL();
+  else
+    return GURL();
 }
 
 void HtmlDialogView::GetDOMMessageHandlers(
     std::vector<DOMMessageHandler*>* handlers) const {
-  delegate_->GetDOMMessageHandlers(handlers);
+  if (delegate_)
+    delegate_->GetDOMMessageHandlers(handlers);
 }
 
 void HtmlDialogView::GetDialogSize(gfx::Size* size) const {
-  delegate_->GetDialogSize(size);
+  if (delegate_)
+    delegate_->GetDialogSize(size);
 }
 
 std::string HtmlDialogView::GetDialogArgs() const {
-  return delegate_->GetDialogArgs();
+  if (delegate_)
+    return delegate_->GetDialogArgs();
+  else
+    return std::string();
 }
 
 void HtmlDialogView::OnDialogClosed(const std::string& json_retval) {
-  delegate_->OnDialogClosed(json_retval);
+  HtmlDialogUIDelegate* dialog_delegate = delegate_;
   delegate_ = NULL;  // We will not communicate further with the delegate.
+  dialog_delegate->OnDialogClosed(json_retval);
   window()->Close();
 }