summaryrefslogtreecommitdiffstats
path: root/cc/base
diff options
context:
space:
mode:
authordanakj <danakj@chromium.org>2014-10-29 11:24:49 -0700
committerCommit bot <commit-bot@chromium.org>2014-10-29 18:25:32 +0000
commit1a3c317f3cca7694a4083f56d776607ad49498ca (patch)
tree5297b68f32ea163b767d3741d4ee150aa7836dfb /cc/base
parent5a3210bc7b24168dc00cf94b8c9b314ab6f26382 (diff)
downloadchromium_src-1a3c317f3cca7694a4083f56d776607ad49498ca.zip
chromium_src-1a3c317f3cca7694a4083f56d776607ad49498ca.tar.gz
chromium_src-1a3c317f3cca7694a4083f56d776607ad49498ca.tar.bz2
cc: Fix weakptr destruction of DelayedUniqueNotifier on shutdown.
During shutdown, the ThreadProxy is destroyed on the main thread, after being Closed on the compositor thread. The DelayedUniqueNotifier has posted weak ptrs inside the class, and when the class is destroyed those weak ptrs are invalidated. But the DelayedUniqueNotifier is destroyed on the main thread with ThreadProxy while the weak ptrs were created on the compositor thread, leading to CHECK failures. This adds a new Shutdown() method to DelayedUniqueNotifier that will immediately invalidate the weak ptrs (on the correct thread) and leave the DelayedUniqueNotifier in a state that it's actually not able to post/create new weak ptrs. This resolves the CHECK failures as when the DelayedUniqueNotifier is later destroyed, there are no weak ptrs left to invalidate. R=vmpstr BUG=428347 Review URL: https://codereview.chromium.org/692573002 Cr-Commit-Position: refs/heads/master@{#301881}
Diffstat (limited to 'cc/base')
-rw-r--r--cc/base/delayed_unique_notifier.cc9
-rw-r--r--cc/base/delayed_unique_notifier.h5
-rw-r--r--cc/base/delayed_unique_notifier_unittest.cc69
3 files changed, 83 insertions, 0 deletions
diff --git a/cc/base/delayed_unique_notifier.cc b/cc/base/delayed_unique_notifier.cc
index c297267..1824226 100644
--- a/cc/base/delayed_unique_notifier.cc
+++ b/cc/base/delayed_unique_notifier.cc
@@ -43,6 +43,15 @@ void DelayedUniqueNotifier::Cancel() {
next_notification_time_ = base::TimeTicks();
}
+void DelayedUniqueNotifier::Shutdown() {
+ // This function must destroy any weak ptrs since after being cancelled, this
+ // class may be destroyed on another thread during compositor shutdown.
+ weak_ptr_factory_.InvalidateWeakPtrs();
+ // Deliberately leaves notification_pending_ = true forever so new tasks with
+ // weak ptrs can not be created.
+ notification_pending_ = true;
+}
+
bool DelayedUniqueNotifier::HasPendingNotification() const {
return notification_pending_ && !next_notification_time_.is_null();
}
diff --git a/cc/base/delayed_unique_notifier.h b/cc/base/delayed_unique_notifier.h
index 59d906b..02051be 100644
--- a/cc/base/delayed_unique_notifier.h
+++ b/cc/base/delayed_unique_notifier.h
@@ -37,6 +37,11 @@ class CC_EXPORT DelayedUniqueNotifier {
// Cancel any previously scheduled runs.
void Cancel();
+ // Cancel previously scheduled runs and prevent any new runs from starting.
+ // After calling this the DelayedUniqueNotifier will have no outstanding
+ // WeakPtrs.
+ void Shutdown();
+
// Returns true if a notification is currently scheduled to run.
bool HasPendingNotification() const;
diff --git a/cc/base/delayed_unique_notifier_unittest.cc b/cc/base/delayed_unique_notifier_unittest.cc
index 41cde83..3e41d32 100644
--- a/cc/base/delayed_unique_notifier_unittest.cc
+++ b/cc/base/delayed_unique_notifier_unittest.cc
@@ -262,5 +262,74 @@ TEST_F(DelayedUniqueNotifierTest, CancelAndHasPendingNotification) {
EXPECT_FALSE(notifier.HasPendingNotification());
}
+TEST_F(DelayedUniqueNotifierTest, ShutdownWithScheduledTask) {
+ base::TimeDelta delay = base::TimeDelta::FromInternalValue(20);
+ TestNotifier notifier(
+ task_runner_.get(),
+ base::Bind(&DelayedUniqueNotifierTest::Notify, base::Unretained(this)),
+ delay);
+
+ EXPECT_EQ(0, NotificationCount());
+
+ // Schedule for |delay| seconds from now.
+ base::TimeTicks schedule_time =
+ notifier.Now() + base::TimeDelta::FromInternalValue(10);
+ notifier.SetNow(schedule_time);
+ notifier.Schedule();
+ EXPECT_TRUE(notifier.HasPendingNotification());
+
+ // Shutdown the notifier.
+ notifier.Shutdown();
+
+ // The task is still there, but...
+ std::deque<base::TestPendingTask> tasks = TakePendingTasks();
+ ASSERT_EQ(1u, tasks.size());
+
+ // Running the task after shutdown does nothing since it's cancelled.
+ tasks[0].task.Run();
+ EXPECT_EQ(0, NotificationCount());
+
+ tasks = TakePendingTasks();
+ EXPECT_EQ(0u, tasks.size());
+
+ // We are no longer able to schedule tasks.
+ notifier.Schedule();
+ tasks = TakePendingTasks();
+ ASSERT_EQ(0u, tasks.size());
+
+ // Verify after the scheduled time happens there is still no task.
+ notifier.SetNow(notifier.Now() + delay);
+ tasks = TakePendingTasks();
+ ASSERT_EQ(0u, tasks.size());
+}
+
+TEST_F(DelayedUniqueNotifierTest, ShutdownPreventsSchedule) {
+ base::TimeDelta delay = base::TimeDelta::FromInternalValue(20);
+ TestNotifier notifier(
+ task_runner_.get(),
+ base::Bind(&DelayedUniqueNotifierTest::Notify, base::Unretained(this)),
+ delay);
+
+ EXPECT_EQ(0, NotificationCount());
+
+ // Schedule for |delay| seconds from now.
+ base::TimeTicks schedule_time =
+ notifier.Now() + base::TimeDelta::FromInternalValue(10);
+ notifier.SetNow(schedule_time);
+
+ // Shutdown the notifier.
+ notifier.Shutdown();
+
+ // Scheduling a task no longer does anything.
+ notifier.Schedule();
+ std::deque<base::TestPendingTask> tasks = TakePendingTasks();
+ ASSERT_EQ(0u, tasks.size());
+
+ // Verify after the scheduled time happens there is still no task.
+ notifier.SetNow(notifier.Now() + delay);
+ tasks = TakePendingTasks();
+ ASSERT_EQ(0u, tasks.size());
+}
+
} // namespace
} // namespace cc