Commit bdb30701 authored by Carlos Caballero's avatar Carlos Caballero Committed by Commit Bot

Reland "Fix flakly content_browsertests"

This is a reland of d2d3af26

Also we now call
base::PostTaskAndroid::SignalNativeSchedulerShutdown() which on the
Java side has the following comment
"This is here to make C++ tests work"

Probably a good idea to call it :)

Also we no longer call ResetForTesting() but Shutdown instead. As this
what happens in prod.

Original change's description:
> Fix flakly content_browsertests
>
> Looks like BrowserTaskExecutor is being deleted while the IO thread is
> still running and thus capable of posting tasks. Also  browser tests are
> like prod, we shouldn't manually shut down pools and schedulers.
>
> Bug: 963702
> Change-Id: I1cb953acc4b495631fe269af9e06b94fbe5f2a52
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1628748
> Commit-Queue: Carlos Caballero <carlscab@google.com>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Reviewed-by: Alex Clarke <alexclarke@chromium.org>
> Reviewed-by: Gabriel Charette <gab@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#667097}

Bug: 963702
Change-Id: I88f2b8d418eeb1a9a44f8f17bd5c1147993e8e71
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1651725Reviewed-by: default avatarScott Violet <sky@chromium.org>
Reviewed-by: default avatarGabriel Charette <gab@chromium.org>
Commit-Queue: Carlos Caballero <carlscab@google.com>
Cr-Commit-Position: refs/heads/master@{#682624}
parent 2a3b1d53
...@@ -62,6 +62,8 @@ class CONTENT_EXPORT BrowserTaskExecutor : public base::TaskExecutor { ...@@ -62,6 +62,8 @@ class CONTENT_EXPORT BrowserTaskExecutor : public base::TaskExecutor {
// //
// Attention: This method can only be called once (as there must be only one // Attention: This method can only be called once (as there must be only one
// IO thread). // IO thread).
// Attention: Must be called after Create()
// Attention: Can not be called after Shutdown() or ResetForTesting()
static std::unique_ptr<BrowserProcessSubThread> CreateIOThread(); static std::unique_ptr<BrowserProcessSubThread> CreateIOThread();
// Enables non best effort queues on the IO thread. Usually called from // Enables non best effort queues on the IO thread. Usually called from
...@@ -83,7 +85,9 @@ class CONTENT_EXPORT BrowserTaskExecutor : public base::TaskExecutor { ...@@ -83,7 +85,9 @@ class CONTENT_EXPORT BrowserTaskExecutor : public base::TaskExecutor {
// Winds down the BrowserTaskExecutor, after this no tasks can be executed // Winds down the BrowserTaskExecutor, after this no tasks can be executed
// and the base::TaskExecutor APIs are non-functional but won't crash if // and the base::TaskExecutor APIs are non-functional but won't crash if
// called. // called. In unittests however we need to clean up, so
// BrowserTaskExecutor::ResetForTesting should be
// called (~TestBrowserThreadBundle() takes care of this).
static void Shutdown(); static void Shutdown();
// Unregister and delete the TaskExecutor after a test. // Unregister and delete the TaskExecutor after a test.
......
...@@ -69,6 +69,7 @@ ...@@ -69,6 +69,7 @@
#include "ui/gl/gl_switches.h" #include "ui/gl/gl_switches.h"
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
#include "base/android/task_scheduler/post_task_android.h"
#include "components/discardable_memory/service/discardable_shared_memory_manager.h" // nogncheck #include "components/discardable_memory/service/discardable_shared_memory_manager.h" // nogncheck
#include "content/app/mojo/mojo_init.h" #include "content/app/mojo/mojo_init.h"
#include "content/app/service_manager_environment.h" #include "content/app/service_manager_environment.h"
...@@ -461,20 +462,22 @@ void BrowserTestBase::SetUp() { ...@@ -461,20 +462,22 @@ void BrowserTestBase::SetUp() {
discardable_shared_memory_manager.reset(); discardable_shared_memory_manager.reset();
spawned_test_server_.reset(); spawned_test_server_.reset();
} }
BrowserTaskExecutor::ResetForTesting();
base::PostTaskAndroid::SignalNativeSchedulerShutdown();
BrowserTaskExecutor::Shutdown();
// Normally the BrowserMainLoop does this during shutdown but on Android we // Normally the BrowserMainLoop does this during shutdown but on Android we
// don't go through shutdown, so this doesn't happen there. We do need it // don't go through shutdown, so this doesn't happen there. We do need it
// for the test harness to be able to delete temp dirs. // for the test harness to be able to delete temp dirs.
base::ThreadRestrictions::SetIOAllowed(true); base::ThreadRestrictions::SetIOAllowed(true);
#else #else // defined(OS_ANDROID)
auto ui_task = std::make_unique<base::Closure>(base::Bind( auto ui_task = std::make_unique<base::Closure>(base::Bind(
&BrowserTestBase::ProxyRunTestOnMainThreadLoop, base::Unretained(this))); &BrowserTestBase::ProxyRunTestOnMainThreadLoop, base::Unretained(this)));
GetContentMainParams()->ui_task = ui_task.release(); GetContentMainParams()->ui_task = ui_task.release();
GetContentMainParams()->created_main_parts_closure = GetContentMainParams()->created_main_parts_closure =
created_main_parts_closure.release(); created_main_parts_closure.release();
EXPECT_EQ(expected_exit_code_, ContentMain(*GetContentMainParams())); EXPECT_EQ(expected_exit_code_, ContentMain(*GetContentMainParams()));
#endif #endif // defined(OS_ANDROID)
TearDownInProcessBrowserTestFixture(); TearDownInProcessBrowserTestFixture();
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment