Defer ServiceWorker update checks until after browser startup is comp…

…lete. Also implement after startup task deferral more reasonably on android. Now its based on a signal from DeferredStartupHandler.java instead of an arbitrary 10 second timer. BUG=460265 Review URL: https://codereview.chromium.org/1078283002 Cr-Commit-Position: refs/heads/master@{#342768}
Denger-Network · Aug 11, 2015 · 68bf4a8 · 68bf4a8
1 parent 698215b
commit 68bf4a8
Show file tree

Hide file tree

Showing 12 changed files with 98 additions and 4 deletions.
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/AfterStartupTaskUtils.java b/chrome/android/java/src/org/chromium/chrome/browser/AfterStartupTaskUtils.java
@@ -0,0 +1,23 @@
+// Copyright 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.
+
+package org.chromium.chrome.browser;
+
+/**
+ * JNI call glue for AfterStartupTaskUtils in C++.
+ */
+public final class AfterStartupTaskUtils {
+    private AfterStartupTaskUtils() {}
+
+    /**
+     * Informs the C++ side that startup is complete. Tasks that
+     * have been deferred until after startup will be scheduled
+     * to run and newly posted tasks will no longer be deferred.
+     */
+    public static void setStartupComplete() {
+        nativeSetStartupComplete();
+    }
+
+    private static native void nativeSetStartupComplete();
+}
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/DeferredStartupHandler.java b/chrome/android/java/src/org/chromium/chrome/browser/DeferredStartupHandler.java
@@ -101,6 +101,8 @@ protected Void doInBackground(Void... params) {
             }
         }.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR);
 
+        AfterStartupTaskUtils.setStartupComplete();
+
         // TODO(aruslan): http://b/6397072 This will be moved elsewhere
         PartnerBookmarksShim.kickOffReading(application);
 

diff --git a/chrome/browser/after_startup_task_utils.cc b/chrome/browser/after_startup_task_utils.cc
@@ -174,10 +174,10 @@ void StartupObserver::Start() {
     delay = base::TimeDelta::FromMinutes(kLongerDelayMins);
   }
 #else
