Disable anti starvation logic on the UI and IO threads

We're seeing expensive low priority tasks getting in the way of
navigation tasks, so we should turn this logic off.  It's an
oversight that it was ever enabled on the UI and IO thread in the
first place.

This change has exposed a few latent issues:

1. BackgroundSyncTest is failing on the bots due to an infra issue
where Play Services is not up to date.

2. Several extension tests where failing due to unwanted TaskPriority
propagation for tasks posted from GetExtensionFileTaskRunner() to
the UI thread. These tasks where inheriting the sequence's
USER_VISIBLE priority and running much later than expected as a result
of removing the anti-starvation code. I've worked around that by
explicitly setting the priority to the default which is USER_BLOCKING.

3. Some navigations in the UrlOverridingTest are actually getting
canceled by the InterceptNavigationThrottle.  Previously the task
notifying this didn't run in time to be picked up by the test, now they
are we need to update the test expectations.

Bug: 1013535
Change-Id: Ie9e5bc38f62e8a451b9287390752d3bd1c3284c3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1863410
Commit-Queue: Alex Clarke <alexclarke@chromium.org>
Reviewed-by: Michael Thiessen <mthiesse@chromium.org>
Reviewed-by: Finnur Thorarinsson <finnur@chromium.org>
Reviewed-by: Rayan Kanso <rayankans@chromium.org>
Reviewed-by: Mugdha Lakhani <nator@chromium.org>
Reviewed-by: Sami Kyöstilä <skyostil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711195}

chrome/android/javatests/src/org/chromium/chrome/browser/background_sync/BackgroundSyncTest.java

@@ -28,6 +28,7 @@ import org.mockito.MockitoAnnotations;
 
 import org.chromium.base.ContextUtils;
 import org.chromium.base.test.util.CommandLineFlags;
+import org.chromium.base.test.util.DisabledTest;
 import org.chromium.base.test.util.Feature;
 import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.ChromeSwitches;
