Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(318)

Issue 3052043: Fix crasher with download bar. (Closed)

Created:
10 years, 4 months ago by Jay Civelli
Modified:
9 years, 7 months ago
Reviewers:
sky
CC:
chromium-reviews, Paul Godavari, ben+cc_chromium.org
Base URL:
http://src.chromium.org/git/chromium.git
Visibility:
Public.

Description

Fix crasher with download bar. When dragging out a file, the download bar is shown and disappear automatically once the download is complete. If the download item menu was opened at that point, we would crash. This was because showing the menu runs an inner message loop that would process the hide for the download bar, leading to the download item that was on the call-stack to be deleted, causing a crasher once the stack-unwinded and the deleted object was accessed. BUG=51187 TEST=In GMail, open an email with a large attachment (like 2 MB). Drag the file from GMail to your desktop. Click on the arrow on the item on the download bar to bring up the download menu. Keep the menu open until the download is completed and the bar gets hidden. It should not crash. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=55430

Patch Set 1 #

Total comments: 1

Patch Set 2 : Nulling deleted_ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -1 line) Patch
M chrome/browser/views/download_item_view.h View 1 1 chunk +5 lines, -0 lines 0 comments Download
M chrome/browser/views/download_item_view.cc View 1 3 chunks +11 lines, -1 line 0 comments Download

Messages

Total messages: 3 (0 generated)
Jay Civelli
10 years, 4 months ago (2010-08-06 23:28:28 UTC) #1
sky
LGTM to with the following change. http://codereview.chromium.org/3052043/diff/1/2 File chrome/browser/views/download_item_view.cc (right): http://codereview.chromium.org/3052043/diff/1/2#newcode827 chrome/browser/views/download_item_view.cc:827: Set deleted_ to ...
10 years, 4 months ago (2010-08-06 23:36:55 UTC) #2
Jay Civelli
10 years, 4 months ago (2010-08-07 00:28:40 UTC) #3
> http://codereview.chromium.org/3052043/diff/1/2#newcode827
> chrome/browser/views/download_item_view.cc:827:
> Set deleted_ to NULL here.
Oops! Thanks for catching this!

Powered by Google App Engine
This is Rietveld 408576698