Skip to content

Commit

Permalink
Fix flaky LacrosWebAppsControllerBrowserTest
Browse files Browse the repository at this point in the history
We add AppRegistrationWaiter, to wait for the app's registration to
appear in AppRegistryCache.

Bug: 1309148
Change-Id: Id78c90ced22c2c52dbeef522ed0b6f0f900e4461
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3551941
Auto-Submit: Eric Willigers <ericwilligers@chromium.org>
Reviewed-by: Daniel Murphy <dmurph@chromium.org>
Commit-Queue: Daniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/main@{#985547}
  • Loading branch information
ericwilligers authored and Chromium LUCI CQ committed Mar 25, 2022
1 parent 8dc2195 commit 54fdc46
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 25 deletions.
3 changes: 3 additions & 0 deletions chrome/browser/web_applications/BUILD.gn
Expand Up @@ -379,6 +379,8 @@ source_set("web_applications_test_support") {
"system_web_apps/test/test_system_web_app_url_data_source.h",
"system_web_apps/test/test_system_web_app_web_ui_controller_factory.cc",
"system_web_apps/test/test_system_web_app_web_ui_controller_factory.h",
"test/app_registration_waiter.cc",
"test/app_registration_waiter.h",
"test/fake_data_retriever.cc",
"test/fake_data_retriever.h",
"test/fake_externally_managed_app_manager.cc",
Expand Down Expand Up @@ -442,6 +444,7 @@ source_set("web_applications_test_support") {
"//chrome/browser/ui",
"//components/crx_file:crx_file",
"//components/keyed_service/content",
"//components/services/app_service/public/cpp:app_update",
"//components/services/app_service/public/cpp:app_url_handling",
"//components/services/app_service/public/cpp:types",
"//components/sync:test_support_model",
Expand Down
Expand Up @@ -10,6 +10,7 @@
#include "chrome/browser/ui/browser_window.h"
#include "chrome/browser/ui/web_applications/web_app_controller_browsertest.h"
#include "chrome/browser/web_applications/app_service/lacros_web_apps_controller.h"
#include "chrome/browser/web_applications/test/app_registration_waiter.h"
#include "chrome/browser/web_applications/web_app_utils.h"
#include "chromeos/crosapi/mojom/app_service_types.mojom.h"
#include "chromeos/crosapi/mojom/test_controller.mojom-test-utils.h"
Expand All @@ -21,40 +22,37 @@

namespace web_app {

using LacrosWebAppsControllerBrowserTest = web_app::WebAppControllerBrowserTest;
class LacrosWebAppsControllerBrowserTest : public WebAppControllerBrowserTest {
public:
LacrosWebAppsControllerBrowserTest() = default;
~LacrosWebAppsControllerBrowserTest() override = default;

// TODO(crbug.com/1309148): Disabled for flakiness.
// Test that the default context menu for a web app has the correct items.
IN_PROC_BROWSER_TEST_F(LacrosWebAppsControllerBrowserTest,
DISABLED_DefaultContextMenu) {
// If ash is does not contain the relevant test controller functionality, then
// there's nothing to do for this test.
if (chromeos::LacrosService::Get()->GetInterfaceVersion(
crosapi::mojom::TestController::Uuid_) <
static_cast<int>(crosapi::mojom::TestController::MethodMinVersions::
kDoesItemExistInShelfMinVersion)) {
LOG(WARNING) << "Unsupported ash version.";
return;
protected:
// If ash is does not contain the relevant test controller functionality,
// then there's nothing to do for this test.
bool IsServiceAvailable() {
DCHECK(IsWebAppsCrosapiEnabled());
auto* const service = chromeos::LacrosService::Get();
return service->GetInterfaceVersion(
crosapi::mojom::TestController::Uuid_) >=
static_cast<int>(crosapi::mojom::TestController::MethodMinVersions::
kDoesItemExistInShelfMinVersion);
}
};

ASSERT_TRUE(embedded_test_server()->Start());

EXPECT_TRUE(IsWebAppsCrosapiEnabled());
LacrosWebAppsController lacros_web_apps_controller(profile());
lacros_web_apps_controller.Init();
// Test that the default context menu for a web app has the correct items.
IN_PROC_BROWSER_TEST_F(LacrosWebAppsControllerBrowserTest, DefaultContextMenu) {
if (!IsServiceAvailable())
GTEST_SKIP() << "Unsupported ash version.";

const AppId app_id =
InstallPWA(embedded_test_server()->GetURL("/web_apps/basic.html"));
InstallPWA(https_server()->GetURL("/web_apps/basic.html"));
AppRegistrationWaiter(profile(), app_id).Await();

// No item should exist in the shelf before the web app is launched.
browser_test_util::WaitForShelfItem(app_id, /*exists=*/false);

crosapi::mojom::LaunchParamsPtr launch_params =
crosapi::mojom::LaunchParams::New();
launch_params->app_id = app_id;
launch_params->launch_source = apps::mojom::LaunchSource::kFromTest;
static_cast<crosapi::mojom::AppController&>(lacros_web_apps_controller)
.Launch(std::move(launch_params), base::DoNothing());
LaunchWebAppBrowser(app_id);

// Wait for item to exist in shelf.
browser_test_util::WaitForShelfItem(app_id, /*exists=*/true);
Expand Down
36 changes: 36 additions & 0 deletions chrome/browser/web_applications/test/app_registration_waiter.cc
@@ -0,0 +1,36 @@
// Copyright 2022 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "chrome/browser/web_applications/test/app_registration_waiter.h"

#include "chrome/browser/apps/app_service/app_service_proxy.h"
#include "chrome/browser/apps/app_service/app_service_proxy_factory.h"

namespace web_app {

AppRegistrationWaiter::AppRegistrationWaiter(Profile* profile,
const AppId& app_id)
: app_id_(app_id) {
apps::AppRegistryCache& cache =
apps::AppServiceProxyFactory::GetForProfile(profile)->AppRegistryCache();
Observe(&cache);
if (cache.ForOneApp(app_id, [](const apps::AppUpdate&) {}))
run_loop_.Quit();
}
AppRegistrationWaiter::~AppRegistrationWaiter() = default;

void AppRegistrationWaiter::Await() {
run_loop_.Run();
}

void AppRegistrationWaiter::OnAppUpdate(const apps::AppUpdate& update) {
if (update.AppId() == app_id_)
run_loop_.Quit();
}
void AppRegistrationWaiter::OnAppRegistryCacheWillBeDestroyed(
apps::AppRegistryCache* cache) {
Observe(nullptr);
}

} // namespace web_app
36 changes: 36 additions & 0 deletions chrome/browser/web_applications/test/app_registration_waiter.h
@@ -0,0 +1,36 @@
// Copyright 2022 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef CHROME_BROWSER_WEB_APPLICATIONS_TEST_APP_REGISTRATION_WAITER_H_
#define CHROME_BROWSER_WEB_APPLICATIONS_TEST_APP_REGISTRATION_WAITER_H_

#include "base/run_loop.h"
#include "chrome/browser/web_applications/web_app_id.h"
#include "components/services/app_service/public/cpp/app_registry_cache.h"
#include "components/services/app_service/public/cpp/app_update.h"

class Profile;

namespace web_app {

class AppRegistrationWaiter : public apps::AppRegistryCache::Observer {
public:
AppRegistrationWaiter(Profile* profile, const AppId& app_id);
~AppRegistrationWaiter() override;

void Await();

private:
// apps::AppRegistryCache::Observer:
void OnAppUpdate(const apps::AppUpdate& update) override;
void OnAppRegistryCacheWillBeDestroyed(
apps::AppRegistryCache* cache) override;

const AppId app_id_;
base::RunLoop run_loop_;
};

} // namespace web_app

#endif // CHROME_BROWSER_WEB_APPLICATIONS_TEST_APP_REGISTRATION_WAITER_H_

0 comments on commit 54fdc46

Please sign in to comment.