[component updater] Fix race condition in background task scheduler

If the background task scheduler started Chrome it could happen that we
wanted to schedule a component update before the update callback was
set. This caused seg faults. Instead, add a delay to give enough time
for all components to finish registering and set the update callback. If
there is still no update callback set finish the task and retry after a
default delay.

Bug: 867354, 876424
Change-Id: I3ef8277a4e45384531a08652d5534831f8fd2056
Reviewed-on: https://chromium-review.googlesource.com/1183961Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: Sorin Jianu <sorin@chromium.org>
Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Commit-Queue: Tibor Goldschwendt <tiborg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#585177}

chrome/android/java/src/org/chromium/chrome/browser/component_updater/UpdateScheduler.java

@@ -20,10 +20,14 @@ import org.chromium.components.background_task_scheduler.TaskInfo;
 /** Java-side implementation of the component update scheduler using the BackgroundTaskScheduler. */
 @JNINamespace("component_updater")
 public class UpdateScheduler {
+    // This is the delay in case we have to reschedule before the component updater could schedule
+    // component updates.
+    private static final int DEFAULT_DELAY_MS = 600000; // 10 minutes.
+
     private static UpdateScheduler sInstance;
     private TaskFinishedCallback mTaskFinishedCallback;
     private long mNativeScheduler;
-    private long mDelayMs;
+    private long mDelayMs = DEFAULT_DELAY_MS;
 
     @CalledByNative
     /* package */ static UpdateScheduler getInstance() {

chrome/browser/android/component_updater/background_task_update_scheduler.cc

@@ -4,10 +4,19 @@
 
 #include "chrome/browser/android/component_updater/background_task_update_scheduler.h"
 
+#include "base/threading/sequenced_task_runner_handle.h"
 #include "jni/UpdateScheduler_jni.h"
 
 namespace component_updater {
 
+namespace {
+
+// Delay of running component updates after the background task fires to give
+// enough time for async component registration.
+const base::TimeDelta kOnStartTaskDelay = base::TimeDelta::FromSeconds(2);
+
+}  // namespace
+
 // static
 bool BackgroundTaskUpdateScheduler::IsAvailable() {
   return Java_UpdateScheduler_isAvailable(base::android::AttachCurrentThread());
@@ -43,8 +52,13 @@ void BackgroundTaskUpdateScheduler::Stop() {
 void BackgroundTaskUpdateScheduler::OnStartTask(
     JNIEnv* env,
     const base::android::JavaParamRef<jobject>& obj) {
-  user_task_.Run(base::BindOnce(&Java_UpdateScheduler_finishTask,
-                                base::Unretained(env), j_update_scheduler_));
+  // Component registration is async. Add some delay to give some time for the
+  // registration.
+  base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE,
+      base::BindOnce(&BackgroundTaskUpdateScheduler::OnStartTaskDelayed,
+                     base::Unretained(this)),
+      kOnStartTaskDelay);
 }
 
 void BackgroundTaskUpdateScheduler::OnStopTask(
@@ -54,4 +68,15 @@ void BackgroundTaskUpdateScheduler::OnStopTask(
   on_stop_.Run();
 }
 
+void BackgroundTaskUpdateScheduler::OnStartTaskDelayed() {
+  JNIEnv* env = base::android::AttachCurrentThread();
+  if (!user_task_) {
+    LOG(WARNING) << "No components registered to update";
+    Java_UpdateScheduler_finishTask(env, j_update_scheduler_);
+    return;
+  }
+  user_task_.Run(base::BindOnce(&Java_UpdateScheduler_finishTask,
+                                base::Unretained(env), j_update_scheduler_));
+}
+
 }  // namespace component_updater

chrome/browser/android/component_updater/background_task_update_scheduler.h

@@ -37,6 +37,8 @@ class BackgroundTaskUpdateScheduler : public UpdateScheduler {
   void OnStopTask(JNIEnv* env, const base::android::JavaParamRef<jobject>& obj);
 
  private:
+  void OnStartTaskDelayed();
+
   base::android::ScopedJavaGlobalRef<jobject> j_update_scheduler_;
   UserTask user_task_;
   OnStopTaskCallback on_stop_;