gtk: Fix use after free

chrome/browser/ui/gtk/omnibox/omnibox_popup_view_gtk.cc

 // Copyright (c) 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/omnibox/omnibox_popup_view_gtk.h"
 
 #include <gtk/gtk.h>
 
 #include <algorithm>
 #include <string>
@@ skipping 13 matching lines @@
 #include "chrome/browser/ui/gtk/gtk_theme_service.h"
 #include "chrome/browser/ui/gtk/gtk_util.h"
 #include "chrome/browser/ui/omnibox/omnibox_edit_model.h"
 #include "chrome/browser/ui/omnibox/omnibox_popup_model.h"
 #include "chrome/browser/ui/omnibox/omnibox_view.h"
 #include "content/public/browser/notification_source.h"
 #include "grit/theme_resources.h"
 #include "ui/base/gtk/gtk_compat.h"
 #include "ui/base/gtk/gtk_hig_constants.h"
 #include "ui/base/gtk/gtk_screen_util.h"
-#include "ui/base/gtk/gtk_signal_registrar.h"
 #include "ui/base/gtk/gtk_windowing.h"
 #include "ui/gfx/color_utils.h"
 #include "ui/gfx/font.h"
 #include "ui/gfx/gtk_util.h"
 #include "ui/gfx/image/cairo_cached_surface.h"
 #include "ui/gfx/image/image.h"
 #include "ui/gfx/rect.h"
 #include "ui/gfx/skia_utils_gtk.h"
 
 namespace {
@@ skipping 215 matching lines @@
 
   pango_layout_set_text(layout, text_utf8.data(), text_utf8.length());
   pango_layout_set_attributes(layout, attrs);  // Ref taken.
   pango_attr_list_unref(attrs);
 }
 
 OmniboxPopupViewGtk::OmniboxPopupViewGtk(const gfx::Font& font,
                                          OmniboxView* omnibox_view,
                                          OmniboxEditModel* edit_model,
                                          GtkWidget* location_bar)
