Delete/inline methods for PWA create/open menu items.

Split out from subsequent CL crrev.com/c/1897398 plus surrounding
cleanup.
Mostly refactoring except:
-remove deprecated metric logging.
-diverge when the "Open in PWA Window" menu item is enabled.
  Previously this used the same CanCreateWebApp check as "Create
  Shortcut" and "Install <PWA>".
  The presence of this item is already controlled separately (by
  checking whether the page is in scope of a PWA) so most of the checks
  in CanCreateWebApp are redundant. The relevant checks are within
  AreWebAppsUserInstallable (but just calling that method is answering
  a different question). So copied body of AreWebAppsUserInstallable.
  Subsequent CL will modify CanCreateWebApp.

Change-Id: Ib2a32b74637be89b6ac230f4d046e31316512680
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1989739
Commit-Queue: Glen Robertson <glenrob@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#729655}

chrome/browser/ui/browser_command_controller.cc

@@ -42,6 +42,7 @@
 #include "chrome/browser/ui/singleton_tabs.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/web_applications/app_browser_controller.h"
+#include "chrome/browser/ui/web_applications/web_app_dialog_utils.h"
 #include "chrome/browser/ui/web_applications/web_app_launch_utils.h"
 #include "chrome/browser/ui/webui/inspect_ui.h"
 #include "chrome/common/content_restriction.h"
@@ -639,13 +640,13 @@ bool BrowserCommandController::ExecuteCommandWithDisposition(
       break;
     case IDC_CREATE_SHORTCUT:
       base::RecordAction(base::UserMetricsAction("CreateShortcut"));
-      CreateBookmarkAppFromCurrentWebContents(browser_,
-                                              true /* force_shortcut_app */);
+      web_app::CreateWebAppFromCurrentWebContents(
+          browser_, true /* force_shortcut_app */);
       break;
     case IDC_INSTALL_PWA:
       base::RecordAction(base::UserMetricsAction("InstallWebAppFromMenu"));
-      CreateBookmarkAppFromCurrentWebContents(browser_,
-                                              false /* force_shortcut_app */);
+      web_app::CreateWebAppFromCurrentWebContents(
+          browser_, false /* force_shortcut_app */);
       break;
     case IDC_DEV_TOOLS:
       ToggleDevToolsWindow(browser_, DevToolsToggleAction::Show());
@@ -1157,13 +1158,14 @@ void BrowserCommandController::UpdateCommandsForTabState() {
   if (browser_->is_type_devtools())
     command_updater_.UpdateCommandEnabled(IDC_OPEN_FILE, false);
 
-  bool can_create_bookmark_app = CanCreateBookmarkApp(browser_);
-  command_updater_.UpdateCommandEnabled(IDC_INSTALL_PWA,
-                                        can_create_bookmark_app);
+  bool can_create_web_app = web_app::CanCreateWebApp(browser_);
+  command_updater_.UpdateCommandEnabled(IDC_INSTALL_PWA, can_create_web_app);
   command_updater_.UpdateCommandEnabled(IDC_CREATE_SHORTCUT,
-                                        can_create_bookmark_app);
+                                        can_create_web_app);
+  // Note that additional logic in AppMenuModel::Build() controls the presence
+  // of this command.
   command_updater_.UpdateCommandEnabled(IDC_OPEN_IN_PWA_WINDOW,
-                                        can_create_bookmark_app);
+                                        web_app::CanPopOutWebApp(profile()));
 
   command_updater_.UpdateCommandEnabled(
       IDC_TOGGLE_REQUEST_TABLET_SITE,

chrome/browser/ui/browser_commands.cc

@@ -109,7 +109,6 @@
 #include "chrome/browser/ui/extensions/app_launch_params.h"
 #include "chrome/browser/ui/extensions/application_launch.h"
 #include "chrome/browser/ui/extensions/settings_api_bubble_helpers.h"
-#include "chrome/browser/ui/web_applications/web_app_dialog_utils.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "chrome/common/extensions/extension_metrics.h"
 #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
@@ -1430,20 +1429,6 @@ bool CanViewSource(const Browser* browser) {
                                              .CanViewSource();
 }
 
-#if BUILDFLAG(ENABLE_EXTENSIONS)
-void CreateBookmarkAppFromCurrentWebContents(Browser* browser,
-                                             bool force_shortcut_app) {
-  // TODO(alancutter): Legacy metric to remove in ~M80.
-  base::RecordAction(UserMetricsAction("CreateHostedApp"));
-  web_app::CreateWebAppFromCurrentWebContents(browser, force_shortcut_app,
-                                              base::DoNothing());
-}
-
-bool CanCreateBookmarkApp(const Browser* browser) {
-  return web_app::CanCreateWebApp(browser);
-}
-#endif  // BUILDFLAG(ENABLE_EXTENSIONS)
-
 #if !defined(TOOLKIT_VIEWS)
 base::Optional<int> GetKeyboardFocusedTabIndex(const Browser* browser) {
   return base::nullopt;

chrome/browser/ui/browser_commands.h

@@ -191,14 +191,6 @@ void CopyURL(Browser* browser);
 Browser* OpenInChrome(Browser* hosted_app_browser);
 bool CanViewSource(const Browser* browser);
 
-// Initiates user flow for creating a bookmark app for the current page.
-// Will install a PWA hosted app if the site meets installability requirements
-// (see |AppBannerManager::PerformInstallableCheck|) unless |force_shortcut_app|
-// is true.
-void CreateBookmarkAppFromCurrentWebContents(Browser* browser,
-                                             bool force_shortcut_app);
-bool CanCreateBookmarkApp(const Browser* browser);
-
 base::Optional<int> GetKeyboardFocusedTabIndex(const Browser* browser);
 
 }  // namespace chrome

chrome/browser/ui/web_applications/web_app_dialog_utils.cc

@@ -13,11 +13,13 @@
 #include "base/feature_list.h"
 #include "base/no_destructor.h"
 #include "chrome/browser/installable/installable_metrics.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_dialogs.h"
 #include "chrome/browser/web_applications/components/install_manager.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/components/web_app_install_utils.h"
+#include "chrome/browser/web_applications/components/web_app_utils.h"
 #include "chrome/browser/web_applications/web_app_provider.h"
 #include "chrome/common/chrome_features.h"
 #include "chrome/common/web_application_info.h"
@@ -65,17 +67,22 @@ void OnWebAppInstalled(WebAppInstalledCallback callback,
 bool CanCreateWebApp(const Browser* browser) {
   content::WebContents* web_contents =
       browser->tab_strip_model()->GetActiveWebContents();
-  auto* provider = WebAppProvider::GetForWebContents(web_contents);
-  if (!provider)
+  if (!WebAppProvider::GetForWebContents(web_contents))
     return false;
+  Profile* web_contents_profile =
+      Profile::FromBrowserContext(web_contents->GetBrowserContext());
 
-  return provider->install_manager().CanInstallWebApp(web_contents);
+  return AreWebAppsUserInstallable(web_contents_profile) &&
+         IsValidWebAppUrl(web_contents->GetLastCommittedURL());
 }
 
-void CreateWebAppFromCurrentWebContents(
-    Browser* browser,
-    bool force_shortcut_app,
-    WebAppInstalledCallback installed_callback) {
+bool CanPopOutWebApp(Profile* profile) {
+  return AreWebAppsEnabled(profile) && !profile->IsGuestSession() &&
+         !profile->IsOffTheRecord();
+}
+
+void CreateWebAppFromCurrentWebContents(Browser* browser,
+                                        bool force_shortcut_app) {
   DCHECK(CanCreateWebApp(browser));
 
   content::WebContents* web_contents =
@@ -89,7 +96,8 @@ void CreateWebAppFromCurrentWebContents(
   provider->install_manager().InstallWebAppFromManifestWithFallback(
       web_contents, force_shortcut_app, install_source,
       base::BindOnce(WebAppInstallDialogCallback, install_source),
-      base::BindOnce(OnWebAppInstalled, std::move(installed_callback)));
+      base::BindOnce(OnWebAppInstalled,
+                     base::DoNothing::Once<const AppId&, InstallResultCode>()));
 }
 
 bool CreateWebAppFromManifest(content::WebContents* web_contents,

chrome/browser/ui/web_applications/web_app_dialog_utils.h

@@ -10,6 +10,7 @@
 
 enum class WebappInstallSource;
 class Browser;
+class Profile;
 
 namespace content {
 class WebContents;
@@ -22,17 +23,22 @@ enum class InstallResultCode;
 // TODO(loyso): Rework these functions (API). Move all of them into
 // WebAppDialogManager.
 
-// Returns true if a WebApp installation is allowed for the current page.
+// Returns whether a WebApp installation is allowed for the current page.
 bool CanCreateWebApp(const Browser* browser);
 
+// Returns whether the current profile is allowed to pop out a web app into a
+// separate window. Does not check whether any particular page can pop out.
+bool CanPopOutWebApp(Profile* profile);
+
 using WebAppInstalledCallback =
     base::OnceCallback<void(const AppId& app_id, InstallResultCode code)>;
 
-// Initiates install of a WebApp for the current page.
-void CreateWebAppFromCurrentWebContents(
-    Browser* browser,
-    bool force_shortcut_app,
-    WebAppInstalledCallback installed_callback);
+// Initiates user install of a WebApp for the current page.
+// If |force_shortcut_app| is true, the current page will be installed even if
+// the site does not meet installability requirements (see
+// |AppBannerManager::PerformInstallableCheck|).
+void CreateWebAppFromCurrentWebContents(Browser* browser,
+                                        bool force_shortcut_app);
 
 // Starts install of a WebApp for a given |web_contents|, initiated from
 // a promotional banner or omnibox install icon.

chrome/browser/web_applications/components/install_manager.h

@@ -67,9 +67,6 @@ class InstallManager {
       InstallableCheckResult result,
       base::Optional<AppId> app_id)>;
 
-  // Returns true if a web app can be installed for a given |web_contents|.
-  virtual bool CanInstallWebApp(content::WebContents* web_contents) = 0;
-
   // Checks a WebApp installability, retrieves manifest and icons and
   // than performs the actual installation.
   virtual void InstallWebAppFromManifest(

chrome/browser/web_applications/web_app_install_manager.cc

@@ -32,15 +32,6 @@ WebAppInstallManager::WebAppInstallManager(Profile* profile)
 
 WebAppInstallManager::~WebAppInstallManager() = default;
 
-bool WebAppInstallManager::CanInstallWebApp(
-    content::WebContents* web_contents) {
-  Profile* web_contents_profile =
-      Profile::FromBrowserContext(web_contents->GetBrowserContext());
-
-  return AreWebAppsUserInstallable(web_contents_profile) &&
-         IsValidWebAppUrl(web_contents->GetLastCommittedURL());
-}
-
 void WebAppInstallManager::LoadWebAppAndCheckInstallability(
     const GURL& web_app_url,
     WebappInstallSource install_source,

chrome/browser/web_applications/web_app_install_manager.h

@@ -37,7 +37,6 @@ class WebAppInstallManager final : public InstallManager,
   ~WebAppInstallManager() override;
 
   // InstallManager:
-  bool CanInstallWebApp(content::WebContents* web_contents) override;
   void LoadWebAppAndCheckInstallability(
       const GURL& web_app_url,
       WebappInstallSource install_source,