-  // TODO(michaeln): We should probably monitor the initial page load here too,
-  // but since ChromeBrowserMainExtraPartsMetrics doesn't, not doing that yet.
-  const int kAndroidDelaySecs = 10;
-  delay = base::TimeDelta::FromSeconds(kAndroidDelaySecs);
+  // Startup completion is signaled via AfterStartupTaskUtils.java,
+  // this is just a failsafe timeout.
+  const int kLongerDelayMins = 3;
+  delay = base::TimeDelta::FromMinutes(kLongerDelayMins);
 #endif  // !defined(OS_ANDROID)
 
   BrowserThread::PostDelayedTask(BrowserThread::UI, FROM_HERE,

diff --git a/chrome/browser/after_startup_task_utils.h b/chrome/browser/after_startup_task_utils.h
@@ -10,6 +10,10 @@
 #include "base/gtest_prod_util.h"
 #include "base/memory/ref_counted.h"
 
+namespace android {
+class AfterStartupTaskUtilsJNI;
+}
+
 namespace base {
 class TaskRunner;
 }
@@ -36,6 +40,8 @@ class AfterStartupTaskUtils {
 
  private:
   friend class AfterStartupTaskTest;
+  friend class android::AfterStartupTaskUtilsJNI;
+  friend class InProcessBrowserTest;
   FRIEND_TEST_ALL_PREFIXES(AfterStartupTaskTest, IsStartupComplete);
   FRIEND_TEST_ALL_PREFIXES(AfterStartupTaskTest, PostTask);
 

diff --git a/chrome/browser/after_startup_task_utils_android.cc b/chrome/browser/after_startup_task_utils_android.cc
@@ -0,0 +1,27 @@
+// Copyright 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_utils_android.h"
+
+#include "chrome/browser/after_startup_task_utils.h"
+#include "jni/AfterStartupTaskUtils_jni.h"
+
+namespace android {
+
+class AfterStartupTaskUtilsJNI {
+ public:
+  static void SetBrowserStartupIsComplete() {
+    AfterStartupTaskUtils::SetBrowserStartupIsComplete();
+  }
+};
+
+}  // android
+
+static void SetStartupComplete(JNIEnv* env, jclass obj) {
+  android::AfterStartupTaskUtilsJNI::SetBrowserStartupIsComplete();
+}
+
+bool RegisterAfterStartupTaskUtilsJNI(JNIEnv* env) {
+  return RegisterNativesImpl(env);
+}
diff --git a/chrome/browser/after_startup_task_utils_android.h b/chrome/browser/after_startup_task_utils_android.h
@@ -0,0 +1,12 @@
+// Copyright 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.
+
+#ifndef CHROME_BROWSER_AFTER_STARTUP_TASK_UTILS_ANDROID_H_
+#define CHROME_BROWSER_AFTER_STARTUP_TASK_UTILS_ANDROID_H_
+
+#include <jni.h>
+
+bool RegisterAfterStartupTaskUtilsJNI(JNIEnv* env);
+
+#endif  // CHROME_BROWSER_AFTER_STARTUP_TASK_UTILS_ANDROID_H_
diff --git a/chrome/browser/android/chrome_jni_registrar.cc b/chrome/browser/android/chrome_jni_registrar.cc
@@ -7,6 +7,7 @@
 #include "base/android/jni_android.h"
 #include "base/android/jni_registrar.h"
 #include "base/trace_event/trace_event.h"
+#include "chrome/browser/after_startup_task_utils_android.h"
 #include "chrome/browser/android/accessibility/font_size_prefs_android.h"
 #include "chrome/browser/android/accessibility_util.h"
 #include "chrome/browser/android/appmenu/app_menu_drag_helper.h"
@@ -168,6 +169,7 @@ static base::android::RegistrationMethod kChromeRegisteredMethods[] = {
     {"AccessibilityUtils", AccessibilityUtil::Register},
     {"AccountChooserInfoBar", RegisterAccountChooserInfoBar},
     {"AccountManagementScreenHelper", AccountManagementScreenHelper::Register},
+    {"AfterStartupTaskUtils", RegisterAfterStartupTaskUtilsJNI},
     {"AnswersImageBridge", RegisterAnswersImageBridge},
     {"AppBannerInfoBarAndroid", RegisterAppBannerInfoBarAndroid},
     {"AppBannerInfoBarDelegateAndroid",

diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi
@@ -22,6 +22,8 @@
       'browser/about_flags.h',
       'browser/after_startup_task_utils.cc',
       'browser/after_startup_task_utils.h',
+      'browser/after_startup_task_utils_android.cc',
+      'browser/after_startup_task_utils_android.h',
       'browser/android/accessibility/font_size_prefs_android.cc',
       'browser/android/accessibility/font_size_prefs_android.h',
       'browser/android/accessibility_util.cc',
@@ -1681,6 +1683,7 @@
     ],
     'chrome_browser_jni_sources': [
       'android/java/src/org/chromium/chrome/browser/AccessibilityUtil.java',
+      'android/java/src/org/chromium/chrome/browser/AfterStartupTaskUtils.java',
       'android/java/src/org/chromium/chrome/browser/ApplicationLifetime.java',
       'android/java/src/org/chromium/chrome/browser/accessibility/FontSizePrefs.java',
       'android/java/src/org/chromium/chrome/browser/appmenu/AppMenuDragHelper.java',

diff --git a/chrome/test/base/in_process_browser_test.cc b/chrome/test/base/in_process_browser_test.cc
@@ -18,6 +18,7 @@
 #include "base/test/test_file_util.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/threading/non_thread_safe.h"
+#include "chrome/browser/after_startup_task_utils.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/devtools/devtools_window.h"
@@ -537,6 +538,8 @@ base::CommandLine InProcessBrowserTest::GetCommandLineForRelaunch() {
 #endif
 
 void InProcessBrowserTest::RunTestOnMainThreadLoop() {
+  AfterStartupTaskUtils::SetBrowserStartupIsComplete();
+
   // Pump startup related events.
   content::RunAllPendingInMessageLoop();
 

diff --git a/chrome/test/base/in_process_browser_test_browsertest.cc b/chrome/test/base/in_process_browser_test_browsertest.cc
@@ -6,6 +6,7 @@
 
 #include "base/files/file_util.h"
 #include "base/path_service.h"
+#include "chrome/browser/after_startup_task_utils.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/test/base/in_process_browser_test.h"
@@ -84,6 +85,12 @@ IN_PROC_BROWSER_TEST_F(InProcessBrowserTest, ExternalConnectionFail) {
   }
 }
 
+// Verify that AfterStartupTaskUtils considers startup to be complete
+// prior to test execution so tasks posted by tests are never deferred.
+IN_PROC_BROWSER_TEST_F(InProcessBrowserTest, AfterStartupTaskUtils) {
+  EXPECT_TRUE(AfterStartupTaskUtils::IsBrowserStartupComplete());
+}
+
 // Paths are to very simple HTML files. One is accessible, the other is not.
 const base::FilePath::CharType kPassHTML[] =
     FILE_PATH_LITERAL("chrome/test/data/accessibility_pass.html");

diff --git a/content/browser/service_worker/service_worker_register_job.cc b/content/browser/service_worker/service_worker_register_job.cc
@@ -16,6 +16,7 @@
 #include "content/browser/service_worker/service_worker_storage.h"
 #include "content/common/service_worker/service_worker_types.h"
 #include "content/common/service_worker/service_worker_utils.h"
+#include "content/public/browser/browser_thread.h"
 #include "net/base/net_errors.h"
 
 namespace content {
@@ -86,6 +87,13 @@ void ServiceWorkerRegisterJob::AddCallback(
 }
 
 void ServiceWorkerRegisterJob::Start() {
+  BrowserThread::PostAfterStartupTask(
+      FROM_HERE, base::ThreadTaskRunnerHandle::Get(),
+      base::Bind(&ServiceWorkerRegisterJob::StartImpl,
+                 weak_factory_.GetWeakPtr()));
+}
+
+void ServiceWorkerRegisterJob::StartImpl() {
   SetPhase(START);
   ServiceWorkerStorage::FindRegistrationCallback next_step;
   if (job_type_ == REGISTRATION_JOB) {

diff --git a/content/browser/service_worker/service_worker_register_job.h b/content/browser/service_worker/service_worker_register_job.h
@@ -105,6 +105,7 @@ class ServiceWorkerRegisterJob : public ServiceWorkerRegisterJobBase {
 
   void SetPhase(Phase phase);
 
+  void StartImpl();
   void ContinueWithRegistration(
       ServiceWorkerStatusCode status,
       const scoped_refptr<ServiceWorkerRegistration>& registration);