Fix a race condition when an IconAnimation is still running at browser shutdown.

Fix a race condition when an IconAnimation is still running at browser shutdown.

Because Extensions are refcounted, they can be destroyed from
~ProfileIOData on the IO thread instead of the UI thread.  If an
animation is running at that time, and the ExtensionAction owns its
animations, then the animation's destructor will CHECK-fail while
destroying its Timer, because the Timer has bound itself to the UI
thread.

After this patch, IconAnimationWrappers own themselves, and use
WeakPtrs to indirectly remove themselves from the ExtensionAction.  To
limit the size of the ExtensionAction's map, we remove all NULLs each
time we observe a single animation as having finished.

BUG=133141
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=149636

[not at google - send to devlin, 2012-08-02 14:44:25]

lgtm

https://chromiumcodereview.appspot.com/10823142/diff/1008/chrome/common/exten...
File chrome/common/extensions/extension_action.cc (right):

https://chromiumcodereview.appspot.com/10823142/diff/1008/chrome/common/exten...
chrome/common/extensions/extension_action.cc:357: icon_animation_.begin(); it !=
icon_animation_.end();) {
nits:

you could re-use the iterator? saves some typing, if a bit strange.

In any case, I'd typedef std::map<int, base::WeakPtr<IconAnimationWrapper> > to
something.

https://chromiumcodereview.appspot.com/10823142/diff/1008/chrome/test/data/ex...
File chrome/test/data/extensions/api_test/script_badge/basics/background.js
(right):

https://chromiumcodereview.appspot.com/10823142/diff/1008/chrome/test/data/ex...
chrome/test/data/extensions/api_test/script_badge/basics/background.js:11: if
(/^http:/.test(details.url)) {
details.url.indexOf('http://') === 0

is a common idiom.

[Jeffrey Yasskin, 2012-08-02 15:11:49]

Thanks!

https://chromiumcodereview.appspot.com/10823142/diff/1008/chrome/common/exten...
File chrome/common/extensions/extension_action.cc (right):

https://chromiumcodereview.appspot.com/10823142/diff/1008/chrome/common/exten...
chrome/common/extensions/extension_action.cc:357: icon_animation_.begin(); it !=
icon_animation_.end();) {
On 2012/08/02 14:44:25, kalman wrote:
> nits:
> 
> you could re-use the iterator? saves some typing, if a bit strange.

Sure.

> In any case, I'd typedef std::map<int, base::WeakPtr<IconAnimationWrapper> >
to
> something.

With the re-used iterator, this map type only appears twice, so I don't think
it's worth obscuring with a typedef.

https://chromiumcodereview.appspot.com/10823142/diff/1008/chrome/test/data/ex...
File chrome/test/data/extensions/api_test/script_badge/basics/background.js
(right):

https://chromiumcodereview.appspot.com/10823142/diff/1008/chrome/test/data/ex...
chrome/test/data/extensions/api_test/script_badge/basics/background.js:11: if
(/^http:/.test(details.url)) {
On 2012/08/02 14:44:25, kalman wrote:
> details.url.indexOf('http://') === 0
> 
> is a common idiom.

Cool. Keep the idioms coming: I don't claim to know Javascript. :)

[commit-bot: I haz the power, 2012-08-02 15:12:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jyasskin@chromium.org/10823142/5001

[commit-bot: I haz the power, 2012-08-02 16:28:38]

Change committed as 149636