final fix for login dialog: AddRef and Platform changes

rogerwang · rogerwang · commit 005a97e3efe6 · 2013-11-29T12:47:04.000+08:00
diff --git a/src/browser/shell_login_dialog.cc b/src/browser/shell_login_dialog.cc
@@ -28,6 +28,7 @@
 #include "content/public/browser/notification_registrar.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/resource_dispatcher_host.h"
+#include "content/public/browser/resource_request_info.h"
 #include "net/base/auth.h"
 #include "net/url_request/url_request.h"
 #include "ui/base/text/text_elider.h"
@@ -51,10 +52,23 @@ ShellLoginDialog::ShellLoginDialogList ShellLoginDialog::dialog_queue_;
 
 ShellLoginDialog::ShellLoginDialog(
     net::AuthChallengeInfo* auth_info,
-    net::URLRequest* request) : auth_info_(auth_info),
-                                request_(request),
-                                handled_auth_(false) {
+    net::URLRequest* request)
+  : render_process_id_(0),
+    render_view_id_(0),
+    auth_info_(auth_info),
+    request_(request),
+    handled_auth_(false) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+
+  if (!ResourceRequestInfo::ForRequest(request_)->GetAssociatedRenderView(
+          &render_process_id_,  &render_view_id_)) {
+    NOTREACHED();
+  }
+
+  // prevent object destruction on wrong (IO) thread
+  // paired with ReleaseSoon()
+  AddRef();
+
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       base::Bind(&ShellLoginDialog::PrepDialog, this,
diff --git a/src/browser/shell_login_dialog.h b/src/browser/shell_login_dialog.h
@@ -95,6 +95,9 @@ class ShellLoginDialog : public ResourceDispatcherHostLoginDelegate,
   // Threading: any
   virtual ~ShellLoginDialog();
 
+  int render_process_id_;
+  int render_view_id_;
+
  private:
 
   // popup next dialog in queue on closing of this dialog
diff --git a/src/browser/shell_login_dialog_gtk.cc b/src/browser/shell_login_dialog_gtk.cc
@@ -94,6 +94,8 @@ void ShellLoginDialog::PlatformCreateDialog(const string16& message) {
   gtk_box_pack_start(GTK_BOX(content_area), table, FALSE, FALSE, 0);
 
   g_signal_connect(root_, "response", G_CALLBACK(OnResponseThunk), this);
+  g_signal_connect(root_, "destroy",  G_CALLBACK(OnDestroyThunk), this);
+
   gtk_widget_grab_focus(username_entry_);
 }
 
diff --git a/src/browser/shell_login_dialog_mac.mm b/src/browser/shell_login_dialog_mac.mm
@@ -134,6 +134,7 @@ - (void)cancel {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   [helper_ release];
   helper_ = nil;
+  ReleaseSoon();
 }
 
 void ShellLoginDialog::PlatformRequestCancelled() {
diff --git a/src/browser/shell_login_dialog_win.cc b/src/browser/shell_login_dialog_win.cc
@@ -52,10 +52,15 @@ INT_PTR CALLBACK ShellLoginDialog::DialogProc(HWND dialog,
           GetWindowLongPtr(dialog, DWL_USER));
       owner->UserCancelledAuth();
       DestroyWindow(owner->dialog_win_);
-      owner->dialog_win_ = NULL;
-      ReleaseSoon();
       break;
     }
+    case WM_DESTROY: {
+      ShellLoginDialog* owner = reinterpret_cast<ShellLoginDialog*>(
+          GetWindowLongPtr(dialog, DWL_USER));
+      owner->dialog_win_ = NULL;
+      owner->ReleaseSoon();
+	  break;
+    }
     case WM_COMMAND: {
       ShellLoginDialog* owner = reinterpret_cast<ShellLoginDialog*>(
           GetWindowLongPtr(dialog, DWL_USER));
@@ -73,7 +78,8 @@ INT_PTR CALLBACK ShellLoginDialog::DialogProc(HWND dialog,
       } else if (LOWORD(wparam) == IDCANCEL) {
         owner->UserCancelledAuth();
       } else {
-        NOTREACHED();
+        DLOG(INFO) << "wparam is " << LOWORD(wparam);
+        // NOTREACHED();
       }
 
       break;
@@ -88,16 +94,9 @@ INT_PTR CALLBACK ShellLoginDialog::DialogProc(HWND dialog,
 void ShellLoginDialog::PlatformCreateDialog(const string16& message) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  int render_process_id;
-  int render_view_id;
-  if (!ResourceRequestInfo::ForRequest(request_)->GetAssociatedRenderView(
-          &render_process_id,  &render_view_id)) {
-    NOTREACHED();
-  }
-
   WebContents* web_contents = NULL;
   RenderViewHost* render_view_host =
-      RenderViewHost::FromID(render_process_id, render_view_id);
+      RenderViewHost::FromID(render_process_id_, render_view_id_);
   if (render_view_host)
     web_contents = WebContents::FromRenderViewHost(render_view_host);
   DCHECK(web_contents);

Original file line number	Diff line number	Diff line change
`@@ -94,6 +94,8 @@ void ShellLoginDialog::PlatformCreateDialog(const string16& message) {`
`94`	`94`	`gtk_box_pack_start(GTK_BOX(content_area), table, FALSE, FALSE, 0);`
`95`	`95`
`96`	`96`	`g_signal_connect(root_, "response", G_CALLBACK(OnResponseThunk), this);`
	`97`	`+ g_signal_connect(root_, "destroy", G_CALLBACK(OnDestroyThunk), this);`
	`98`	`+`
`97`	`99`	`gtk_widget_grab_focus(username_entry_);`
`98`	`100`	`}`
`99`	`101`
Original file line number	Diff line number	Diff line change
`@@ -134,6 +134,7 @@ - (void)cancel {`
`134`	`134`	`DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));`
`135`	`135`	`[helper_ release];`
`136`	`136`	`helper_ = nil;`
	`137`	`+ ReleaseSoon();`
`137`	`138`	`}`
`138`	`139`
`139`	`140`	`void ShellLoginDialog::PlatformRequestCancelled() {`