Implement a poor man's PostAfterStartupTask() function.

chrome/browser/after_startup_task.cc

+// Copyright (c) 2015 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 "chrome/browser/after_startup_task.h"
+
+#include "base/lazy_instance.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/metrics/histogram_macros.h"
+#include "base/process/process_info.h"
+#include "base/rand_util.h"
+#include "base/synchronization/cancellation_flag.h"
+#include "base/task_runner.h"
+#include "base/tracked_objects.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_iterator.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_observer.h"
+
+using content::BrowserThread;
+using content::WebContents;
+using content::WebContentsObserver;
+using StartupCompleteFlag = base::CancellationFlag;
+
+namespace {
+
+struct AfterStartupTask {
+  AfterStartupTask(const tracked_objects::Location& from_here,
+                   const scoped_refptr<base::TaskRunner>& task_runner,
+                   const base::Closure& task)
+      : from_here(from_here), task_runner(task_runner), task(task) {}
+  ~AfterStartupTask() {}
+
+  const tracked_objects::Location from_here;
+  const scoped_refptr<base::TaskRunner> task_runner;
+  const base::Closure task;
+};
+
+class StartupObserver;
+void SetBrowserStartupIsComplete();
+bool IsBrowserStartupComplete();
+void QueueTask(scoped_ptr<AfterStartupTask> queued_task);
+void ScheduleTask(scoped_ptr<AfterStartupTask> queued_task);
+void RunTask(scoped_ptr<AfterStartupTask> queued_task);

[gab, 2015/03/30 18:29:37]

Fwd-declaring is quite atypical in Chromium, can you avoid this?

[michaeln, 2015/04/03 19:51:38]

Done

+
+base::LazyInstance<StartupCompleteFlag>::Leaky g_startup_complete_flag;
+base::LazyInstance<std::deque<AfterStartupTask*>>::Leaky g_after_startup_tasks;

[gab, 2015/03/30 18:29:37]

Add a comment that this should only be accessed on the UI thread.

[michaeln, 2015/04/03 19:51:38]

Done.

+
+void SetBrowserStartupIsComplete() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

[gab, 2015/03/30 18:29:36]

Use DCHECK_CURRENTLY_ON.

[michaeln, 2015/04/03 19:51:38]

Done.

+#if defined(OS_MACOSX) || defined(OS_WIN) || defined(OS_LINUX)
+  // CurrentProcessInfo::CreationTime() is not available on all platforms.
+  const base::Time process_creation_time =
+      base::CurrentProcessInfo::CreationTime();
+  if (!process_creation_time.is_null()) {
+    UMA_HISTOGRAM_LONG_TIMES("AfterStartupTasks.RunTime",
+                             base::Time::Now() - process_creation_time);
+  }
+#endif  // defined(OS_MACOSX) || defined(OS_WIN) || defined(OS_LINUX)
+  UMA_HISTOGRAM_COUNTS_100("AfterStartupTasks.Count",

[gab, 2015/03/30 18:29:36]

I can imagine this growing beyond 100.

[michaeln, 2015/04/03 19:51:38]

Done, how about 10000

+                           g_after_startup_tasks.Get().size());
+  g_startup_complete_flag.Get().Set();
+  for (AfterStartupTask* queued_task : g_after_startup_tasks.Get())
+    ScheduleTask(make_scoped_ptr(queued_task));
+  g_after_startup_tasks.Get().clear();
+
+  // poorman's shrink_to_fit()
+  std::deque<AfterStartupTask*>(g_after_startup_tasks.Get()).swap(
+      g_after_startup_tasks.Get());
+}
+
+bool IsBrowserStartupComplete() {
+  // Be sure to allocate the flag on the main thread.
+  if (g_startup_complete_flag == nullptr)
+    return false;
+  return g_startup_complete_flag.Get().IsSet();
+}
+
+void QueueTask(scoped_ptr<AfterStartupTask> queued_task) {
+  if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        base::Bind(QueueTask, base::Passed(queued_task.Pass())));
+    return;
+  }
+  if (IsBrowserStartupComplete()) {

[gab, 2015/03/30 18:29:37]

Please add a comment as to why checking this is necessary.

Also, I think it would make more sense to check this first in this method (i.e.,
escape hatch at top and then have the remainder of the logic be about
"queuing").

[michaeln, 2015/04/03 19:51:38]

Done.
I'd end up testing IsBrowserStartupComplete() on background threads twice to do
that. I'd like to avoid heap allocating the AfterStartupTask unless its needed.

+    ScheduleTask(queued_task.Pass());
+    return;
+  }
+  g_after_startup_tasks.Get().push_back(queued_task.release());
+}
+
+void ScheduleTask(scoped_ptr<AfterStartupTask> queued_task) {
+  // Spread their execution over a brief time.
+  const int kMinDelay = 0;
+  const int kMaxDelay = 10;
+  scoped_refptr<base::TaskRunner> target_runner = queued_task->task_runner;
+  tracked_objects::Location from_here = queued_task->from_here;
+  target_runner->PostDelayedTask(
+      from_here, base::Bind(&RunTask, base::Passed(queued_task.Pass())),
+      base::TimeDelta::FromSeconds(base::RandInt(kMinDelay, kMaxDelay)));
+}
+
+void RunTask(scoped_ptr<AfterStartupTask> queued_task) {
+  // We're careful to delete the caller's |task| on the target runner's thread.
+  DCHECK(queued_task->task_runner->RunsTasksOnCurrentThread());
+  queued_task->task.Run();
+}
+
+// Observes the first page load and set the startup complete flag accordingly.

