Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(34)

Issue 23257002: gtk: Fix use after free (Closed)

Created:
6 years ago by Evan Stade
Modified:
5 years, 11 months ago
CC:
chromium-reviews, James Su
Visibility:
Public.

Description

gtk: Fix use after free This undoes r132498, which I'm pretty sure was barking up the wrong tree. The issue in the bug is that the model is cleared and the popup is /hidden/ (not destroyed), but still has events queued up. LineFromY does attempt to sanitize the result (in case the model has changed without the UI having a chance to catch up), but doesn't handle an empty model. BUG=123530 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=219636

Patch Set 1 #

Patch Set 2 : . #

Total comments: 1

Patch Set 3 : 0U #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -20 lines) Patch
M chrome/browser/ui/gtk/omnibox/omnibox_popup_view_gtk.h View 2 chunks +0 lines, -5 lines 0 comments Download
M chrome/browser/ui/gtk/omnibox/omnibox_popup_view_gtk.cc View 1 2 8 chunks +19 lines, -15 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
Evan Stade
6 years ago (2013-08-15 18:55:24 UTC) #1
Elliot Glaysher
lgtm
6 years ago (2013-08-15 19:45:39 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/estade@chromium.org/23257002/19001
6 years ago (2013-08-15 22:50:37 UTC) #3
commit-bot: I haz the power
Sorry for I got bad news for ya. Compile failed with a clobber build on ...
6 years ago (2013-08-15 23:24:49 UTC) #4
Jered
https://codereview.chromium.org/23257002/diff/19001/chrome/browser/ui/gtk/omnibox/omnibox_popup_view_gtk.cc File chrome/browser/ui/gtk/omnibox/omnibox_popup_view_gtk.cc (right): https://codereview.chromium.org/23257002/diff/19001/chrome/browser/ui/gtk/omnibox/omnibox_popup_view_gtk.cc#newcode464 chrome/browser/ui/gtk/omnibox/omnibox_popup_view_gtk.cc:464: DCHECK_NE(0, model_->result().size()); should be 0U
6 years ago (2013-08-16 21:14:30 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/estade@chromium.org/23257002/44001
5 years, 11 months ago (2013-08-26 17:47:14 UTC) #6
commit-bot: I haz the power
5 years, 11 months ago (2013-08-26 23:14:57 UTC) #7
Message was sent while issue was closed.
Change committed as 219636

Powered by Google App Engine
This is Rietveld 408576698