Commit 5cc75144 authored by Olivier Li's avatar Olivier Li Committed by Commit Bot

Have...

Have ExtensionWebRequestApiTest.WebRequestApiDoesNotCrashOnErrorAfterProfileDestroyed better represent task ordering in production.

Work done in the context of crbug.com/661143 uncovered a problem in this test
that should be fixed before going forward. The problem is present regardless of
HistoryService's task runner behavior but is currently hidden by the fact that
HistoryService joins its backend_task_runner on destruction.

Like explained in the comment there should not be any tasks pending when we
reach Profile destruction because otherwise it does not represent the shutdown
sequence present in the browser.

In this particular case this presented itself in the form of a use after free
of the profile object by callbacks of tasks still running on the ThreadPool.

This test is the only test in the codebase that destroys a profile using
DestroyProfileWhenAppropriate directly (apart from tests testing the function
itself). We will be able to fix other tests seamlessly by improving

as it uses a function that is not only for tests to achieve a similar purpose.

KeyedServiceFactory: :SetTestingFactory but this test has to be fixed directly
Change-Id: Iab7020b9c33091b2c997acce6bcbe2ac225da0f3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1879202Reviewed-by: default avatarDominic Battré <battre@chromium.org>
Commit-Queue: Oliver Li <olivierli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709492}
parent dd8e4931
...@@ -1903,6 +1903,14 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest, ...@@ -1903,6 +1903,14 @@ IN_PROC_BROWSER_TEST_F(ExtensionWebRequestApiTest,
// WebRequestAPI. This will cause the connection error that will reach the // WebRequestAPI. This will cause the connection error that will reach the
// proxy before the ProxySet shutdown code runs on the IO thread. // proxy before the ProxySet shutdown code runs on the IO thread.
api->Shutdown(); api->Shutdown();
// We are about to destroy a profile. In production that will only happen
// as part of the destruction of BrowserProcess's ProfileManager. This
// happens in ShutdownPostThreadsStop(). This means that to have this test
// represent production we have to make sure that no tasks are pending before
// we destroy the profile.
content::RunAllTasksUntilIdle();
ProfileDestroyer::DestroyProfileWhenAppropriate(temp_profile); ProfileDestroyer::DestroyProfileWhenAppropriate(temp_profile);
client.Unbind(); client.Unbind();
api.reset(); api.reset();
......
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