Skip to content

Commit

Permalink
[dPWA] Migrating uses of deprecated registrar to registrar_unsafe
Browse files Browse the repository at this point in the history
This CL is migrating usage of WebAppProvider::registrar to
WebAppProvider::registrar_unsafe before WebAppProvider::registrar
is removed.

Design doc linked in crbug.com/1399641

There is no functional change in this CL.

This CL was uploaded by git cl split.

R=alancutter@chromium.org

Bug: 1399641 b/260863363
Change-Id: Iac38cae6e7d67c7088d18abc20c963a38d9f9eed
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4087849
Auto-Submit: Camden King <camdenking@google.com>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1084855}
  • Loading branch information
camden-king authored and Chromium LUCI CQ committed Dec 19, 2022
1 parent 32aacb5 commit a7a5c33
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 43 deletions.
Expand Up @@ -113,7 +113,7 @@ class FileHandlerLaunchDialogTest : public InProcessBrowserTest {

const WebApp* GetApp() {
return WebAppProvider::GetForTest(browser()->profile())
->registrar()
->registrar_unsafe()
.GetAppById(app_id_);
}

Expand Down
Expand Up @@ -119,7 +119,7 @@ void LaunchAppUserChoiceDialogView::InitChildViews() {
{
web_app::WebAppProvider* provider =
web_app::WebAppProvider::GetForWebApps(profile_);
web_app::WebAppRegistrar& registrar = provider->registrar();
web_app::WebAppRegistrar& registrar = provider->registrar_unsafe();
auto app_info_view = std::make_unique<views::View>();
int icon_label_spacing = layout_provider->GetDistanceMetric(
views::DISTANCE_RELATED_CONTROL_HORIZONTAL);
Expand All @@ -129,7 +129,7 @@ void LaunchAppUserChoiceDialogView::InitChildViews() {

provider->icon_manager().ReadIcons(
app_id_, IconPurpose::ANY,
provider->registrar().GetAppDownloadedIconSizesAny(app_id_),
provider->registrar_unsafe().GetAppDownloadedIconSizesAny(app_id_),
base::BindOnce(&LaunchAppUserChoiceDialogView::OnIconsRead,
weak_ptr_factory_.GetWeakPtr()));
icon_image_view_ =
Expand Down
70 changes: 36 additions & 34 deletions chrome/browser/ui/views/web_apps/web_app_integration_test_driver.cc
Expand Up @@ -441,7 +441,8 @@ class UninstallCompleteWaiter final : public BrowserListObserver,
BrowserList::AddObserver(this);
WebAppProvider* provider = WebAppProvider::GetForTest(profile);
observation_.Observe(&provider->install_manager());
uninstall_complete_ = provider->registrar().GetAppById(app_id) == nullptr;
uninstall_complete_ =
provider->registrar_unsafe().GetAppById(app_id) == nullptr;
MaybeFinishWaiting();
}

Expand Down Expand Up @@ -817,13 +818,13 @@ void WebAppIntegrationTestDriver::TearDownOnMainThread() {
auto* provider = GetProviderForProfile(profile);
if (!provider)
continue;
std::vector<AppId> app_ids = provider->registrar().GetAppIds();
std::vector<AppId> app_ids = provider->registrar_unsafe().GetAppIds();
for (auto& app_id : app_ids) {
LOG(INFO) << "TearDownOnMainThread: Uninstalling " << app_id << ".";
const WebApp* app = provider->registrar().GetAppById(app_id);
const WebApp* app = provider->registrar_unsafe().GetAppById(app_id);
if (app->IsPolicyInstalledApp())
UninstallPolicyAppById(app_id);
if (provider->registrar().IsInstalled(app_id)) {
if (provider->registrar_unsafe().IsInstalled(app_id)) {
DCHECK(app->CanUserUninstallWebApp());
UninstallCompleteWaiter uninstall_waiter(profile, app_id);
base::RunLoop run_loop;
Expand Down Expand Up @@ -912,7 +913,7 @@ void WebAppIntegrationTestDriver::AwaitManifestUpdate(Site site) {
if (!BeforeStateChangeAction(__FUNCTION__))
return;
AppId app_id = GetAppIdBySiteMode(site);
ASSERT_TRUE(provider()->registrar().GetAppById(app_id));
ASSERT_TRUE(provider()->registrar_unsafe().GetAppById(app_id));
if (!previous_manifest_updates_.contains(app_id)) {
waiting_for_update_id_ = app_id;
waiting_for_update_run_loop_ = std::make_unique<base::RunLoop>();
Expand All @@ -929,7 +930,7 @@ void WebAppIntegrationTestDriver::AwaitManifestUpdate(Site site) {
// the app's scope in the web app database. Returns immediately if they are
// already consistent.
WebAppScopeWaiter(profile(), app_id,
provider()->registrar().GetAppScope(app_id))
provider()->registrar_unsafe().GetAppScope(app_id))
.Await();

AfterStateChangeAction();
Expand Down Expand Up @@ -1041,7 +1042,7 @@ void WebAppIntegrationTestDriver::InstallLocally(Site site) {
if (!BeforeStateChangeAction(__FUNCTION__))
return;
AppId app_id = GetAppIdBySiteMode(site);
ASSERT_TRUE(provider()->registrar().GetAppById(app_id))
ASSERT_TRUE(provider()->registrar_unsafe().GetAppById(app_id))
<< "No app installed for site: " << static_cast<int>(site);
content::TestWebUI test_web_ui;
content::WebContents* web_contents =
Expand Down Expand Up @@ -1257,10 +1258,10 @@ void WebAppIntegrationTestDriver::LaunchFromChromeApps(Site site) {
if (!BeforeStateChangeAction(__FUNCTION__))
return;
AppId app_id = GetAppIdBySiteMode(site);
ASSERT_TRUE(provider()->registrar().GetAppById(app_id))
ASSERT_TRUE(provider()->registrar_unsafe().GetAppById(app_id))
<< "No app installed for site: " << static_cast<int>(site);

WebAppRegistrar& app_registrar = provider()->registrar();
WebAppRegistrar& app_registrar = provider()->registrar_unsafe();
DisplayMode display_mode = app_registrar.GetAppEffectiveDisplayMode(app_id);
if (display_mode == blink::mojom::DisplayMode::kBrowser) {
ui_test_utils::UrlLoadObserver url_observer(
Expand All @@ -1281,7 +1282,7 @@ void WebAppIntegrationTestDriver::LaunchFromLaunchIcon(Site site) {
base::AutoReset<bool> intent_picker_bubble_scope =
IntentPickerBubbleView::SetAutoAcceptIntentPickerBubbleForTesting();
AppId app_id = GetAppIdBySiteMode(site);
ASSERT_TRUE(provider()->registrar().GetAppById(app_id))
ASSERT_TRUE(provider()->registrar_unsafe().GetAppById(app_id))
<< "No app installed for site: " << static_cast<int>(site);

NavigateTabbedBrowserToSite(GetInScopeURL(site), NavigationMode::kNewTab);
Expand Down Expand Up @@ -1314,7 +1315,7 @@ void WebAppIntegrationTestDriver::LaunchFromMenuOption(Site site) {
if (!BeforeStateChangeAction(__FUNCTION__))
return;
AppId app_id = GetAppIdBySiteMode(site);
ASSERT_TRUE(provider()->registrar().GetAppById(app_id))
ASSERT_TRUE(provider()->registrar_unsafe().GetAppById(app_id))
<< "No app installed for site: " << static_cast<int>(site);

NavigateTabbedBrowserToSite(GetInScopeURL(site), NavigationMode::kNewTab);
Expand All @@ -1336,10 +1337,10 @@ void WebAppIntegrationTestDriver::LaunchFromPlatformShortcut(Site site) {
if (!BeforeStateChangeAction(__FUNCTION__))
return;
AppId app_id = GetAppIdBySiteMode(site);
ASSERT_TRUE(provider()->registrar().GetAppById(app_id))
ASSERT_TRUE(provider()->registrar_unsafe().GetAppById(app_id))
<< "No app installed for site: " << static_cast<int>(site);

WebAppRegistrar& app_registrar = provider()->registrar();
WebAppRegistrar& app_registrar = provider()->registrar_unsafe();
DisplayMode display_mode = app_registrar.GetAppEffectiveDisplayMode(app_id);
bool is_open_in_app_browser =
(display_mode != blink::mojom::DisplayMode::kBrowser);
Expand Down Expand Up @@ -1385,10 +1386,10 @@ void WebAppIntegrationTestDriver::LaunchFromAppShimFallback(Site site) {
return;

AppId app_id = GetAppIdBySiteMode(site);
ASSERT_TRUE(provider()->registrar().GetAppById(app_id))
ASSERT_TRUE(provider()->registrar_unsafe().GetAppById(app_id))
<< "No app installed for site: " << static_cast<int>(site);

WebAppRegistrar& app_registrar = provider()->registrar();
WebAppRegistrar& app_registrar = provider()->registrar_unsafe();
DisplayMode display_mode = app_registrar.GetAppEffectiveDisplayMode(app_id);
bool is_open_in_app_browser =
(display_mode != blink::mojom::DisplayMode::kBrowser);
Expand Down Expand Up @@ -1458,7 +1459,7 @@ void WebAppIntegrationTestDriver::OpenAppSettingsFromChromeApps(Site site) {
if (!BeforeStateChangeAction(__FUNCTION__))
return;
AppId app_id = GetAppIdBySiteMode(site);
ASSERT_TRUE(provider()->registrar().GetAppById(app_id))
ASSERT_TRUE(provider()->registrar_unsafe().GetAppById(app_id))
<< "No app installed for site: " << static_cast<int>(site);
;

Expand Down Expand Up @@ -1486,7 +1487,7 @@ void WebAppIntegrationTestDriver::CreateShortcutsFromList(Site site) {
if (!BeforeStateChangeAction(__FUNCTION__))
return;
AppId app_id = GetAppIdBySiteMode(site);
ASSERT_TRUE(provider()->registrar().GetAppById(app_id))
ASSERT_TRUE(provider()->registrar_unsafe().GetAppById(app_id))
<< "No app installed for site: " << static_cast<int>(site);
content::TestWebUI test_web_ui;
content::WebContents* web_contents =
Expand Down Expand Up @@ -1523,7 +1524,7 @@ void WebAppIntegrationTestDriver::DeletePlatformShortcut(Site site) {
return;
base::ScopedAllowBlockingForTesting allow_blocking;
AppId app_id = GetAppIdBySiteMode(site);
std::string app_name = provider()->registrar().GetAppShortName(app_id);
std::string app_name = provider()->registrar_unsafe().GetAppShortName(app_id);
if (app_name.empty()) {
app_name = GetSiteConfiguration(site).app_name;
}
Expand Down Expand Up @@ -1754,7 +1755,7 @@ void WebAppIntegrationTestDriver::SetOpenInTab(Site site) {
if (!BeforeStateChangeAction(__FUNCTION__))
return;
AppId app_id = GetAppIdBySiteMode(site);
ASSERT_TRUE(provider()->registrar().GetAppById(app_id))
ASSERT_TRUE(provider()->registrar_unsafe().GetAppById(app_id))
<< "No app installed for site: " << static_cast<int>(site);
;
// Will need to add feature flag based condition for web app settings page
Expand All @@ -1774,7 +1775,7 @@ void WebAppIntegrationTestDriver::SetOpenInWindow(Site site) {
if (!BeforeStateChangeAction(__FUNCTION__))
return;
AppId app_id = GetAppIdBySiteMode(site);
ASSERT_TRUE(provider()->registrar().GetAppById(app_id))
ASSERT_TRUE(provider()->registrar_unsafe().GetAppById(app_id))
<< "No app installed for site: " << static_cast<int>(site);
;
// Will need to add feature flag based condition for web app settings page.
Expand Down Expand Up @@ -1861,7 +1862,7 @@ void WebAppIntegrationTestDriver::UninstallFromList(Site site) {
if (!BeforeStateChangeAction(__FUNCTION__))
return;
AppId app_id = GetAppIdBySiteMode(site);
ASSERT_TRUE(provider()->registrar().GetAppById(app_id))
ASSERT_TRUE(provider()->registrar_unsafe().GetAppById(app_id))
<< "No app installed for site: " << static_cast<int>(site);

UninstallCompleteWaiter uninstall_waiter(profile(), app_id);
Expand Down Expand Up @@ -1909,7 +1910,7 @@ void WebAppIntegrationTestDriver::UninstallFromAppSettings(Site site) {
if (!BeforeStateChangeAction(__FUNCTION__))
return;
AppId app_id = GetAppIdBySiteMode(site);
ASSERT_TRUE(provider()->registrar().GetAppById(app_id))
ASSERT_TRUE(provider()->registrar_unsafe().GetAppById(app_id))
<< "No app installed for site: " << static_cast<int>(site);

UninstallCompleteWaiter uninstall_waiter(profile(), app_id);
Expand Down Expand Up @@ -1946,7 +1947,7 @@ void WebAppIntegrationTestDriver::UninstallFromMenu(Site site) {
if (!BeforeStateChangeAction(__FUNCTION__))
return;
AppId app_id = GetAppIdBySiteMode(site);
ASSERT_TRUE(provider()->registrar().GetAppById(app_id))
ASSERT_TRUE(provider()->registrar_unsafe().GetAppById(app_id))
<< "No app installed for site: " << static_cast<int>(site);

UninstallCompleteWaiter uninstall_waiter(profile(), app_id);
Expand Down Expand Up @@ -2010,7 +2011,7 @@ void WebAppIntegrationTestDriver::UninstallPolicyApp(Site site) {
ASSERT_GT(removed_count, 0U);
}
run_loop.Run();
const WebApp* app = provider()->registrar().GetAppById(policy_app->id);
const WebApp* app = provider()->registrar_unsafe().GetAppById(policy_app->id);
// If the app was fully uninstalled, wait for the change to propagate through
// App Service.
if (app == nullptr)
Expand All @@ -2024,7 +2025,7 @@ void WebAppIntegrationTestDriver::UninstallFromOs(Site site) {
if (!BeforeStateChangeAction(__FUNCTION__))
return;
AppId app_id = GetAppIdBySiteMode(site);
ASSERT_TRUE(provider()->registrar().GetAppById(app_id))
ASSERT_TRUE(provider()->registrar_unsafe().GetAppById(app_id))
<< "No app installed for site: " << static_cast<int>(site);

UninstallCompleteWaiter uninstall_waiter(profile(), app_id);
Expand Down Expand Up @@ -2159,7 +2160,7 @@ void WebAppIntegrationTestDriver::CheckAppNavigationIsStartUrl() {
ASSERT_TRUE(app_browser());
GURL url =
app_browser()->tab_strip_model()->GetActiveWebContents()->GetVisibleURL();
EXPECT_EQ(url, provider()->registrar().GetAppStartUrl(active_app_id_));
EXPECT_EQ(url, provider()->registrar_unsafe().GetAppStartUrl(active_app_id_));
AfterStateCheckAction();
}

Expand All @@ -2178,7 +2179,7 @@ void WebAppIntegrationTestDriver::CheckBrowserNavigationIsAppSettings(
if (!BeforeStateCheckAction(__FUNCTION__))
return;
AppId app_id = GetAppIdBySiteMode(site);
ASSERT_TRUE(provider()->registrar().GetAppById(app_id))
ASSERT_TRUE(provider()->registrar_unsafe().GetAppById(app_id))
<< "No app installed for site: " << static_cast<int>(site);
;

Expand Down Expand Up @@ -2739,7 +2740,7 @@ void WebAppIntegrationTestDriver::AfterStateChangeAction() {
auto* provider = GetProviderForProfile(profile);
if (!provider)
continue;
std::vector<AppId> app_ids = provider->registrar().GetAppIds();
std::vector<AppId> app_ids = provider->registrar_unsafe().GetAppIds();
for (auto& app_id : app_ids) {
// Wait for any shims to finish connecting.
auto* app_shim_manager = apps::AppShimManager::Get();
Expand All @@ -2760,7 +2761,8 @@ void WebAppIntegrationTestDriver::AfterStateChangeAction() {
for (const auto& [app_id, open_browsers] : open_browsers_per_app) {
if (open_browsers != 0)
continue;
std::string app_name = provider()->registrar().GetAppShortName(app_id);
std::string app_name =
provider()->registrar_unsafe().GetAppShortName(app_id);
base::FilePath app_path = GetShortcutPath(
override_registration_->shortcut_override->chrome_apps_folder.GetPath(),
app_name, app_id);
Expand Down Expand Up @@ -2912,7 +2914,7 @@ WebAppIntegrationTestDriver::ConstructStateSnapshot() {
WebAppProvider* provider = GetProviderForProfile(profile);
base::flat_map<AppId, AppState> app_state;
if (provider) {
WebAppRegistrar& registrar = provider->registrar();
WebAppRegistrar& registrar = provider->registrar_unsafe();
auto app_ids = registrar.GetAppIds();
for (const auto& app_id : app_ids) {
std::string manifest_launcher_icon_filename;
Expand Down Expand Up @@ -3060,7 +3062,7 @@ void WebAppIntegrationTestDriver::UninstallPolicyAppById(const AppId& id) {
run_loop.Quit();
}));

const WebApp* web_app = provider()->registrar().GetAppById(id);
const WebApp* web_app = provider()->registrar_unsafe().GetAppById(id);

base::flat_set<GURL> install_urls;
{
Expand All @@ -3082,7 +3084,7 @@ void WebAppIntegrationTestDriver::UninstallPolicyAppById(const AppId& id) {
ASSERT_GT(removed_count, 0U);
}
run_loop.Run();
const WebApp* app = provider()->registrar().GetAppById(id);
const WebApp* app = provider()->registrar_unsafe().GetAppById(id);
// If the app was fully uninstalled, wait for the change to propagate through
// App Service.
if (app == nullptr)
Expand Down Expand Up @@ -3275,7 +3277,7 @@ void WebAppIntegrationTestDriver::SetFileHandlingEnabled(Site site,
bool enabled) {
#if !BUILDFLAG(IS_CHROMEOS)
AppId app_id = GetAppIdBySiteMode(site);
ASSERT_TRUE(provider()->registrar().GetAppById(app_id))
ASSERT_TRUE(provider()->registrar_unsafe().GetAppById(app_id))
<< "No app installed for site: " << static_cast<int>(site);
auto app_management_page_handler = CreateAppManagementPageHandler(profile());
app_management_page_handler.SetFileHandlingEnabled(app_id, enabled);
Expand Down Expand Up @@ -3313,7 +3315,7 @@ void WebAppIntegrationTestDriver::SetRunOnOsLoginMode(
apps::RunOnOsLoginMode login_mode) {
#if !BUILDFLAG(IS_CHROMEOS)
AppId app_id = GetAppIdBySiteMode(site);
ASSERT_TRUE(provider()->registrar().GetAppById(app_id))
ASSERT_TRUE(provider()->registrar_unsafe().GetAppById(app_id))
<< "No app installed for site: " << static_cast<int>(site);
auto app_management_page_handler = CreateAppManagementPageHandler(profile());
app_management_page_handler.SetRunOnOsLoginMode(app_id, login_mode);
Expand Down
Expand Up @@ -105,7 +105,7 @@ class WebAppTabStripBrowserTest : public InProcessBrowserTest {
}

WebAppRegistrar& registrar() {
return WebAppProvider::GetForTest(browser()->profile())->registrar();
return WebAppProvider::GetForTest(browser()->profile())->registrar_unsafe();
}

private:
Expand Down
Expand Up @@ -66,7 +66,7 @@ WebAppUninstallDialogDelegateView::WebAppUninstallDialogDelegateView(
auto* provider = web_app::WebAppProvider::GetForWebApps(profile_);
DCHECK(provider);

app_start_url_ = provider->registrar().GetAppStartUrl(app_id_);
app_start_url_ = provider->registrar_unsafe().GetAppStartUrl(app_id_);
DCHECK(!app_start_url_.is_empty());
DCHECK(app_start_url_.is_valid());

Expand All @@ -84,7 +84,8 @@ WebAppUninstallDialogDelegateView::WebAppUninstallDialogDelegateView(
SetShowIcon(true);
SetTitle(l10n_util::GetStringFUTF16(
IDS_EXTENSION_PROMPT_UNINSTALL_TITLE,
base::UTF8ToUTF16(provider->registrar().GetAppShortName(app_id_))));
base::UTF8ToUTF16(
provider->registrar_unsafe().GetAppShortName(app_id_))));

SetButtonLabel(
ui::DIALOG_BUTTON_OK,
Expand All @@ -111,7 +112,7 @@ WebAppUninstallDialogDelegateView::WebAppUninstallDialogDelegateView(

// For IWAs checkbox will not be displayed, removal of
// storage is automatically enforced.
if (!provider->registrar().IsIsolated(app_id_)) {
if (!provider->registrar_unsafe().IsIsolated(app_id_)) {
std::u16string checkbox_label = l10n_util::GetStringFUTF16(
IDS_EXTENSION_UNINSTALL_PROMPT_REMOVE_DATA_CHECKBOX,
url_formatter::FormatUrlForSecurityDisplay(
Expand All @@ -136,7 +137,7 @@ void WebAppUninstallDialogDelegateView::OnDialogAccepted() {

auto* provider = web_app::WebAppProvider::GetForWebApps(profile_);
DCHECK(provider);
bool is_isolated_web_app = provider->registrar().IsIsolated(app_id_);
bool is_isolated_web_app = provider->registrar_unsafe().IsIsolated(app_id_);

HistogramCloseAction action =
is_isolated_web_app || (checkbox_ && checkbox_->GetChecked())
Expand Down Expand Up @@ -251,7 +252,7 @@ void WebAppUninstallDialogViews::ConfirmUninstall(

provider->icon_manager().ReadIcons(
app_id, IconPurpose::ANY,
provider->registrar().GetAppDownloadedIconSizesAny(app_id),
provider->registrar_unsafe().GetAppDownloadedIconSizesAny(app_id),
base::BindOnce(&WebAppUninstallDialogViews::OnIconsRead,
weak_ptr_factory_.GetWeakPtr(), uninstall_source));
}
Expand Down

0 comments on commit a7a5c33

Please sign in to comment.