Commit 93ca0f8e authored by Michael Giuffrida's avatar Michael Giuffrida Committed by Commit Bot

Fix flaky ShellNativeAppWindowAuraTest.Bounds

This test creates its own TestBrowserContext, but it derives from
ExtensionsTest, which also creates one. Making the test use the
provided context instead of creating its own fixes the flake. The
flake is fairly rare but becomes more frequent as we add new tests
to app_shell_unittests. Read on for details on how this failure
occurs, why it was flaky and why more tests make it fail more often.

When run in combination with other tests, once in a while the new
context happened to be created at the same address as a context used
in a previous test, just by chance.

The process-wide DependencyManager keeps track of the pointers to
contexts that have already been shut down. ExtensionsTest is smart
enough to register its own context as "live", so this flake doesn't
normally happen, but this test didn't bother.

A new context at an old address would be fine *if* the context was
marked "live" again. When it isn't, accessing KeyedService with the new
context will cause DependencyManager to identify it as a "dead" context
(as seen in a previous test) and assert.

If we really needed a separate TestBrowserContext, we could mark it
live, but why bother?

Bug: 750530
Change-Id: I094be7ea4c715a8316547fb2b9f06c081f963483
Reviewed-on: https://chromium-review.googlesource.com/592546
Commit-Queue: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: default avatarElliot Glaysher <erg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491178}
parent 4297bf0f
...@@ -9,7 +9,6 @@ ...@@ -9,7 +9,6 @@
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "content/public/browser/browser_context.h" #include "content/public/browser/browser_context.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "content/public/test/test_browser_context.h"
#include "content/public/test/test_browser_thread_bundle.h" #include "content/public/test/test_browser_thread_bundle.h"
#include "extensions/browser/app_window/app_window.h" #include "extensions/browser/app_window/app_window.h"
#include "extensions/browser/app_window/test_app_window_contents.h" #include "extensions/browser/app_window/test_app_window_contents.h"
...@@ -37,10 +36,6 @@ class ShellNativeAppWindowAuraTest : public ExtensionsTest { ...@@ -37,10 +36,6 @@ class ShellNativeAppWindowAuraTest : public ExtensionsTest {
}; };
TEST_F(ShellNativeAppWindowAuraTest, Bounds) { TEST_F(ShellNativeAppWindowAuraTest, Bounds) {
// The BrowserContext used here must be destroyed before the thread bundle,
// because of destructors of things spawned from creating a WebContents.
std::unique_ptr<content::BrowserContext> browser_context(
new content::TestBrowserContext);
scoped_refptr<Extension> extension = scoped_refptr<Extension> extension =
ExtensionBuilder() ExtensionBuilder()
.SetManifest(DictionaryBuilder() .SetManifest(DictionaryBuilder()
...@@ -50,10 +45,10 @@ TEST_F(ShellNativeAppWindowAuraTest, Bounds) { ...@@ -50,10 +45,10 @@ TEST_F(ShellNativeAppWindowAuraTest, Bounds) {
.Build()) .Build())
.Build(); .Build();
AppWindow* app_window = new AppWindow( AppWindow* app_window =
browser_context.get(), new ShellAppDelegate, extension.get()); new AppWindow(browser_context(), new ShellAppDelegate, extension.get());
content::WebContents* web_contents = content::WebContents::Create( content::WebContents* web_contents = content::WebContents::Create(
content::WebContents::CreateParams(browser_context.get())); content::WebContents::CreateParams(browser_context()));
app_window->SetAppWindowContentsForTesting( app_window->SetAppWindowContentsForTesting(
base::MakeUnique<TestAppWindowContents>(web_contents)); base::MakeUnique<TestAppWindowContents>(web_contents));
......
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