Commit 1b15a718 authored by Glen Robertson's avatar Glen Robertson Committed by Commit Bot

Deflake InstallReplacementWebAppApiTests by disabling legacy finalizer.

Tests were occasionally failing due to having an unexpected extra app
counted in ExtensionBrowserTest::InstallOrUpdateExtension. This was
because the test is awaiting the new BMO system to complete an install
but not the async legacy extension-based install that is fired
simultaneously by the new system. If the legacy install happens to
complete between counting num_before and num_after then we saw the web
app as an extra, unexpected installed extension.

Bug: 1094616
Change-Id: I2a14717e3f84102497c7a42c5b30bd6a5f868ed4
Fixed: 1094616
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2262357Reviewed-by: default avatarAlexey Baskakov <loyso@chromium.org>
Reviewed-by: default avatarGlen Robertson <glenrob@chromium.org>
Reviewed-by: default avatarDevlin <rdevlin.cronin@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Auto-Submit: Glen Robertson <glenrob@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781926}
parent 1d724b6c
......@@ -18,6 +18,7 @@
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/browser/web_applications/components/app_registrar.h"
#include "chrome/browser/web_applications/components/app_shortcut_manager.h"
#include "chrome/browser/web_applications/components/install_finalizer.h"
#include "chrome/browser/web_applications/components/web_app_helpers.h"
#include "chrome/browser/web_applications/components/web_app_provider_base.h"
#include "chrome/browser/web_applications/test/test_web_app_ui_manager.h"
......@@ -36,6 +37,7 @@
#include "extensions/test/result_catcher.h"
#include "extensions/test/test_extension_dir.h"
#include "net/dns/mock_host_resolver.h"
#include "testing/gtest/include/gtest/gtest.h"
using extensions::Extension;
using extensions::Manifest;
......@@ -241,9 +243,12 @@ class InstallReplacementWebAppApiTest : public ExtensionManagementApiTest {
chrome::SetAutoAcceptPWAInstallConfirmationForTesting(true);
const GURL start_url = https_test_server_.GetURL(web_app_start_url);
web_app::AppId web_app_id = web_app::GenerateAppIdFromURL(start_url);
auto* provider =
web_app::WebAppProviderBase::GetProviderBase(browser()->profile());
// Async legacy finalizer install was causing this test to be flaky (see
// crbug.com/1094616).
provider->install_finalizer().RemoveLegacyInstallFinalizerForTesting();
EXPECT_FALSE(provider->registrar().IsLocallyInstalled(start_url));
EXPECT_EQ(0, static_cast<int>(
provider->ui_manager().GetNumWindowsForApp(web_app_id)));
......
......@@ -98,6 +98,8 @@ class InstallFinalizer {
bool shortcut_created,
content::WebContents* web_contents);
virtual void RemoveLegacyInstallFinalizerForTesting() {}
virtual void Start() {}
virtual void Shutdown() {}
......
......@@ -343,6 +343,10 @@ void WebAppInstallFinalizer::FinalizeUpdate(
legacy_finalizer_->FinalizeUpdate(web_app_info, base::DoNothing());
}
void WebAppInstallFinalizer::RemoveLegacyInstallFinalizerForTesting() {
legacy_finalizer_ = nullptr;
}
void WebAppInstallFinalizer::Start() {
DCHECK(!started_);
started_ = true;
......
......@@ -50,6 +50,7 @@ class WebAppInstallFinalizer final : public InstallFinalizer {
void UninstallExternalAppByUser(const AppId& app_id,
UninstallWebAppCallback callback) override;
bool WasExternalAppUninstalledByUser(const AppId& app_id) const override;
void RemoveLegacyInstallFinalizerForTesting() override;
void Start() override;
void Shutdown() override;
......
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