From 78c2b6631dffe019deb24d87cf86750408683e6d Mon Sep 17 00:00:00 2001 From: Yuri Wiitala Date: Wed, 30 Jan 2019 01:22:43 +0000 Subject: [PATCH] Add NoBestEffortTasksTest for extension loading/messaging. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Reviewed-by: Avi Drissman Reviewed-by: Chris Mumford Commit-Queue: François Doray Cr-Original-Commit-Position: refs/heads/master@{#626544}(cherry picked from commit 60ea85d4f73dba5765581e6c47093d8d204ebfa1) Reviewed-on: https://chromium-review.googlesource.com/c/1444993 Reviewed-by: Yuri Wiitala Cr-Commit-Position: refs/branch-heads/3683@{#55} Cr-Branched-From: e51029943e0a38dd794b73caaf6373d5496ae783-refs/heads/master@{#625896} --- .../no_best_effort_tasks_browsertest.cc | 103 ++++++++++++++++++ .../background.js | 10 ++ .../manifest.json | 14 +++ content/browser/file_url_loader_factory.cc | 9 +- 4 files changed, 134 insertions(+), 2 deletions(-) create mode 100644 chrome/test/data/extensions/no_best_effort_tasks_test_extension/background.js create mode 100644 chrome/test/data/extensions/no_best_effort_tasks_test_extension/manifest.json diff --git a/chrome/browser/no_best_effort_tasks_browsertest.cc b/chrome/browser/no_best_effort_tasks_browsertest.cc index ef89781473556..3e72c0a227043 100644 --- a/chrome/browser/no_best_effort_tasks_browsertest.cc +++ b/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) diff --git a/chrome/test/data/extensions/no_best_effort_tasks_test_extension/background.js b/chrome/test/data/extensions/no_best_effort_tasks_test_extension/background.js new file mode 100644 index 0000000000000..ee7ee7de446f6 --- /dev/null +++ b/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}); + } +}); diff --git a/chrome/test/data/extensions/no_best_effort_tasks_test_extension/manifest.json b/chrome/test/data/extensions/no_best_effort_tasks_test_extension/manifest.json new file mode 100644 index 0000000000000..6ea59be10c054 --- /dev/null +++ b/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/*"] + } +} diff --git a/content/browser/file_url_loader_factory.cc b/content/browser/file_url_loader_factory.cc index e3eed3dfa5bb7..61494efeae4eb 100644 --- a/content/browser/file_url_loader_factory.cc +++ b/content/browser/file_url_loader_factory.cc @@ -851,8 +851,11 @@ void CreateFileURLLoader( network::mojom::URLLoaderClientPtr client, std::unique_ptr observer, scoped_refptr 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 CreateFileURLLoaderFactory( const base::FilePath& profile_path, scoped_refptr shared_cors_origin_access_list) { + // TODO(crbug.com/924416): Re-evaluate TaskPriority: Should the caller provide + // it? return std::make_unique( 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})); }