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

This is a reland of 402e8644

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
> 
> 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}

Bug: 1057526
Change-Id: I2911dca52a8fbd2f2f0c461489823f2087328e9a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2100022
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@{#759299}

chrome/browser/ui/browser.cc

@@ -2645,10 +2645,15 @@ void Browser::ProcessPendingUIUpdates() {
           TabChangeType::kAll);
     }
 
-    // Update the bookmark bar. It may happen that the tab is crashed, and if
-    // so, the bookmark bar should be hidden.
-    if (flags & content::INVALIDATE_TYPE_TAB)
+    // Update the bookmark bar and PWA install icon. It may happen that the tab
+    // is crashed, and if so, the bookmark bar and PWA install icon should be
+    // hidden.
+    if (flags & content::INVALIDATE_TYPE_TAB) {
       UpdateBookmarkBarState(BOOKMARK_BAR_STATE_CHANGE_TAB_STATE);
+      // TODO(crbug.com/1062235): Ideally, we should simply ask the state to
+      // update, and doing that in an appropriate and efficient manner.
+      window()->UpdatePageActionIcon(PageActionIconType::kPwaInstall);
+    }
 
     // We don't need to process INVALIDATE_STATE, since that's not visible.
   }

chrome/browser/ui/views/page_action/pwa_install_view.cc

@@ -36,6 +36,11 @@ void PwaInstallView::UpdateImpl() {
   if (!web_contents)
     return;
 
+  if (web_contents->IsCrashed()) {
+    SetVisible(false);
+    return;
+  }
+
   auto* manager = banners::AppBannerManager::FromWebContents(web_contents);
   // May not be present e.g. in incognito mode.
   if (!manager)

chrome/browser/ui/views/page_action/pwa_install_view_browsertest.cc

@@ -37,6 +37,7 @@
 #include "net/test/embedded_test_server/http_response.h"
 #include "services/network/public/cpp/network_switches.h"
 #include "ui/gfx/color_utils.h"
+#include "ui/views/view_observer.h"
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/arc/arc_util.h"
@@ -46,6 +47,38 @@
 #include "components/arc/test/fake_app_instance.h"
 #endif  // defined(OS_CHROMEOS)
 
+namespace {
+
+class PwaInstallIconChangeWaiter : public views::ViewObserver {
+ public:
+  static void VerifyIconVisibility(views::View* iconView, bool visible) {
+    if (visible != iconView->GetVisible())
+      PwaInstallIconChangeWaiter(iconView).run_loop_.Run();
+
+    EXPECT_EQ(visible, iconView->GetVisible());
+  }
+
+ private:
+  explicit PwaInstallIconChangeWaiter(views::View* view) {
+    observed_.Add(view);
+  }
+  ~PwaInstallIconChangeWaiter() override = default;
+
+  // ViewObserver
+  void OnViewVisibilityChanged(views::View* observed_view,
+                               views::View* starting_view) override {
+    run_loop_.Quit();
+  }
+
+  base::RunLoop run_loop_;
+
+  ScopedObserver<views::View, views::ViewObserver> observed_{this};
+
+  DISALLOW_COPY_AND_ASSIGN(PwaInstallIconChangeWaiter);
+};
+
+}  // namespace
+
 class PwaInstallViewBrowserTest
     : public extensions::ExtensionBrowserTest,
       public ::testing::WithParamInterface<web_app::ProviderType> {
@@ -296,6 +329,18 @@ IN_PROC_BROWSER_TEST_P(PwaInstallViewBrowserTest,
   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());
+  PwaInstallIconChangeWaiter::VerifyIconVisibility(pwa_install_view_, false);
+}
+
 // Tests that the plus icon updates its visibility once the installability check
 // completes.
 IN_PROC_BROWSER_TEST_P(PwaInstallViewBrowserTest,

chrome/browser/ui/web_applications/web_app_browsertest.cc

@@ -534,6 +534,20 @@ IN_PROC_BROWSER_TEST_P(WebAppBrowserTest,
   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) {
+  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.
 IN_PROC_BROWSER_TEST_P(WebAppBrowserTest, MenuOptionsOutsideInstalledPwaScope) {
   NavigateToURLAndWait(

chrome/browser/ui/web_applications/web_app_dialog_utils.cc

@@ -67,24 +67,30 @@ void OnWebAppInstalled(WebAppInstalledCallback callback,
 }  // namespace
 
 bool CanCreateWebApp(const Browser* browser) {
+  // Check whether user is allowed to install web app.
+  if (!WebAppProvider::Get(browser->profile()) ||
+      !AreWebAppsUserInstallable(browser->profile()))
+    return false;
+
+  // Check whether we're able to install the current page as an app.
   content::WebContents* web_contents =
       browser->tab_strip_model()->GetActiveWebContents();
-  if (!WebAppProvider::GetForWebContents(web_contents))
+  if (!IsValidWebAppUrl(web_contents->GetLastCommittedURL()) ||
+      web_contents->IsCrashed())
     return false;
   content::NavigationEntry* entry =
       web_contents->GetController().GetLastCommittedEntry();
-  bool is_error_page =
-      entry && entry->GetPageType() == content::PAGE_TYPE_ERROR;
-  Profile* web_contents_profile =
-      Profile::FromBrowserContext(web_contents->GetBrowserContext());
+  if (entry && entry->GetPageType() == content::PAGE_TYPE_ERROR)
+    return false;
+
+  // Check whether the app is externally installed.
   banners::AppBannerManager* app_banner_manager =
       banners::AppBannerManager::FromWebContents(web_contents);
-  bool externally_installed =
-      app_banner_manager && app_banner_manager->IsExternallyInstalledWebApp();
 
-  return AreWebAppsUserInstallable(web_contents_profile) &&
-         IsValidWebAppUrl(web_contents->GetLastCommittedURL()) &&
-         !is_error_page && !externally_installed;
+  if (app_banner_manager && app_banner_manager->IsExternallyInstalledWebApp())
+    return false;
+
+  return true;
 }
 
 bool CanPopOutWebApp(Profile* profile) {