Skip to content

Commit

Permalink
Fix breakages while performance_browser_tests was not running on Mac.
Browse files Browse the repository at this point in the history
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}
  • Loading branch information
miu-chromium authored and Commit Bot committed May 31, 2017
1 parent 4ac11ed commit 8e0e80c
Show file tree
Hide file tree
Showing 4 changed files with 17 additions and 8 deletions.
2 changes: 2 additions & 0 deletions PRESUBMIT.py
Original file line number Diff line number Diff line change
Expand Up @@ -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$",
Expand Down
3 changes: 3 additions & 0 deletions chrome/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -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" ]
Expand Down
16 changes: 8 additions & 8 deletions chrome/test/perf/mach_ports_performancetest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<int> port_counts_;
};

Expand Down
4 changes: 4 additions & 0 deletions media/cast/test/utility/standalone_cast_environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit 8e0e80c

Please sign in to comment.