Delay bookmarks load while the profile is loading.

base/deferred_sequenced_task_runner.cc

Index: base/deferred_sequenced_task_runner.cc
diff --git a/base/deferred_sequenced_task_runner.cc b/base/deferred_sequenced_task_runner.cc
new file mode 100644
index 0000000000000000000000000000000000000000..9661178bd894d159a4b472029b03aeba416c02bd
--- /dev/null
+++ b/base/deferred_sequenced_task_runner.cc
@@ -0,0 +1,93 @@
+// Copyright (c) 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/deferred_sequenced_task_runner.h"
+
+#include "base/bind.h"
+#include "base/threading/thread_checker.h"
+
+namespace base {
+
+DeferredSequencedTaskRunner::DeferredTask::DeferredTask() {
+}
+
+DeferredSequencedTaskRunner::DeferredTask::~DeferredTask() {
+}
+
+DeferredSequencedTaskRunner::DeferredSequencedTaskRunner(
+    const scoped_refptr<SequencedTaskRunner>& target_task_runner)
+    : target_task_runner_(target_task_runner) {
+}
+
+DeferredSequencedTaskRunner::~DeferredSequencedTaskRunner() {
+}
+
+bool DeferredSequencedTaskRunner::PostDelayedTask(
+    const tracked_objects::Location& from_here,
+    const Closure& task,
+    TimeDelta delay) {
+  AutoLock lock(lock_);
+  if (started_) {
+    DCHECK(deferred_tasks_queue_.empty());
+    return target_task_runner_->PostDelayedTask(from_here, task, delay);
+  }
+
+  QueueDeferredTask(from_here, task, delay, false /* is_non_nestable */);
+  return true;
+}
+
+bool DeferredSequencedTaskRunner::RunsTasksOnCurrentThread() const {
+  return target_task_runner_->RunsTasksOnCurrentThread();
+}
+
+bool DeferredSequencedTaskRunner::PostNonNestableDelayedTask(
+    const tracked_objects::Location& from_here,
+    const Closure& task,
+    TimeDelta delay) {
+  AutoLock lock(lock_);
+  if (started_) {
+    DCHECK(deferred_tasks_queue_.empty());
+    return target_task_runner_->PostDelayedTask(from_here, task, delay);
+  }
+  QueueDeferredTask(from_here, task, delay, true /* is_non_nestable */);
+  return true;
+}
+
+void DeferredSequencedTaskRunner::QueueDeferredTask(
+    const tracked_objects::Location& from_here,
+    const Closure& task,
+    TimeDelta delay,
+    bool is_non_nestable) {
+  DeferredTask deferred_task;
+  deferred_task.posted_from = from_here;
+  deferred_task.task = task;
+  deferred_task.delay = delay;
+  deferred_task.is_non_nestable = true;
+  deferred_tasks_queue_.push_back(deferred_task);
+}
+
+
+void DeferredSequencedTaskRunner::Start() {
+  AutoLock lock(lock_);
+  if (started_)
+    return;
+
+  started_ = true;
+  for (std::vector<DeferredTask>::iterator i = deferred_tasks_queue_.begin();

[willchan no longer on Chromium, 2013/04/05 02:56:41]

There's a really subtle issue here that I'd like to draw your attention to so
you can evaluate if it's an issue. I believe it is.

You aren't freeing tasks immediately. I think it's a not-uncommon assumption
that when you PostTask() to a TaskRunner and don't keep the task around
elsewhere in your code, that after that any arguments bound to that task are
deleted.

Note here that you keep the tasks around longer, which prevents them from
getting deleted, since they're internally refcounted. Is that desired? I suspect
not.

If I'm correct and we should preserve the expectation that we do not
artificially extend task lifetime, then please add a test that asserts this.
Please upload a patchset with the new test with a tryjob demonstrating its
failure. Then upload a patchset with the fix that shows that the new test got
fixed.

[msarda, 2013/04/08 17:37:18]

I just want to be sure I understand your comment as it may refer to the
following cases:

1. I should remove the tasks from deferred_tasks_queue_ as soon as they are
posted.

2. The DeferredSequencedTaskRunner is wrong entirely as it keeps taks in a queue
until Start() is called, having as a consequence the fact that they live longer.
I have to keep the tasks around for a longer period of time as I need to run
them later. I don't really see how I can do otherwise. In any case, during
start-up there is much contention (at least on mobile), so the tasks should
expect to live longer, right?


So, what case are you actually talking about? If 1, then I'll try to put up the
test. If 2, then this CL has no meaning anymore as the tasks need to live until
they are posted, right?

[willchan no longer on Chromium, 2013/04/08 17:41:09]

(1) is what I was talking about.

+      i != deferred_tasks_queue_.end(); ++i) {
+    const DeferredTask& task = *i;
+    if (task.is_non_nestable) {
+      target_task_runner_->PostNonNestableDelayedTask(task.posted_from,
+                                                      task.task,
+                                                      task.delay);
+    } else {
+      target_task_runner_->PostDelayedTask(task.posted_from,
+                                           task.task,
+                                           task.delay);
+    }
+  }
+  deferred_tasks_queue_.clear();
+}
+
+}  // namespace base