-    : signal_registrar_(new ui::GtkSignalRegistrar),
-      model_(new OmniboxPopupModel(this, edit_model)),
+    : model_(new OmniboxPopupModel(this, edit_model)),
       omnibox_view_(omnibox_view),
       location_bar_(location_bar),
       window_(gtk_window_new(GTK_WINDOW_POPUP)),
       layout_(NULL),
       theme_service_(GtkThemeService::GetFrom(edit_model->profile())),
       font_(font.DeriveFont(kEditFontAdjust)),
       ignore_mouse_drag_(false),
       opened_(false) {
   gtk_widget_set_can_focus(window_, FALSE);
   // Don't allow the window to be resized.  This also forces the window to
   // shrink down to the size of its child contents.
   gtk_window_set_resizable(GTK_WINDOW(window_), FALSE);
   gtk_widget_set_app_paintable(window_, TRUE);
   // Have GTK double buffer around the expose signal.
   gtk_widget_set_double_buffered(window_, TRUE);
 
   // Cache the layout so we don't have to create it for every expose.  If we
   // were a real widget we should handle changing directions, but we're not
   // doing RTL or anything yet, so it shouldn't be important now.
   layout_ = gtk_widget_create_pango_layout(window_, NULL);
   // We don't want the layout of search results depending on their language.
   pango_layout_set_auto_dir(layout_, FALSE);
   // We always ellipsize when drawing our text runs.
   pango_layout_set_ellipsize(layout_, PANGO_ELLIPSIZE_END);
 
   gtk_widget_add_events(window_, GDK_BUTTON_MOTION_MASK |
                                  GDK_POINTER_MOTION_MASK |
                                  GDK_BUTTON_PRESS_MASK |
                                  GDK_BUTTON_RELEASE_MASK);
-  signal_registrar_->Connect(window_, "motion-notify-event",
-                             G_CALLBACK(HandleMotionThunk), this);
-  signal_registrar_->Connect(window_, "button-press-event",
-                             G_CALLBACK(HandleButtonPressThunk), this);
-  signal_registrar_->Connect(window_, "button-release-event",
-                             G_CALLBACK(HandleButtonReleaseThunk), this);
-  signal_registrar_->Connect(window_, "expose-event",
-                             G_CALLBACK(HandleExposeThunk), this);
+  g_signal_connect(window_, "motion-notify-event",
+                   G_CALLBACK(HandleMotionThunk), this);
+  g_signal_connect(window_, "button-press-event",
+                   G_CALLBACK(HandleButtonPressThunk), this);
+  g_signal_connect(window_, "button-release-event",
+                   G_CALLBACK(HandleButtonReleaseThunk), this);
+  g_signal_connect(window_, "expose-event",
+                   G_CALLBACK(HandleExposeThunk), this);
 
   registrar_.Add(this,
                  chrome::NOTIFICATION_BROWSER_THEME_CHANGED,
                  content::Source<ThemeService>(theme_service_));
   theme_service_->InitThemesFor(this);
 
   // TODO(erg): There appears to be a bug somewhere in something which shows
   // itself when we're in NX. Previously, we called
   // gtk_util::ActAsRoundedWindow() to make this popup have rounded
   // corners. This worked on the standard xorg server (both locally and
   // remotely), but broke over NX. My current hypothesis is that it can't
   // handle shaping top-level windows during an expose event, but I'm not sure
   // how else to get accurate shaping information.
   //
   // r25080 (the original patch that added rounded corners here) should
   // eventually be cherry picked once I know what's going
   // on. http://crbug.com/22015.
 }
 
 OmniboxPopupViewGtk::~OmniboxPopupViewGtk() {
-  // Stop listening to our signals before we destroy the model. I suspect that
-  // we can race window destruction, otherwise.
-  signal_registrar_.reset();
-
   // Explicitly destroy our model here, before we destroy our GTK widgets.
   // This is because the model destructor can call back into us, and we need
   // to make sure everything is still valid when it does.
   model_.reset();
   g_object_unref(layout_);
   gtk_widget_destroy(window_);
 }
 
 bool OmniboxPopupViewGtk::IsOpen() const {
   return opened_;
@@ skipping 117 matching lines @@
 
 void OmniboxPopupViewGtk::StackWindow() {
   gfx::NativeView omnibox_view = omnibox_view_->GetNativeView();
   DCHECK(GTK_IS_WIDGET(omnibox_view));
   GtkWidget* toplevel = gtk_widget_get_toplevel(omnibox_view);
   DCHECK(gtk_widget_is_toplevel(toplevel));
   ui::StackPopupWindow(window_, toplevel);
 }
 
 size_t OmniboxPopupViewGtk::LineFromY(int y) {
+  DCHECK_NE(0U, model_->result().size());
   size_t line = std::max(y - kBorderThickness, 0) / kHeightPerResult;
   return std::min(line, model_->result().size() - 1);
 }
 
 void OmniboxPopupViewGtk::AcceptLine(size_t line,
                                      WindowOpenDisposition disposition) {
   // OpenMatch() may close the popup, which will clear the result set and, by
   // extension, |match| and its contents.  So copy the relevant match out to
   // make sure it stays alive until the call completes.
   AutocompleteMatch match = model_->result().match_at(line);
@@ skipping 55 matching lines @@
     *is_selected_keyword = true;
     return;
   }
 
   *match = &result.match_at(index);
   *is_selected_keyword = false;
 }
 
 gboolean OmniboxPopupViewGtk::HandleMotion(GtkWidget* widget,
                                            GdkEventMotion* event) {
+  if (!IsOpen())
+    return FALSE;
+
   // TODO(deanm): Windows has a bunch of complicated logic here.
   size_t line = LineFromY(static_cast<int>(event->y));
   // There is both a hovered and selected line, hovered just means your mouse
   // is over it, but selected is what's showing in the location edit.
   model_->SetHoveredLine(line);
   // Select the line if the user has the left mouse button down.
   if (!ignore_mouse_drag_ && (event->state & GDK_BUTTON1_MASK))
     model_->SetSelectedLine(line, false, false);
   return TRUE;
 }
 
 gboolean OmniboxPopupViewGtk::HandleButtonPress(GtkWidget* widget,
                                                 GdkEventButton* event) {
+  if (!IsOpen())
+    return FALSE;
+
   ignore_mouse_drag_ = false;
   // Very similar to HandleMotion.
   size_t line = LineFromY(static_cast<int>(event->y));
   model_->SetHoveredLine(line);
   if (event->button == 1)
     model_->SetSelectedLine(line, false, false);
   return TRUE;
 }
 
 gboolean OmniboxPopupViewGtk::HandleButtonRelease(GtkWidget* widget,
                                                   GdkEventButton* event) {
+  if (!IsOpen())
+    return FALSE;
+
   if (ignore_mouse_drag_) {
     // See header comment about this flag.
     ignore_mouse_drag_ = false;
     return TRUE;
   }
 
   size_t line = LineFromY(static_cast<int>(event->y));
   switch (event->button) {
     case 1:  // Left click.
       AcceptLine(line, CURRENT_TAB);
@@ skipping 142 matching lines @@
                               theme_service_->GetImageNamed(
                                   is_selected ? IDR_OMNIBOX_TTS_DARK :
                                   IDR_OMNIBOX_TTS),
                               icon_start_x, line_rect.y() + kIconTopPadding);
     }
   }
 
   cairo_destroy(cr);
   return TRUE;
 }