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

Unified Diff: Source/core/workers/WorkerThread.cpp

Issue 563203002: [Blink-WebWorkers] WorkerSharedTimer cancels extra delayed tasks. (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@master
Patch Set: Changes to create cancelable task. Removed weak_ptr. Created 6 years, 3 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
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: Source/core/workers/WorkerThread.cpp
diff --git a/Source/core/workers/WorkerThread.cpp b/Source/core/workers/WorkerThread.cpp
index 988256e367b9a256b1245ff2fdac409fb9aa786d..e5fe43132ef59d5e9d4b651d10fbe30a46698b6e 100644
--- a/Source/core/workers/WorkerThread.cpp
+++ b/Source/core/workers/WorkerThread.cpp
@@ -85,6 +85,74 @@ unsigned WorkerThread::workerThreadCount()
return workerThreads().size();
}
+class WorkerThreadTask : public blink::WebThread::Task {
jochen (gone - plz use gerrit) 2014/09/15 14:29:41 why do you move this class up here?
Mayur Kankanwadi 2014/09/15 14:38:27 Was done to fix earlier compilation errors. Fixed
+ WTF_MAKE_NONCOPYABLE(WorkerThreadTask); WTF_MAKE_FAST_ALLOCATED;
+public:
+ static PassOwnPtr<WorkerThreadTask> create(const WorkerThread& workerThread, PassOwnPtr<ExecutionContextTask> task, bool isInstrumented)
+ {
+ return adoptPtr(new WorkerThreadTask(workerThread, task, isInstrumented));
+ }
+
+ virtual ~WorkerThreadTask() { }
+
+ virtual void run() OVERRIDE
+ {
+ WorkerGlobalScope* workerGlobalScope = m_workerThread.workerGlobalScope();
+ // Tasks could be put on the message loop after the cleanup task,
+ // ensure none of those are ran.
+ if (!workerGlobalScope)
+ return;
+
+ if (m_isInstrumented)
+ InspectorInstrumentation::willPerformExecutionContextTask(workerGlobalScope, m_task.get());
+ if ((!workerGlobalScope->isClosing() && !m_workerThread.terminated()) || m_task->isCleanupTask())
+ m_task->performTask(workerGlobalScope);
+ if (m_isInstrumented)
+ InspectorInstrumentation::didPerformExecutionContextTask(workerGlobalScope);
+ }
+
+private:
+ WorkerThreadTask(const WorkerThread& workerThread, PassOwnPtr<ExecutionContextTask> task, bool isInstrumented)
+ : m_workerThread(workerThread)
+ , m_task(task)
+ , m_isInstrumented(isInstrumented)
+ {
+ if (m_isInstrumented)
+ m_isInstrumented = !m_task->taskNameForInstrumentation().isEmpty();
+ if (m_isInstrumented)
+ InspectorInstrumentation::didPostExecutionContextTask(m_workerThread.workerGlobalScope(), m_task.get());
+ }
+
+ const WorkerThread& m_workerThread;
+ OwnPtr<ExecutionContextTask> m_task;
+ bool m_isInstrumented;
+};
+
+class WorkerThreadCancelableTask FINAL : public ExecutionContextTask {
jochen (gone - plz use gerrit) 2014/09/15 14:29:42 add wtf_make_noncopyable and fast allocated macros
Mayur Kankanwadi 2014/09/15 14:38:27 Done.
+public:
+ static PassOwnPtr<WorkerThreadCancelableTask> create(const Closure& closure)
+ {
+ return adoptPtr(new WorkerThreadCancelableTask(closure));
+ }
+
+ virtual void performTask(ExecutionContext*) OVERRIDE
+ {
+ if (!m_taskCanceled)
+ m_closure();
+ }
+
+ void cancelTask() { m_taskCanceled = true; }
+
+private:
+ explicit WorkerThreadCancelableTask(const Closure& closure)
+ : m_closure(closure)
+ , m_taskCanceled(false)
+ { }
+
+ Closure m_closure;
+ bool m_taskCanceled;
+};
+
class WorkerSharedTimer : public SharedTimer {
public:
explicit WorkerSharedTimer(WorkerThread* workerThread)
@@ -116,12 +184,28 @@ public:
m_running = true;
m_nextFireTime = currentTime() + interval;
- m_workerThread->postDelayedTask(createSameThreadTask(&WorkerSharedTimer::OnTimeout, this), delay);
+ if (m_lastQueuedTask) {
+ // If something was queued up before this, check if its later than the current timer
+ if (interval < m_lastQueuedTaskTimer->nextFireInterval()) {
+ m_lastQueuedTask->cancelTask();
+ m_lastQueuedTask = 0;
+ m_lastQueuedTaskTimer = 0;
+ }
+ }
+ if (!m_lastQueuedTask) {
+ // Now queue the task as a cancellable one.
+ m_lastQueuedTask = WorkerThreadCancelableTask::create(bind(&WorkerSharedTimer::OnTimeout, this)).leakPtr();
+ m_workerThread->postDelayedTask(adoptPtr(m_lastQueuedTask), delay);
+ // The current timer should ideally be at the top of the heap as its assumed to be the smallest interval in the heap
+ m_lastQueuedTaskTimer = PlatformThreadData::current().threadTimers().timerHeap().first();
+ }
}
virtual void stop()
{
m_running = false;
+ m_lastQueuedTask = 0;
+ m_lastQueuedTaskTimer = 0;
}
double nextFireTime() { return m_nextFireTime; }
@@ -130,6 +214,10 @@ private:
void OnTimeout()
{
ASSERT(m_workerThread->workerGlobalScope());
+
+ m_lastQueuedTask = 0;
+ m_lastQueuedTaskTimer = 0;
+
if (m_sharedTimerFunction && m_running && !m_workerThread->workerGlobalScope()->isClosing())
m_sharedTimerFunction();
}
@@ -138,50 +226,12 @@ private:
SharedTimerFunction m_sharedTimerFunction;
double m_nextFireTime;
bool m_running;
+ static WorkerThreadCancelableTask* m_lastQueuedTask;
jochen (gone - plz use gerrit) 2014/09/15 14:29:42 why static? there can be several worker threads
Mayur Kankanwadi 2014/09/15 14:38:27 Yes, it's unnecessary. Acknowledged.
+ static TimerBase* m_lastQueuedTaskTimer;
};
-class WorkerThreadTask : public blink::WebThread::Task {
- WTF_MAKE_NONCOPYABLE(WorkerThreadTask); WTF_MAKE_FAST_ALLOCATED;
-public:
- static PassOwnPtr<WorkerThreadTask> create(const WorkerThread& workerThread, PassOwnPtr<ExecutionContextTask> task, bool isInstrumented)
- {
- return adoptPtr(new WorkerThreadTask(workerThread, task, isInstrumented));
- }
-
- virtual ~WorkerThreadTask() { }
-
- virtual void run() OVERRIDE
- {
- WorkerGlobalScope* workerGlobalScope = m_workerThread.workerGlobalScope();
- // Tasks could be put on the message loop after the cleanup task,
- // ensure none of those are ran.
- if (!workerGlobalScope)
- return;
-
- if (m_isInstrumented)
- InspectorInstrumentation::willPerformExecutionContextTask(workerGlobalScope, m_task.get());
- if ((!workerGlobalScope->isClosing() && !m_workerThread.terminated()) || m_task->isCleanupTask())
- m_task->performTask(workerGlobalScope);
- if (m_isInstrumented)
- InspectorInstrumentation::didPerformExecutionContextTask(workerGlobalScope);
- }
-
-private:
- WorkerThreadTask(const WorkerThread& workerThread, PassOwnPtr<ExecutionContextTask> task, bool isInstrumented)
- : m_workerThread(workerThread)
- , m_task(task)
- , m_isInstrumented(isInstrumented)
- {
- if (m_isInstrumented)
- m_isInstrumented = !m_task->taskNameForInstrumentation().isEmpty();
- if (m_isInstrumented)
- InspectorInstrumentation::didPostExecutionContextTask(m_workerThread.workerGlobalScope(), m_task.get());
- }
-
- const WorkerThread& m_workerThread;
- OwnPtr<ExecutionContextTask> m_task;
- bool m_isInstrumented;
-};
+WorkerThreadCancelableTask* WorkerSharedTimer::m_lastQueuedTask = 0;
+TimerBase* WorkerSharedTimer::m_lastQueuedTaskTimer = 0;
class RunDebuggerQueueTask FINAL : public ExecutionContextTask {
public:
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698