Commit 1acd6cea authored by Junyi Xiao's avatar Junyi Xiao Committed by Commit Bot

Revert "WebApps: Remove install pwa icon from toolbar when page crashed"

This reverts commit 402e8644.

Reason for revert: This changes caused an new bug and a perf regression on startup. 

Back it out for now, and will try to fix the original bug appropriately later.

Original change's description:
> WebApps: Remove install pwa icon from toolbar when page crashed
> 
> This change 1. hides the install pwa page action icon in the toolbar
> 2. grayed out "create shortcut" and "Install {pwaName}" menu items
> when page crashed.
> 
> Bug: 1057526, 1059934, 1059172
> 
> Change-Id: I63d5e3607e9aff71ab2bd799d792f025b142ec42
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2078771
> Commit-Queue: Junyi Xiao <juxiao@microsoft.com>
> Reviewed-by: Peter Kasting <pkasting@chromium.org>
> Reviewed-by: Alan Cutter <alancutter@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#746305}

TBR=pkasting@chromium.org,alancutter@chromium.org,mjackson@microsoft.com,lomitch@microsoft.com,juxiao@microsoft.com,sunggch@microsoft.com

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1057526
Change-Id: I69d4e20f4b50c12a56abade08eaf0843c3764d08
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2097540
Commit-Queue: Lorne Mitchell <lomitch@microsoft.com>
Reviewed-by: default avatarLorne Mitchell <lomitch@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#749294}
parent 5ac26e64
...@@ -1686,9 +1686,6 @@ void Browser::NavigationStateChanged(WebContents* source, ...@@ -1686,9 +1686,6 @@ void Browser::NavigationStateChanged(WebContents* source,
content::INVALIDATE_TYPE_TAB)) content::INVALIDATE_TYPE_TAB))
command_controller_->TabStateChanged(); command_controller_->TabStateChanged();
if (changed_flags & content::INVALIDATE_TYPE_TAB)
UpdateToolbar(false);
if (app_controller_) if (app_controller_)
app_controller_->UpdateCustomTabBarVisibility(true); app_controller_->UpdateCustomTabBarVisibility(true);
} }
......
...@@ -36,11 +36,6 @@ void PwaInstallView::UpdateImpl() { ...@@ -36,11 +36,6 @@ void PwaInstallView::UpdateImpl() {
if (!web_contents) if (!web_contents)
return; return;
if (web_contents->IsCrashed()) {
SetVisible(false);
return;
}
auto* manager = banners::AppBannerManager::FromWebContents(web_contents); auto* manager = banners::AppBannerManager::FromWebContents(web_contents);
// May not be present e.g. in incognito mode. // May not be present e.g. in incognito mode.
if (!manager) if (!manager)
......
...@@ -297,18 +297,6 @@ IN_PROC_BROWSER_TEST_P(PwaInstallViewBrowserTest, ...@@ -297,18 +297,6 @@ IN_PROC_BROWSER_TEST_P(PwaInstallViewBrowserTest,
EXPECT_FALSE(pwa_install_view_->GetVisible()); EXPECT_FALSE(pwa_install_view_->GetVisible());
} }
// Tests that the plus icon updates its visibility when tab crashes
IN_PROC_BROWSER_TEST_P(PwaInstallViewBrowserTest,
IconVisibilityAfterTabCrashed) {
StartNavigateToUrl(GetInstallableAppURL());
ASSERT_TRUE(app_banner_manager_->WaitForInstallableCheck());
EXPECT_TRUE(pwa_install_view_->GetVisible());
web_contents_->SetIsCrashed(base::TERMINATION_STATUS_PROCESS_CRASHED, -1);
ASSERT_TRUE(web_contents_->IsCrashed());
EXPECT_FALSE(pwa_install_view_->GetVisible());
}
// Tests that the plus icon updates its visibility once the installability check // Tests that the plus icon updates its visibility once the installability check
// completes. // completes.
IN_PROC_BROWSER_TEST_P(PwaInstallViewBrowserTest, IN_PROC_BROWSER_TEST_P(PwaInstallViewBrowserTest,
......
...@@ -535,22 +535,6 @@ IN_PROC_BROWSER_TEST_P(WebAppBrowserTest, ...@@ -535,22 +535,6 @@ IN_PROC_BROWSER_TEST_P(WebAppBrowserTest,
EXPECT_EQ(GetAppMenuCommandState(IDC_INSTALL_PWA, browser()), kEnabled); EXPECT_EQ(GetAppMenuCommandState(IDC_INSTALL_PWA, browser()), kEnabled);
} }
// Tests that both installing a PWA and creating a shortcut app are disabled
// when page crashes.
IN_PROC_BROWSER_TEST_P(WebAppBrowserTest, ShortcutMenuOptionsForCrashedTab) {
ASSERT_TRUE(https_server()->Start());
EXPECT_TRUE(
NavigateAndAwaitInstallabilityCheck(browser(), GetInstallableAppURL()));
content::WebContents* tab_contents =
browser()->tab_strip_model()->GetActiveWebContents();
tab_contents->SetIsCrashed(base::TERMINATION_STATUS_PROCESS_CRASHED, -1);
ASSERT_TRUE(tab_contents->IsCrashed());
EXPECT_EQ(GetAppMenuCommandState(IDC_CREATE_SHORTCUT, browser()), kDisabled);
EXPECT_EQ(GetAppMenuCommandState(IDC_INSTALL_PWA, browser()), kDisabled);
}
// Tests that an installed PWA is not used when out of scope by one path level. // Tests that an installed PWA is not used when out of scope by one path level.
IN_PROC_BROWSER_TEST_P(WebAppBrowserTest, MenuOptionsOutsideInstalledPwaScope) { IN_PROC_BROWSER_TEST_P(WebAppBrowserTest, MenuOptionsOutsideInstalledPwaScope) {
ASSERT_TRUE(https_server()->Start()); ASSERT_TRUE(https_server()->Start());
......
...@@ -69,22 +69,22 @@ void OnWebAppInstalled(WebAppInstalledCallback callback, ...@@ -69,22 +69,22 @@ void OnWebAppInstalled(WebAppInstalledCallback callback,
bool CanCreateWebApp(const Browser* browser) { bool CanCreateWebApp(const Browser* browser) {
content::WebContents* web_contents = content::WebContents* web_contents =
browser->tab_strip_model()->GetActiveWebContents(); browser->tab_strip_model()->GetActiveWebContents();
if (!WebAppProvider::Get(browser->profile())) if (!WebAppProvider::GetForWebContents(web_contents))
return false;
if (web_contents->IsCrashed())
return false; return false;
content::NavigationEntry* entry = content::NavigationEntry* entry =
web_contents->GetController().GetLastCommittedEntry(); web_contents->GetController().GetLastCommittedEntry();
if (entry && entry->GetPageType() == content::PAGE_TYPE_ERROR) bool is_error_page =
return false; entry && entry->GetPageType() == content::PAGE_TYPE_ERROR;
Profile* web_contents_profile =
Profile::FromBrowserContext(web_contents->GetBrowserContext());
banners::AppBannerManager* app_banner_manager = banners::AppBannerManager* app_banner_manager =
banners::AppBannerManager::FromWebContents(web_contents); banners::AppBannerManager::FromWebContents(web_contents);
bool externally_installed = bool externally_installed =
app_banner_manager && app_banner_manager->IsExternallyInstalledWebApp(); app_banner_manager && app_banner_manager->IsExternallyInstalledWebApp();
return AreWebAppsUserInstallable(browser->profile()) && return AreWebAppsUserInstallable(web_contents_profile) &&
IsValidWebAppUrl(web_contents->GetLastCommittedURL()) && IsValidWebAppUrl(web_contents->GetLastCommittedURL()) &&
!externally_installed; !is_error_page && !externally_installed;
} }
bool CanPopOutWebApp(Profile* profile) { bool CanPopOutWebApp(Profile* profile) {
......
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