Add NoBestEffortTasksTest for extension loading/messaging.

Adds a browser test that loads an extension, sends it a message, and
receives a reply; all while BEST_EFFORT tasks are disabled. This is a
regression test for http://crbug.com/177163#c112.

Note that the test caught a bug in that BEST_EFFORT task priority was
being used when the NetworkService was enabled (different code path
than normal). This change fixes that as well.

Bug: 177163,924416,925117
Change-Id: I9e4e1874e0f03d5be53224845a4f6fa42ffc68b3
Reviewed-on: https://chromium-review.googlesource.com/c/1427468
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Chris Mumford <cmumford@google.com>
Commit-Queue: François Doray <fdoray@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#626544}(cherry picked from commit 60ea85d)
Reviewed-on: https://chromium-review.googlesource.com/c/1444993
Reviewed-by: Yuri Wiitala <miu@chromium.org>
Cr-Commit-Position: refs/branch-heads/3683@{#55}
Cr-Branched-From: e510299-refs/heads/master@{#625896}

chrome/browser/no_best_effort_tasks_browsertest.cc

@@ -4,17 +4,35 @@
 
 #include "base/base_switches.h"
 #include "base/command_line.h"
+#include "base/files/file_path.h"
 #include "base/macros.h"
+#include "base/path_service.h"
 #include "base/run_loop.h"
+#include "base/strings/string_piece.h"
+#include "base/strings/stringprintf.h"
 #include "build/build_config.h"
+#include "build/buildflag.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/common/chrome_paths.h"
 #include "chrome/test/base/in_process_browser_test.h"
+#include "chrome/test/base/ui_test_utils.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
+#include "content/public/test/browser_test_utils.h"
 #include "content/public/test/test_utils.h"
+#include "net/dns/mock_host_resolver.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+#include "chrome/browser/extensions/unpacked_installer.h"
+#include "extensions/browser/extension_registry.h"
+#include "extensions/browser/extension_system.h"
+#include "extensions/browser/test_extension_registry_observer.h"
+#include "extensions/common/extension.h"
+#endif
+
 namespace {
 
 class RunLoopUntilLoadedAndPainted : public content::WebContentsObserver {
@@ -63,11 +81,25 @@ class NoBestEffortTasksTest : public InProcessBrowserTest {
  private:
   void SetUpCommandLine(base::CommandLine* command_line) override {
     command_line->AppendSwitch(switches::kDisableBackgroundTasks);
+    InProcessBrowserTest::SetUpCommandLine(command_line);
+  }
+
+  void SetUpOnMainThread() override {
+    // Redirect all DNS requests back to localhost (to the embedded test
+    // server).
+    host_resolver()->AddRule("*", "127.0.0.1");
+    InProcessBrowserTest::SetUpOnMainThread();
   }
 
   DISALLOW_COPY_AND_ASSIGN(NoBestEffortTasksTest);
 };
 
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+constexpr base::StringPiece kExtensionId = "ddchlicdkolnonkihahngkmmmjnjlkkf";
+constexpr base::TimeDelta kSendMessageRetryPeriod =
+    base::TimeDelta::FromMilliseconds(250);
+#endif
+
 }  // namespace
 
 // Verify that it is possible to load and paint the initial about:blank page
@@ -106,3 +138,74 @@ IN_PROC_BROWSER_TEST_F(NoBestEffortTasksTest, LoadAndPaintFromNetwork) {
   RunLoopUntilLoadedAndPainted run_until_loaded_and_painted(web_contents);
   run_until_loaded_and_painted.Run();
 }
+
+// Verify that an extension can be loaded and perform basic messaging without
+// running BEST_EFFORT tasks. Regression test for http://crbug.com/177163#c112.
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+IN_PROC_BROWSER_TEST_F(NoBestEffortTasksTest, LoadExtensionAndSendMessages) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+
+  // Load the extension, waiting until the ExtensionRegistry reports that its
+  // renderer has been started.
+  base::FilePath extension_dir;
+  const bool have_test_data_dir =
+      base::PathService::Get(chrome::DIR_TEST_DATA, &extension_dir);
+  ASSERT_TRUE(have_test_data_dir);
+  extension_dir = extension_dir.AppendASCII("extensions")
+                      .AppendASCII("no_best_effort_tasks_test_extension");
+  extensions::UnpackedInstaller::Create(
+      extensions::ExtensionSystem::Get(browser()->profile())
+          ->extension_service())
+      ->Load(extension_dir);
+  auto* const extension =
+      extensions::TestExtensionRegistryObserver(
+          extensions::ExtensionRegistry::Get(browser()->profile()))
+          .WaitForExtensionReady();
+  ASSERT_TRUE(extension);
+  ASSERT_EQ(kExtensionId, extension->id());
+
+  // Navigate to a test page, waiting until complete. Note that the hostname
+  // here must match the pattern found in the extension's manifest file, or it
+  // will not be able to send/receive messaging from the test web page (due to
+  // extension permissions).
+  ui_test_utils::NavigateToURL(
+      browser(),
+      embedded_test_server()->GetURL("fake.chromium.org", "/empty.html"));
+
+  // Execute JavaScript in the test page, to send a ping message to the
+  // extension and await the reply. The chrome.runtime.sendMessage() operation
+  // can fail if the extension's background page hasn't finished running yet
+  // (i.e., there is no message listener yet). Thus, use a retry loop.
+  const std::string request_reply_javascript = base::StringPrintf(
+      "new Promise((resolve, reject) => {\n"
+      "  chrome.runtime.sendMessage(\n"
+      "      '%s',\n"
+      "      {ping: true},\n"
+      "      response => {\n"
+      "        if (response) {\n"
+      "          resolve(response);\n"
+      "        } else {\n"
+      "          reject(chrome.runtime.lastError.message);\n"
+      "        }\n"
+      "      });\n"
+      "})",
+      extension->id().c_str());
+  for (;;) {
+    const auto result =
+        content::EvalJs(browser()->tab_strip_model()->GetActiveWebContents(),
+                        request_reply_javascript);
+    if (result.error.empty()) {
+      LOG(INFO) << "Got a response from the extension.";
+      EXPECT_TRUE(result.value.FindBoolKey("pong").value_or(false));
+      break;
+    }
+    // An error indicates the extension's message listener isn't up yet. Wait a
+    // little before trying again.
+    LOG(INFO) << "Waiting for the extension's message listener...";
+    base::RunLoop run_loop;
+    base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
+        FROM_HERE, run_loop.QuitClosure(), kSendMessageRetryPeriod);
+    run_loop.Run();
+  }
+}
+#endif  // BUILDFLAG(ENABLE_EXTENSIONS)

chrome/test/data/extensions/no_best_effort_tasks_test_extension/background.js

@@ -0,0 +1,10 @@
+// Copyright 2019 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.
+
+chrome.runtime.onMessageExternal.addListener((request, from, sendResponse) => {
+  if (request.ping) {
+    console.info("Got ping, sending pong...");
+    sendResponse({pong: true});
+  }
+});

chrome/test/data/extensions/no_best_effort_tasks_test_extension/manifest.json

@@ -0,0 +1,14 @@
+{
+  "key": "MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC8xv6iO+j4kzj1HiBL93+XVJH/CRyAQMUHS/Z0l8nCAzaAFkW/JsNwxJqQhrZspnxLqbQxNncXs6g6bsXAwKHiEs+LSs+bIv0Gc/2ycZdhXJ8GhEsSMakog5dpQd1681c2gLK/8CrAoewE/0GIKhaFcp7a2iZlGh4Am6fgMKy0iQIDAQAB",
+  "name": "NoBestEffortTasksTest extension",
+  "version": "1",
+  "manifest_version": 2,
+  "description": "Responds to ping messages.",
+  "background": {
+    "scripts": ["background.js"],
+    "persistent": true
+  },
+  "externally_connectable": {
+    "matches": ["*://*.chromium.org/*"]
+  }
+}

content/browser/file_url_loader_factory.cc

@@ -851,8 +851,11 @@ void CreateFileURLLoader(
     network::mojom::URLLoaderClientPtr client,
     std::unique_ptr<FileURLLoaderObserver> observer,
     scoped_refptr<net::HttpResponseHeaders> extra_response_headers) {
+  // TODO(crbug.com/924416): Re-evaluate how TaskPriority is set here and in
+  // other file URL-loading-related code. Some callers require USER_VISIBLE
+  // (i.e., BEST_EFFORT is not enough).
   auto task_runner = base::CreateSequencedTaskRunnerWithTraits(
-      {base::MayBlock(), base::TaskPriority::BEST_EFFORT,
+      {base::MayBlock(), base::TaskPriority::USER_VISIBLE,
        base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN});
   task_runner->PostTask(
       FROM_HERE,
@@ -868,10 +871,12 @@ std::unique_ptr<network::mojom::URLLoaderFactory> CreateFileURLLoaderFactory(
     const base::FilePath& profile_path,
     scoped_refptr<const SharedCorsOriginAccessList>
         shared_cors_origin_access_list) {
+  // TODO(crbug.com/924416): Re-evaluate TaskPriority: Should the caller provide
+  // it?
   return std::make_unique<content::FileURLLoaderFactory>(
       profile_path, shared_cors_origin_access_list,
       base::CreateSequencedTaskRunnerWithTraits(
-          {base::MayBlock(), base::TaskPriority::BEST_EFFORT,
+          {base::MayBlock(), base::TaskPriority::USER_VISIBLE,
            base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN}));
 }