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

Unified Diff: chrome/common/extensions/extension_action.cc

Issue 10823142: Fix a race condition when an IconAnimation is still running at browser shutdown. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Fix nits. Created 8 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: chrome/common/extensions/extension_action.cc
diff --git a/chrome/common/extensions/extension_action.cc b/chrome/common/extensions/extension_action.cc
index e045e32ee0fbab00d05cc27fd0a2705294b10136..022fcbd94a698a60d68f648b4d401ce385fe7790 100644
--- a/chrome/common/extensions/extension_action.cc
+++ b/chrome/common/extensions/extension_action.cc
@@ -61,15 +61,15 @@ int Width(const gfx::Image& image) {
} // namespace
-// Wraps an IconAnimation and implements its ui::AnimationDelegate to erase the
-// animation from a map when the animation ends or is cancelled, causing itself
-// and its owned IconAnimation to be deleted.
-class ExtensionAction::IconAnimationWrapper : public ui::AnimationDelegate {
+// Wraps an IconAnimation and implements its ui::AnimationDelegate to delete
+// itself when the animation ends or is cancelled, causing its owned
+// IconAnimation to be destroyed.
+class ExtensionAction::IconAnimationWrapper
+ : public ui::AnimationDelegate,
+ public base::SupportsWeakPtr<IconAnimationWrapper> {
public:
- IconAnimationWrapper(ExtensionAction* owner, int tab_id)
- : owner_(owner),
- tab_id_(tab_id),
- ALLOW_THIS_IN_INITIALIZER_LIST(animation_(this)) {}
+ IconAnimationWrapper()
+ : ALLOW_THIS_IN_INITIALIZER_LIST(animation_(this)) {}
virtual ~IconAnimationWrapper() {}
@@ -87,12 +87,9 @@ class ExtensionAction::IconAnimationWrapper : public ui::AnimationDelegate {
}
void Done() {
- owner_->icon_animation_.erase(tab_id_);
- // this will now have been deleted.
+ delete this;
}
- ExtensionAction* owner_;
- int tab_id_;
IconAnimation animation_;
};
@@ -344,26 +341,47 @@ void ExtensionAction::PaintBadge(gfx::Canvas* canvas,
canvas->Restore();
}
-base::WeakPtr<ExtensionAction::IconAnimation> ExtensionAction::GetIconAnimation(
+ExtensionAction::IconAnimationWrapper* ExtensionAction::GetIconAnimationWrapper(
int tab_id) const {
- std::map<int, linked_ptr<IconAnimationWrapper> >::const_iterator it =
+ std::map<int, base::WeakPtr<IconAnimationWrapper> >::iterator it =
icon_animation_.find(tab_id);
- return (it != icon_animation_.end()) ? it->second->animation()->AsWeakPtr()
- : base::WeakPtr<IconAnimation>();
+ if (it == icon_animation_.end())
+ return NULL;
+ if (it->second)
+ return it->second;
+
+ // Take this opportunity to remove all the NULL IconAnimationWrappers from
+ // icon_animation_.
+ icon_animation_.erase(it);
+ for (it = icon_animation_.begin(); it != icon_animation_.end();) {
+ if (it->second) {
+ ++it;
+ } else {
+ // The WeakPtr is null; remove it from the map.
+ icon_animation_.erase(it++);
+ }
+ }
+ return NULL;
+}
+
+base::WeakPtr<ExtensionAction::IconAnimation> ExtensionAction::GetIconAnimation(
+ int tab_id) const {
+ IconAnimationWrapper* wrapper = GetIconAnimationWrapper(tab_id);
+ return wrapper ? wrapper->animation()->AsWeakPtr()
+ : base::WeakPtr<IconAnimation>();
}
gfx::Image ExtensionAction::ApplyIconAnimation(int tab_id,
const gfx::Image& orig) const {
- std::map<int, linked_ptr<IconAnimationWrapper> >::const_iterator it =
- icon_animation_.find(tab_id);
- if (it == icon_animation_.end())
+ IconAnimationWrapper* wrapper = GetIconAnimationWrapper(tab_id);
+ if (wrapper == NULL)
return orig;
- return gfx::Image(it->second->animation()->Apply(*orig.ToSkBitmap()));
+ return gfx::Image(wrapper->animation()->Apply(*orig.ToSkBitmap()));
}
void ExtensionAction::RunIconAnimation(int tab_id) {
IconAnimationWrapper* icon_animation =
- new IconAnimationWrapper(this, tab_id);
- icon_animation_[tab_id] = make_linked_ptr(icon_animation);
+ new IconAnimationWrapper();
+ icon_animation_[tab_id] = icon_animation->AsWeakPtr();
icon_animation->animation()->Start();
}
« no previous file with comments | « chrome/common/extensions/extension_action.h ('k') | chrome/test/data/extensions/api_test/script_badge/basics/background.js » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698