@@ -113,6 +114,7 @@ public final class BackgroundSyncTest {
     @Test
     @MediumTest
     @Feature({"BackgroundSync"})
+    @DisabledTest(message = "crbug.com/1015055")
     public void onSyncCalledWithNetworkConnectivity() throws Exception {
         forceConnectionType(ConnectionType.CONNECTION_NONE);
 
@@ -146,6 +148,7 @@ public final class BackgroundSyncTest {
     @Test
     @MediumTest
     @Feature({"BackgroundSync"})
+    @DisabledTest(message = "crbug.com/1015055")
     public void browserWakeUpScheduledWhenSyncEventFails() throws Exception {
         forceConnectionType(ConnectionType.CONNECTION_NONE);
 

chrome/android/javatests/src/org/chromium/chrome/browser/externalnav/UrlOverridingTest.java

@@ -224,7 +224,9 @@ public class UrlOverridingTest {
                 Assert.fail("Haven't received navigation failure of intents.");
                 return;
             }
-        } else if (createsNewTab) {
+        }
+
+        if (createsNewTab) {
             try {
                 destroyedCallback.waitForCallback(0, 1, 20, TimeUnit.SECONDS);
             } catch (TimeoutException ex) {
@@ -403,7 +405,7 @@ public class UrlOverridingTest {
         boolean opensNewTab =
                 !(mActivityTestRule.getActivity().getCurrentTabModel() instanceof SingleTabModel);
         loadUrlAndWaitForIntentUrl(mTestServer.getURL(OPEN_WINDOW_FROM_USER_GESTURE_PAGE), true,
-                opensNewTab, true, null, false);
+                opensNewTab, true, null, true);
     }
 
     @Test
@@ -413,7 +415,7 @@ public class UrlOverridingTest {
         boolean opensNewTab =
                 !(mActivityTestRule.getActivity().getCurrentTabModel() instanceof SingleTabModel);
         loadUrlAndWaitForIntentUrl(mTestServer.getURL(OPEN_WINDOW_FROM_LINK_USER_GESTURE_PAGE),
-                true, opensNewTab, true, null, false, "link");
+                true, opensNewTab, true, null, true, "link");
     }
 
     @Test
@@ -423,7 +425,7 @@ public class UrlOverridingTest {
         boolean opensNewTab =
                 !(mActivityTestRule.getActivity().getCurrentTabModel() instanceof SingleTabModel);
         loadUrlAndWaitForIntentUrl(mTestServer.getURL(OPEN_WINDOW_FROM_SVG_USER_GESTURE_PAGE), true,
-                opensNewTab, true, null, false, "link");
+                opensNewTab, true, null, true, "link");
     }
 
     @Test

chrome/browser/extensions/unpacked_installer.cc

@@ -303,8 +303,9 @@ bool UnpackedInstaller::IsLoadingUnpackedAllowed() const {
 void UnpackedInstaller::GetAbsolutePath() {
   extension_path_ = base::MakeAbsoluteFilePath(extension_path_);
 
+  // Set priority explicitly to avoid unwanted task priority inheritance.
   base::PostTask(
-      FROM_HERE, {BrowserThread::UI},
+      FROM_HERE, {BrowserThread::UI, base::TaskPriority::USER_BLOCKING},
       base::BindOnce(&UnpackedInstaller::CheckExtensionFileAccess, this));
 }
 
@@ -326,13 +327,17 @@ void UnpackedInstaller::CheckExtensionFileAccess() {
 void UnpackedInstaller::LoadWithFileAccess(int flags) {
   std::string error;
   if (!LoadExtension(Manifest::UNPACKED, flags, &error)) {
-    base::PostTask(FROM_HERE, {BrowserThread::UI},
+    // Set priority explicitly to avoid unwanted task priority inheritance.
+    base::PostTask(FROM_HERE,
+                   {BrowserThread::UI, base::TaskPriority::USER_BLOCKING},
                    base::BindOnce(&UnpackedInstaller::ReportExtensionLoadError,
                                   this, error));
     return;
   }
 
-  base::PostTask(FROM_HERE, {BrowserThread::UI},
+  // Set priority explicitly to avoid unwanted task priority inheritance.
+  base::PostTask(FROM_HERE,
+                 {BrowserThread::UI, base::TaskPriority::USER_BLOCKING},
                  base::BindOnce(&UnpackedInstaller::StartInstallChecks, this));
 }
 

content/browser/scheduler/browser_io_thread_delegate.cc

@@ -54,6 +54,7 @@ BrowserIOThreadDelegate::BrowserIOThreadDelegate()
     : owned_sequence_manager_(CreateUnboundSequenceManager(
           SequenceManager::Settings::Builder()
               .SetMessagePumpType(base::MessagePumpType::IO)
+              .SetAntiStarvationLogicForPrioritiesDisabled(true)
               .Build())),
       sequence_manager_(owned_sequence_manager_.get()) {
   Init();

content/browser/scheduler/browser_ui_thread_scheduler.cc

@@ -40,6 +40,7 @@ BrowserUIThreadScheduler::BrowserUIThreadScheduler()
           base::sequence_manager::CreateUnboundSequenceManager(
               base::sequence_manager::SequenceManager::Settings::Builder()
                   .SetMessagePumpType(base::MessagePumpType::UI)
+                  .SetAntiStarvationLogicForPrioritiesDisabled(true)
                   .Build())),
       task_queues_(BrowserThread::UI,
                    owned_sequence_manager_.get(),

extensions/browser/api/declarative_net_request/file_sequence_helper.cc

@@ -369,8 +369,10 @@ void FileSequenceHelper::LoadRulesets(
   }
 
   if (success) {
+    // Set priority explicitly to avoid unwanted task priority inheritance.
     base::PostTask(
-        FROM_HERE, {content::BrowserThread::UI},
+        FROM_HERE,
+        {content::BrowserThread::UI, base::TaskPriority::USER_BLOCKING},
         base::BindOnce(std::move(ui_callback), std::move(load_data)));
     return;
   }
@@ -402,9 +404,12 @@ void FileSequenceHelper::UpdateDynamicRules(
                                               UpdateDynamicRulesStatus status) {
     base::UmaHistogramEnumeration(kUpdateDynamicRulesStatusHistogram, status);
 
-    base::PostTask(FROM_HERE, {content::BrowserThread::UI},
-                   base::BindOnce(std::move(ui_callback), std::move(load_data),
-                                  std::move(error)));
+    // Set priority explicitly to avoid unwanted task priority inheritance.
+    base::PostTask(
+        FROM_HERE,
+        {content::BrowserThread::UI, base::TaskPriority::USER_BLOCKING},
+        base::BindOnce(std::move(ui_callback), std::move(load_data),
+                       std::move(error)));
   };
 
   int new_ruleset_checksum = -1;
@@ -449,8 +454,10 @@ void FileSequenceHelper::OnRulesetsReindexed(LoadRulesetsUICallback ui_callback,
   }
 
   // The UI thread will handle success or failure.
-  base::PostTask(FROM_HERE, {content::BrowserThread::UI},
-                 base::BindOnce(std::move(ui_callback), std::move(load_data)));
+  base::PostTask(
+      FROM_HERE,
+      {content::BrowserThread::UI, base::TaskPriority::USER_BLOCKING},
+      base::BindOnce(std::move(ui_callback), std::move(load_data)));
 }
 
 }  // namespace declarative_net_request

extensions/browser/extension_user_script_loader.cc

@@ -115,8 +115,10 @@ bool LoadScriptContent(const HostID& host_id,
     if (verifier.get()) {
       // Call VerifyContent() after yielding on UI thread so it is ensured that
       // ContentVerifierIOData is populated at the time we call VerifyContent().
+      // Priority set explicitly to avoid unwanted task priority inheritance.
       base::PostTask(
-          FROM_HERE, {content::BrowserThread::UI},
+          FROM_HERE,
+          {content::BrowserThread::UI, base::TaskPriority::USER_BLOCKING},
           base::BindOnce(
               &ForwardVerifyContentToIO,
               VerifyContentInfo(verifier, host_id.id(),
@@ -193,9 +195,12 @@ void LoadScriptsOnFileTaskRunner(
   LoadUserScripts(user_scripts.get(), hosts_info, added_script_ids, verifier);
   base::ReadOnlySharedMemoryRegion memory =
       UserScriptLoader::Serialize(*user_scripts);
-  base::PostTask(FROM_HERE, {content::BrowserThread::UI},
-                 base::BindOnce(std::move(callback), std::move(user_scripts),
-                                std::move(memory)));
+  // Explicit priority to prevent unwanted task priority inheritance.
+  base::PostTask(
+      FROM_HERE,
+      {content::BrowserThread::UI, base::TaskPriority::USER_BLOCKING},
+      base::BindOnce(std::move(callback), std::move(user_scripts),
+                     std::move(memory)));
 }
 
 }  // namespace