Fix Linux new Autofill UI Crash

Only remove gtk signal handler if connected and only hide the popup if it is still visible.

BUG=160866


Review URL: https://chromiumcodereview.appspot.com/11416015

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

5 files changed, 71 insertions, 2 deletions

diff --git a/chrome/browser/autofill/autofill_external_delegate.h b/chrome/browser/autofill/autofill_external_delegate.h
index e013095..d0a4189 100644
--- a/chrome/browser/autofill/autofill_external_delegate.h
+++ b/chrome/browser/autofill/autofill_external_delegate.h
@@ -139,6 +139,8 @@ class AutofillExternalDelegate
   // Return the web_contents associated with this delegate.
   content::WebContents* web_contents() { return web_contents_; }
 
+  bool popup_visible() const { return popup_visible_; }
+
  private:
   // Fills the form with the Autofill data corresponding to |unique_id|.
   // If |is_preview| is true then this is just a preview to show the user what
diff --git a/chrome/browser/autofill/autofill_popup_view.h b/chrome/browser/autofill/autofill_popup_view.h
index b396de0..5240276 100644
--- a/chrome/browser/autofill/autofill_popup_view.h
+++ b/chrome/browser/autofill/autofill_popup_view.h
@@ -41,7 +41,7 @@ class AutofillPopupView {
   // the delegate.
   void ClearExternalDelegate();
 
-  const gfx::Rect& element_bounds() { return element_bounds_; }
+  const gfx::Rect& element_bounds() const { return element_bounds_; }
 
  protected:
   // Display the autofill popup and fill it in with the values passed in.
diff --git a/chrome/browser/ui/gtk/autofill/autofill_external_delegate_gtk.cc b/chrome/browser/ui/gtk/autofill/autofill_external_delegate_gtk.cc
index 95386e6..9ca6f09 100644
--- a/chrome/browser/ui/gtk/autofill/autofill_external_delegate_gtk.cc
+++ b/chrome/browser/ui/gtk/autofill/autofill_external_delegate_gtk.cc
@@ -40,7 +40,8 @@ void AutofillExternalDelegateGtk::HideAutofillPopupInternal() {
   view_.reset();
 
   GtkWidget* toplevel = gtk_widget_get_toplevel(tab_native_view_);
-  g_signal_handler_disconnect(toplevel, event_handler_id_);
+  if (g_signal_handler_is_connected(toplevel, event_handler_id_))
+    g_signal_handler_disconnect(toplevel, event_handler_id_);
 }
 
 void AutofillExternalDelegateGtk::ApplyAutofillSuggestions(
@@ -82,6 +83,9 @@ void AutofillExternalDelegateGtk::CreateViewIfNeeded() {
 
 gboolean AutofillExternalDelegateGtk::HandleViewFocusOut(GtkWidget* sender,
                                                          GdkEventFocus* event) {
+  if (!popup_visible())
+    return FALSE;
+
   HideAutofillPopup();
 
   return TRUE;
diff --git a/chrome/browser/ui/gtk/autofill/autofill_external_delegate_gtk_browsertest.cc b/chrome/browser/ui/gtk/autofill/autofill_external_delegate_gtk_browsertest.cc
new file mode 100644
index 0000000..ceef438
--- /dev/null
+++ b/chrome/browser/ui/gtk/autofill/autofill_external_delegate_gtk_browsertest.cc
@@ -0,0 +1,62 @@
+// Copyright 2012 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/gtk/autofill/autofill_external_delegate_gtk.h"
+
+#include "base/basictypes.h"
+#include "base/memory/scoped_ptr.h"
+#include "chrome/browser/autofill/autofill_manager.h"
+#include "chrome/browser/autofill/test_autofill_external_delegate.h"
+#include "chrome/browser/ui/browser_commands.h"
+#include "chrome/browser/ui/browser_tabstrip.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+
+class MockAutofillExternalDelegateGtk : public AutofillExternalDelegateGtk {
+ public:
+  explicit MockAutofillExternalDelegateGtk(content::WebContents* web_contents)
+      : AutofillExternalDelegateGtk(
+            web_contents,
+            AutofillManager::FromWebContents(web_contents)) {}
+  ~MockAutofillExternalDelegateGtk() {}
+
+  virtual void ClearPreviewedForm() OVERRIDE {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockAutofillExternalDelegateGtk);
+};
+
+}  // namespace
+
+class AutofillExternalDelegateGtkBrowserTest : public InProcessBrowserTest {
+ public:
+  AutofillExternalDelegateGtkBrowserTest() {}
+  virtual ~AutofillExternalDelegateGtkBrowserTest() {}
+
+  virtual void SetUpOnMainThread() OVERRIDE {
+    web_contents_ = chrome::GetActiveWebContents(browser());
+    ASSERT_TRUE(web_contents_ != NULL);
+
+    autofill_external_delegate_.reset(
+        new MockAutofillExternalDelegateGtk(web_contents_));
+  }
+
+ protected:
+  content::WebContents* web_contents_;
+  scoped_ptr<MockAutofillExternalDelegateGtk> autofill_external_delegate_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(AutofillExternalDelegateGtkBrowserTest);
+};
+
+// Ensure that the Autofill code doesn't crash if the window is closed while
+// the Autofill popup is still visible. See http://crbug.com/160866
+IN_PROC_BROWSER_TEST_F(AutofillExternalDelegateGtkBrowserTest,
+                       CloseBrowserWithPopupOpen) {
+  autofill::GenerateTestAutofillPopup(autofill_external_delegate_.get());
+
+  chrome::CloseWindow(browser());
+}
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index c700cec..10f98c2 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1081,6 +1081,7 @@
         'browser/ui/find_bar/find_bar_host_browsertest.cc',
         'browser/ui/fullscreen/fullscreen_controller_browsertest.cc',
         'browser/ui/global_error/global_error_service_browsertest.cc',
+        'browser/ui/gtk/autofill/autofill_external_delegate_gtk_browsertest.cc',
         'browser/ui/gtk/bubble/bubble_gtk_browsertest.cc',
         'browser/ui/gtk/confirm_bubble_gtk_browsertest.cc',
         'browser/ui/gtk/location_bar_view_gtk_browsertest.cc',