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

Issue 12319104: Fix crasher in tab icon indicator (Closed)

Created:
7 years, 10 months ago by cpu_(ooo_6.6-7.5)
Modified:
7 years, 10 months ago
Reviewers:
sky
CC:
chromium-reviews, tfarina
Visibility:
Public.

Description

Fix crasher in tab icon indicator We believe from evidence that the audio animation and the recording / projecting animation make the state of icon_animation_ get out of whack For example, consider: 1- that we are projecting 2- then a sound is played in the tab 3- then the sounds ends 4- tab needs to be repainted At #3 the icon_animation_ is .reset() yet capture_state != CAPTURE_STATE_NONE So at #4 we would crash as seen in the bug comment #1 BUG=178154 TEST=see bug Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=184518

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -33 lines) Patch
M chrome/browser/ui/views/tabs/tab.h View 1 chunk +1 line, -5 lines 0 comments Download
M chrome/browser/ui/views/tabs/tab.cc View 4 chunks +20 lines, -28 lines 1 comment Download

Messages

Total messages: 4 (0 generated)
cpu_(ooo_6.6-7.5)
.
7 years, 10 months ago (2013-02-25 22:48:49 UTC) #1
sky
LGTM https://codereview.chromium.org/12319104/diff/1/chrome/browser/ui/views/tabs/tab.cc File chrome/browser/ui/views/tabs/tab.cc (right): https://codereview.chromium.org/12319104/diff/1/chrome/browser/ui/views/tabs/tab.cc#newcode1606 chrome/browser/ui/views/tabs/tab.cc:1606: icon_animation_->Stop(); Do you need the Stop()? Can this ...
7 years, 10 months ago (2013-02-25 22:58:12 UTC) #2
cpu_(ooo_6.6-7.5)
Good question, the main difference is that in ~Animation() dtor we don't call the delegate ...
7 years, 10 months ago (2013-02-25 23:06:43 UTC) #3
cpu_(ooo_6.6-7.5)
7 years, 10 months ago (2013-02-25 23:47:46 UTC) #4
Message was sent while issue was closed.
Committed patchset #1 manually as r184518 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698