[gab, 2015/03/30 18:29:36]

I would add "visible" to that (i.e., "first visibile page load").

You can get that below with |render_frame_host->GetVisibilityState() ==
blink::WebPageVisibilityStateVisible| I think.

[michaeln, 2015/04/03 19:51:38]

Done, should work well with background mode launches.

+class StartupObserver : public WebContentsObserver {

[gab, 2015/03/30 18:29:36]

I'd feel safer (and it would be self-documenting code) if we marked this class
NonThreadSafe and dchecked CalledOnValidThread() in every method since this
entire files deals with multi-threads a lot while this specific class is not
intended to be thread-safe.

[michaeln, 2015/04/03 19:51:38]

Done, except only put the dcheck in OnStartupComplete() which all methods call.

+ public:
+  StartupObserver() : weak_factory_(this) {}
+  ~StartupObserver() override { DCHECK(IsBrowserStartupComplete()); }
+
+  void Start();
+
+ private:
+  void OnStartupComplete() {
+    SetBrowserStartupIsComplete();
+    delete this;
+  }
+
+  void OnFailsafeTimeout() { OnStartupComplete(); }
+
+  // WebContentsObserver overrides
+  void DidFinishLoad(content::RenderFrameHost* render_frame_host,
+                     const GURL& validated_url) override {
+    if (!render_frame_host->GetParent())
+      OnStartupComplete();
+  }
+
+  void DidFailLoad(content::RenderFrameHost* render_frame_host,
+                   const GURL& validated_url,
+                   int error_code,
+                   const base::string16& error_description) override {
+    if (!render_frame_host->GetParent())
+      OnStartupComplete();
+  }
+
+  void WebContentsDestroyed() override { OnStartupComplete(); }
+
+  base::WeakPtrFactory<StartupObserver> weak_factory_;
+
+  DISALLOW_COPY_AND_ASSIGN(StartupObserver);
+};
+
+void StartupObserver::Start() {
+  // Signal completion quickly when there is no first page to load.
+  const int kShortDelaySecs = 3;
+  base::TimeDelta delay = base::TimeDelta::FromSeconds(kShortDelaySecs);
+
+#if !defined(OS_ANDROID)
+  WebContents* contents = nullptr;
+  for (chrome::BrowserIterator iter; !iter.done(); iter.Next()) {
+    contents = (*iter)->tab_strip_model()->GetActiveWebContents();
+    if (contents)
+      break;
+  }
+
+  if (contents) {
+    // Give the page time to finish loading.
+    const int kLongerDelayMins = 3;
+    Observe(contents);
+    delay = base::TimeDelta::FromMinutes(kLongerDelayMins);
+  }
+#endif  // !defined(OS_ANDROID)
+
+  BrowserThread::PostDelayedTask(BrowserThread::UI, FROM_HERE,
+                                 base::Bind(&StartupObserver::OnFailsafeTimeout,
+                                            weak_factory_.GetWeakPtr()),
+                                 delay);
+}
+
+}  // namespace
+
+void StartMonitoringStartup() {
+  // The observer is self-deleting.
+  (new StartupObserver)->Start();

[gab, 2015/03/30 18:29:37]

This is a little weird, should we make the constructor private and have a
StartupObserver::BeginObserving() static method on that class?

[michaeln, 2015/04/03 19:51:38]

You mean a static method with this body :) I originally had it that way and
decided to remove the extra function call given how single purpose this .cc file
is.

This is done plenty places elsewhere too, a few examples...

  // AppLaunchManager manages its own lifetime.
  (new AppLaunchManager(profile, app_id))->Start();

  // IconLoader deletes itself when done.
  (new IconLoader(AsWeakPtr(), icon_path_))->Start();

  // ChromeRestartRequest deletes itself after request sent to session manager.
  (new ChromeRestartRequest(command_line))->Start();

+}
+
+void PostAfterStartupTask(const tracked_objects::Location& from_here,
+                          const scoped_refptr<base::TaskRunner>& task_runner,
+                          const base::Closure& task) {
+  if (IsBrowserStartupComplete()) {
+    task_runner->PostTask(from_here, task);
+    return;
+  }
+
+  scoped_ptr<AfterStartupTask> queued_task(
+      new AfterStartupTask(from_here, task_runner, task));
+  QueueTask(queued_task.Pass());
+}