From 8e0e80c3608ac2a08e87ead7d103578f6751f2f6 Mon Sep 17 00:00:00 2001 From: miu Date: Tue, 30 May 2017 20:35:57 -0700 Subject: [PATCH] Fix breakages while performance_browser_tests was not running on Mac. In our vain attempt to re-enable performance_browser_tests on the Mac bots, a number of breakages seem to have happened over the months where we were not even aware the tests weren't running. This change fixes all of the following: 1. Adds a BUILD dependency on //chrome:chrome_app. Without this, the test binary would be missing many runtime dependencies (e.g., the whole "Chrome Framework" bundle). 2. Migrate MachPortsTest to use BrowserTestBase::embedded_test_server() instead of its own server of "fake content" for its browser tests. 3. Add base::ScopedAllowIO for a test utility class that creates and tears down its own set of threads. It's not clear why the thread restriction is only checked on Mac (perhaps it's in the way the browser test's main thread is set up?). Nevertheless, it's perfectly reasonable for the main thread to block on stopping these other threads: After the test has run, the threads are shut down before the data analysis begins, since we want to be sure the data sets are no longer mutating. Also, added PRESUBMIT.py exception for this use case. BUG=697444,722367 TBR=nduca Review-Url: https://codereview.chromium.org/2901113003 Cr-Commit-Position: refs/heads/master@{#475773} --- PRESUBMIT.py | 2 ++ chrome/test/BUILD.gn | 3 +++ chrome/test/perf/mach_ports_performancetest.cc | 16 ++++++++-------- .../test/utility/standalone_cast_environment.cc | 4 ++++ 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 215ffbcfde47d1..7f09ce510a65b5 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -212,6 +212,8 @@ r"^content[\\\/]shell[\\\/]browser[\\\/]shell_browser_main\.cc$", r"^content[\\\/]shell[\\\/]browser[\\\/]shell_message_filter\.cc$", r"^content[\\\/]test[\\\/]ppapi[\\\/]ppapi_test\.cc$", + r"^media[\\\/]cast[\\\/]test[\\\/]utility[\\\/]" + + r"standalone_cast_environment\.cc$", r"^mojo[\\\/]edk[\\\/]embedder[\\\/]" + r"simple_platform_shared_buffer_posix\.cc$", r"^net[\\\/]disk_cache[\\\/]cache_util\.cc$", diff --git a/chrome/test/BUILD.gn b/chrome/test/BUILD.gn index 56b69361698d20..cd80d9be5a8589 100644 --- a/chrome/test/BUILD.gn +++ b/chrome/test/BUILD.gn @@ -5026,6 +5026,9 @@ if (!is_android && !is_chromecast) { "$root_out_dir/resources.pak", ] } + if (is_mac) { + deps += [ "//chrome:chrome_app" ] + } # This target should not require the Chrome executable to run. assert_no_deps = [ "//chrome" ] diff --git a/chrome/test/perf/mach_ports_performancetest.cc b/chrome/test/perf/mach_ports_performancetest.cc index 7887298a76fbd5..e7b1d786fd4a0b 100644 --- a/chrome/test/perf/mach_ports_performancetest.cc +++ b/chrome/test/perf/mach_ports_performancetest.cc @@ -25,13 +25,13 @@ namespace { class MachPortsTest : public InProcessBrowserTest { public: MachPortsTest() { - server_.ServeFilesFromSourceDirectory("data/mach_ports/moz"); + embedded_test_server()->ServeFilesFromSourceDirectory( + "data/mach_ports/moz"); } - void SetUp() override { - InProcessBrowserTest::SetUp(); - - ASSERT_TRUE(server_.Start()); + void SetUpOnMainThread() override { + InProcessBrowserTest::SetUpOnMainThread(); + ASSERT_TRUE(embedded_test_server()->Start()); } void TearDown() override { @@ -72,12 +72,12 @@ class MachPortsTest : public InProcessBrowserTest { // Adds a tab from the page cycler data at the specified domain. void AddTab(const std::string& domain) { - GURL url = server_.GetURL("/" + domain + "/").Resolve("?skip"); - AddTabAtIndex(0, url, ui::PAGE_TRANSITION_TYPED); + AddTabAtIndex( + 0, embedded_test_server()->GetURL("/" + domain + "/").Resolve("?skip"), + ui::PAGE_TRANSITION_TYPED); } private: - net::EmbeddedTestServer server_; std::vector port_counts_; }; diff --git a/media/cast/test/utility/standalone_cast_environment.cc b/media/cast/test/utility/standalone_cast_environment.cc index 272f04d96a57ed..9a2b4b23dbcf79 100644 --- a/media/cast/test/utility/standalone_cast_environment.cc +++ b/media/cast/test/utility/standalone_cast_environment.cc @@ -6,6 +6,7 @@ #include "base/memory/ptr_util.h" #include "base/message_loop/message_loop.h" +#include "base/threading/thread_restrictions.h" #include "base/time/default_tick_clock.h" namespace media { @@ -40,6 +41,9 @@ StandaloneCastEnvironment::~StandaloneCastEnvironment() { void StandaloneCastEnvironment::Shutdown() { CHECK(CalledOnValidThread()); + + base::ThreadRestrictions::ScopedAllowIO + because_i_brought_you_into_this_world_and_i_am_gonna_take_you_out; main_thread_.Stop(); audio_thread_.Stop(); video_thread_.Stop();