[desktop-pwas] Remove 'Open as window' checkbox from bookmark app dialog.

This CL removes the 'Open as window' checkbox from the bookmark app
dialog when DesktopPWAWindowing is enabled and chooses to open as a
window if the site is an installable PWA and as a tab otherwise.

This requires adding an InstallableManager to the BookmarkAppHelper to
determine if the site is an installable PWA.

Bug: 729922
Change-Id: I30d6329d52a9fabe314e6a8b76aa46e209cd171f
Reviewed-on: https://chromium-review.googlesource.com/700137Reviewed-by: Ben Wells <benwells@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: calamity <calamity@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512761}

chrome/browser/banners/app_banner_infobar_delegate_desktop.cc

@@ -99,10 +99,8 @@ bool AppBannerInfoBarDelegateDesktop::Accept() {
   TrackUserResponse(USER_RESPONSE_WEB_APP_ACCEPTED);
   has_user_interaction_ = true;
 
-  bookmark_app_helper_->CreateFromAppBanner(
-      base::Bind(&AppBannerManager::DidFinishCreatingBookmarkApp,
-                 weak_manager_),
-      manifest_url_, manifest_);
+  bookmark_app_helper_->Create(base::Bind(
+      &AppBannerManager::DidFinishCreatingBookmarkApp, weak_manager_));
   return true;
 }
 

chrome/browser/extensions/bookmark_app_helper.cc

@@ -10,6 +10,7 @@
 #include <string>
 
 #include "base/command_line.h"
+#include "base/feature_list.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/utf_string_conversions.h"
@@ -25,6 +26,9 @@
 #include "chrome/browser/extensions/favicon_downloader.h"
 #include "chrome/browser/extensions/launch_util.h"
 #include "chrome/browser/extensions/tab_helper.h"
+#include "chrome/browser/installable/installable_data.h"
+#include "chrome/browser/installable/installable_manager.h"
+#include "chrome/browser/installable/installable_params.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/app_list/app_list_service.h"
 #include "chrome/browser/ui/app_list/app_list_util.h"
@@ -33,6 +37,7 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/web_applications/web_app.h"
 #include "chrome/browser/webshare/share_target_pref_helper.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/common/extensions/api/url_handlers/url_handlers_parser.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
@@ -533,14 +538,20 @@ BookmarkAppHelper::BookmarkAppHelper(Profile* profile,
     : profile_(profile),
       contents_(contents),
       web_app_info_(web_app_info),
-      crx_installer_(
-          extensions::CrxInstaller::CreateSilent(ExtensionSystem::Get(profile)
-                                                     ->extension_service())),
+      crx_installer_(extensions::CrxInstaller::CreateSilent(
+          ExtensionSystem::Get(profile)->extension_service())),
       weak_factory_(this) {
-  web_app_info_.open_as_window =
-      profile_->GetPrefs()->GetInteger(
-          extensions::pref_names::kBookmarkAppCreationLaunchType) ==
-      extensions::LAUNCH_TYPE_WINDOW;
+  if (contents)
+    installable_manager_ = InstallableManager::FromWebContents(contents);
+
+  // Use the last bookmark app creation type. The launch container is decided by
+  // the system for desktop PWAs.
+  if (!base::FeatureList::IsEnabled(features::kDesktopPWAWindowing)) {
+    web_app_info_.open_as_window =
+        profile_->GetPrefs()->GetInteger(
+            extensions::pref_names::kBookmarkAppCreationLaunchType) ==
+        extensions::LAUNCH_TYPE_WINDOW;
+  }
 
   // The default app title is the page title, which can be quite long. Limit the
   // default name used to something sensible.
@@ -569,51 +580,66 @@ void BookmarkAppHelper::Create(const CreateBookmarkAppCallback& callback) {
   // Do not fetch the manifest for extension URLs.
   if (contents_ &&
       !contents_->GetVisibleURL().SchemeIs(extensions::kExtensionScheme)) {
-    contents_->GetManifest(base::Bind(&BookmarkAppHelper::OnDidGetManifest,
-                                      weak_factory_.GetWeakPtr()));
+    // Null in tests. OnDidPerformInstallableCheck is called via a testing API.
+    if (installable_manager_) {
+      InstallableParams params;
+      params.valid_primary_icon = true;
+      params.valid_manifest = true;
+      // Do not wait for a service worker if it doesn't exist.
+      params.has_worker = true;
+      installable_manager_->GetData(
+          params, base::Bind(&BookmarkAppHelper::OnDidPerformInstallableCheck,
+                             weak_factory_.GetWeakPtr()));
+    }
   } else {
     OnIconsDownloaded(true, std::map<GURL, std::vector<SkBitmap>>());
   }
 }
 
-void BookmarkAppHelper::CreateFromAppBanner(
-    const CreateBookmarkAppCallback& callback,
-    const GURL& manifest_url,
-    const content::Manifest& manifest) {
-  DCHECK(!manifest.short_name.is_null() || !manifest.name.is_null());
-  DCHECK(manifest.start_url.is_valid());
-
-  callback_ = callback;
-  OnDidGetManifest(manifest_url, manifest);
-}
-
-void BookmarkAppHelper::OnDidGetManifest(const GURL& manifest_url,
-                                         const content::Manifest& manifest) {
-  DCHECK(manifest_url.is_valid() || manifest.IsEmpty());
+void BookmarkAppHelper::OnDidPerformInstallableCheck(
+    const InstallableData& data) {
+  DCHECK(data.manifest_url.is_valid() || data.manifest->IsEmpty());
 
   if (contents_->IsBeingDestroyed())
     return;
 
-  UpdateWebAppInfoFromManifest(manifest, &web_app_info_);
+  installable_ =
+      data.error_code == NO_ERROR_DETECTED ? INSTALLABLE_YES : INSTALLABLE_NO;
+
+  UpdateWebAppInfoFromManifest(*data.manifest, &web_app_info_);
 
   // TODO(mgiuca): Web Share Target should have its own flag, rather than using
   // the experimental-web-platform-features flag. https://crbug.com/736178.
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableExperimentalWebPlatformFeatures)) {
-    UpdateShareTargetInPrefs(manifest_url, manifest, profile_->GetPrefs());
+    UpdateShareTargetInPrefs(data.manifest_url, *data.manifest,
+                             profile_->GetPrefs());
   }
 
   VLOG(1) << "Preparing to download bookmark app icons";
-  // Add urls from the WebApplicationInfo.
+  // Add icon urls to download from the WebApplicationInfo.
   std::vector<GURL> web_app_info_icon_urls;
-  for (std::vector<WebApplicationInfo::IconInfo>::const_iterator it =
-           web_app_info_.icons.begin();
-       it != web_app_info_.icons.end();
-       ++it) {
-    if (it->url.is_valid()) {
-      VLOG(1) << "Adding icon to download: " << it->url.spec();
-      web_app_info_icon_urls.push_back(it->url);
-    }
+  for (auto& info : web_app_info_.icons) {
+    if (!info.url.is_valid())
+      continue;
+
+    // Skip downloading icon if we already have it from the InstallableManager.
+    if (info.url == data.primary_icon_url && data.primary_icon)
+      continue;
+
+    VLOG(1) << "Adding icon to download: " << info.url.spec();
+    web_app_info_icon_urls.push_back(info.url);
+  }
+
+  // Add the primary icon to the final bookmark app creation data.
+  if (data.primary_icon_url.is_valid()) {
+    WebApplicationInfo::IconInfo primary_icon_info;
+    const SkBitmap& icon = *data.primary_icon;
+    primary_icon_info.url = data.primary_icon_url;
+    primary_icon_info.data = icon;
+    primary_icon_info.width = icon.width();
+    primary_icon_info.height = icon.height();
+    web_app_info_.icons.push_back(primary_icon_info);
   }
 
   favicon_downloader_.reset(
@@ -622,7 +648,7 @@ void BookmarkAppHelper::OnDidGetManifest(const GURL& manifest_url,
                                        weak_factory_.GetWeakPtr())));
 
   // If the manifest specified icons, don't use the page icons.
-  if (!manifest.icons.empty())
+  if (!data.manifest->icons.empty())
     favicon_downloader_->SkipPageFavicons();
 
   favicon_downloader_->Start();
@@ -686,6 +712,7 @@ void BookmarkAppHelper::OnIconsDownloaded(
     OnBubbleCompleted(true, web_app_info_);
     return;
   }
+
   chrome::ShowBookmarkAppDialog(
       browser->window()->GetNativeWindow(), web_app_info_,
       base::Bind(&BookmarkAppHelper::OnBubbleCompleted,
@@ -709,6 +736,13 @@ void BookmarkAppHelper::FinishInstallation(const Extension* extension) {
   extensions::LaunchType launch_type = web_app_info_.open_as_window
                                            ? extensions::LAUNCH_TYPE_WINDOW
                                            : extensions::LAUNCH_TYPE_REGULAR;
+
+  if (base::FeatureList::IsEnabled(features::kDesktopPWAWindowing)) {
+    DCHECK_NE(INSTALLABLE_UNKNOWN, installable_);
+    launch_type = installable_ == INSTALLABLE_YES
+                      ? extensions::LAUNCH_TYPE_WINDOW
+                      : extensions::LAUNCH_TYPE_REGULAR;
+  }
   profile_->GetPrefs()->SetInteger(
       extensions::pref_names::kBookmarkAppCreationLaunchType, launch_type);
 

chrome/browser/extensions/bookmark_app_helper.h

@@ -20,6 +20,8 @@
 
 class ExtensionService;
 class FaviconDownloader;
+struct InstallableData;
+class InstallableManager;
 class Profile;
 class SkBitmap;
 
@@ -105,26 +107,29 @@ class BookmarkAppHelper : public content::NotificationObserver {
   // Begins the asynchronous bookmark app creation.
   void Create(const CreateBookmarkAppCallback& callback);
 
-  // Begins the asynchronous bookmark app creation from an app banner.
-  void CreateFromAppBanner(const CreateBookmarkAppCallback& callback,
-                           const GURL& manifest_url,
-                           const content::Manifest& manifest);
-
  protected:
   // Protected methods for testing.
 
-  // Called by the WebContents when the manifest has been downloaded. If there
-  // is no manifest, or the WebContents is destroyed before the manifest could
-  // be downloaded, this is called with an empty manifest.
-  void OnDidGetManifest(const GURL& manifest_url,
-                        const content::Manifest& manifest);
+  // Called by the InstallableManager when the installability check is
+  // completed.
+  void OnDidPerformInstallableCheck(const InstallableData& data);
 
   // Performs post icon download tasks including installing the bookmark app.
   virtual void OnIconsDownloaded(
       bool success,
       const std::map<GURL, std::vector<SkBitmap>>& bitmaps);
 
+  // Downloads icons from the given WebApplicationInfo using the given
+  // WebContents.
+  std::unique_ptr<FaviconDownloader> favicon_downloader_;
+
  private:
+  enum Installable {
+    INSTALLABLE_YES,
+    INSTALLABLE_NO,
+    INSTALLABLE_UNKNOWN,
+  };
+
   // Called after the bubble has been shown, and the user has either accepted or
   // the dialog was dismissed.
   void OnBubbleCompleted(bool user_accepted,
@@ -151,15 +156,15 @@ class BookmarkAppHelper : public content::NotificationObserver {
   // Called on app creation or failure.
   CreateBookmarkAppCallback callback_;
 
-  // Downloads icons from the given WebApplicationInfo using the given
-  // WebContents.
-  std::unique_ptr<FaviconDownloader> favicon_downloader_;
-
   // Used to install the created bookmark app.
   scoped_refptr<extensions::CrxInstaller> crx_installer_;
 
   content::NotificationRegistrar registrar_;
 
+  InstallableManager* installable_manager_;
+
+  Installable installable_ = INSTALLABLE_UNKNOWN;
+
   // With fast tab unloading enabled, shutting down can cause BookmarkAppHelper
   // to be destroyed before the bookmark creation bubble. Use weak pointers to
   // prevent a heap-use-after free in this instance (https://crbug.com/534994).

chrome/browser/extensions/bookmark_app_helper_unittest.cc

@@ -7,10 +7,15 @@
 #include "base/macros.h"
 #include "base/run_loop.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/scoped_feature_list.h"
 #include "chrome/browser/banners/app_banner_settings_helper.h"
 #include "chrome/browser/extensions/convert_web_app.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_service_test_base.h"
+#include "chrome/browser/extensions/favicon_downloader.h"
+#include "chrome/browser/extensions/launch_util.h"
+#include "chrome/browser/installable/installable_data.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/common/extensions/manifest_handlers/app_launch_info.h"
 #include "chrome/common/extensions/manifest_handlers/app_theme_color_info.h"
 #include "chrome/test/base/testing_profile.h"
@@ -18,6 +23,7 @@
 #include "content/public/browser/web_contents.h"
 #include "content/public/test/test_utils.h"
 #include "content/public/test/web_contents_tester.h"
+#include "extensions/browser/extension_prefs.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/common/constants.h"
 #include "extensions/common/extension_icon_set.h"
@@ -277,7 +283,8 @@ class TestBookmarkAppHelper : public BookmarkAppHelper {
   TestBookmarkAppHelper(ExtensionService* service,
                         WebApplicationInfo web_app_info,
                         content::WebContents* contents)
-      : BookmarkAppHelper(service->profile(), web_app_info, contents) {}
+      : BookmarkAppHelper(service->profile(), web_app_info, contents),
+        bitmap_(CreateSquareBitmapWithColor(32, SK_ColorRED)) {}
 
   ~TestBookmarkAppHelper() override {}
 
@@ -286,9 +293,21 @@ class TestBookmarkAppHelper : public BookmarkAppHelper {
     extension_ = extension;
   }
 
-  void CompleteGetManifest(const char* manifest_url,
-                           const content::Manifest& manifest) {
-    BookmarkAppHelper::OnDidGetManifest(GURL(manifest_url), manifest);
+  void CompleteInstallableCheck(const char* manifest_url,
+                                const content::Manifest& manifest,
+                                bool installable) {
+    InstallableData data = {
+        installable ? NO_ERROR_DETECTED : MANIFEST_DISPLAY_NOT_SUPPORTED,
+        GURL(manifest_url),
+        &manifest,
+        GURL(kAppIconURL1),
+        &bitmap_,
+        GURL(),
+        nullptr,
+        installable,
+        installable,
+    };
+    BookmarkAppHelper::OnDidPerformInstallableCheck(data);
   }
 
   void CompleteIconDownload(
@@ -299,8 +318,13 @@ class TestBookmarkAppHelper : public BookmarkAppHelper {
 
   const Extension* extension() { return extension_; }
 
+  const FaviconDownloader* favicon_downloader() {
+    return favicon_downloader_.get();
+  }
+
  private:
   const Extension* extension_;
+  SkBitmap bitmap_;
 
   DISALLOW_COPY_AND_ASSIGN(TestBookmarkAppHelper);
 };
@@ -358,7 +382,7 @@ TEST_F(BookmarkAppHelperExtensionServiceTest, CreateBookmarkAppWithManifest) {
   manifest.name = base::NullableString16(base::UTF8ToUTF16(kAppTitle), false);
   manifest.scope = GURL(kAppScope);
   manifest.theme_color = SK_ColorBLUE;
-  helper.CompleteGetManifest(kManifestUrl, manifest);
+  helper.CompleteInstallableCheck(kManifestUrl, manifest, true);
 
   std::map<GURL, std::vector<SkBitmap> > icon_map;
   helper.CompleteIconDownload(true, icon_map);
@@ -381,6 +405,98 @@ TEST_F(BookmarkAppHelperExtensionServiceTest, CreateBookmarkAppWithManifest) {
           .is_null());
 }
 
+TEST_F(BookmarkAppHelperExtensionServiceTest,
+       CreateBookmarkAppWithManifestIcons) {
+  WebApplicationInfo web_app_info;
+  std::unique_ptr<content::WebContents> contents(
+      content::WebContentsTester::CreateTestWebContents(profile(), nullptr));
+  TestBookmarkAppHelper helper(service_, web_app_info, contents.get());
+  helper.Create(base::Bind(&TestBookmarkAppHelper::CreationComplete,
+                           base::Unretained(&helper)));
+
+  content::Manifest manifest;
+  manifest.start_url = GURL(kAppUrl);
+  manifest.name = base::NullableString16(base::UTF8ToUTF16(kAppTitle), false);
+  manifest.scope = GURL(kAppScope);
+  content::Manifest::Icon icon;
+  icon.src = GURL(kAppIconURL1);
+  manifest.icons.push_back(icon);
+  icon.src = GURL(kAppIconURL2);
+  manifest.icons.push_back(icon);
+  helper.CompleteInstallableCheck(kManifestUrl, manifest, true);
+
+  // Favicon URLs are ignored because the site has a manifest with icons.
+  EXPECT_FALSE(helper.favicon_downloader()->need_favicon_urls_);
+
+  // Only 1 icon should be downloading since the other was provided by the
+  // InstallableManager.
+  EXPECT_EQ(1u, helper.favicon_downloader()->in_progress_requests_.size());
+
+  std::map<GURL, std::vector<SkBitmap>> icon_map;
+  icon_map[GURL(kAppIconURL2)].push_back(
+      CreateSquareBitmapWithColor(kIconSizeSmall, SK_ColorRED));
+  helper.CompleteIconDownload(true, icon_map);
+
+  content::RunAllTasksUntilIdle();
+  EXPECT_TRUE(helper.extension());
+  const Extension* extension =
+      service_->GetInstalledExtension(helper.extension()->id());
+  EXPECT_TRUE(extension);
+  EXPECT_EQ(1u, registry()->enabled_extensions().size());
+  EXPECT_TRUE(extension->from_bookmark());
+  EXPECT_EQ(kAppTitle, extension->name());
+  EXPECT_EQ(GURL(kAppUrl), AppLaunchInfo::GetLaunchWebURL(extension));
+  EXPECT_EQ(GURL(kAppScope), GetScopeURLFromBookmarkApp(extension));
+}
+
+TEST_F(BookmarkAppHelperExtensionServiceTest,
+       CreateBookmarkAppDefaultLauncherContainers) {
+  auto scoped_feature_list = std::make_unique<base::test::ScopedFeatureList>();
+  scoped_feature_list->InitAndEnableFeature(features::kDesktopPWAWindowing);
+
+  WebApplicationInfo web_app_info;
+  std::map<GURL, std::vector<SkBitmap>> icon_map;
+
+  std::unique_ptr<content::WebContents> contents(
+      content::WebContentsTester::CreateTestWebContents(profile(), nullptr));
+  content::Manifest manifest;
+  manifest.start_url = GURL(kAppUrl);
+  manifest.name = base::NullableString16(base::UTF8ToUTF16(kAppTitle), false);
+  manifest.scope = GURL(kAppScope);
+  {
+    TestBookmarkAppHelper helper(service_, web_app_info, contents.get());
+    helper.Create(base::Bind(&TestBookmarkAppHelper::CreationComplete,
+                             base::Unretained(&helper)));
+
+    helper.CompleteInstallableCheck(kManifestUrl, manifest, true);
+    helper.CompleteIconDownload(true, icon_map);
+
+    content::RunAllTasksUntilIdle();
+    EXPECT_TRUE(helper.extension());
+    const Extension* extension =
+        service_->GetInstalledExtension(helper.extension()->id());
+    EXPECT_TRUE(extension);
+    EXPECT_EQ(LAUNCH_CONTAINER_WINDOW,
+              GetLaunchContainer(ExtensionPrefs::Get(profile()), extension));
+  }
+  {
+    TestBookmarkAppHelper helper(service_, web_app_info, contents.get());
+    helper.Create(base::Bind(&TestBookmarkAppHelper::CreationComplete,
+                             base::Unretained(&helper)));
+
+    helper.CompleteInstallableCheck(kManifestUrl, manifest, false);
+    helper.CompleteIconDownload(true, icon_map);
+
+    content::RunAllTasksUntilIdle();
+    EXPECT_TRUE(helper.extension());
+    const Extension* extension =
+        service_->GetInstalledExtension(helper.extension()->id());
+    EXPECT_TRUE(extension);
+    EXPECT_EQ(LAUNCH_CONTAINER_TAB,
+              GetLaunchContainer(ExtensionPrefs::Get(profile()), extension));
+  }
+}
+
 TEST_F(BookmarkAppHelperExtensionServiceTest,
        CreateBookmarkAppWithManifestNoScope) {
   WebApplicationInfo web_app_info;
@@ -394,7 +510,7 @@ TEST_F(BookmarkAppHelperExtensionServiceTest,
   content::Manifest manifest;
   manifest.start_url = GURL(kAppUrl);
   manifest.name = base::NullableString16(base::UTF8ToUTF16(kAppTitle), false);
-  helper.CompleteGetManifest(kManifestUrl, manifest);
+  helper.CompleteInstallableCheck(kManifestUrl, manifest, true);
 
   std::map<GURL, std::vector<SkBitmap>> icon_map;
   helper.CompleteIconDownload(true, icon_map);
@@ -419,7 +535,7 @@ TEST_F(BookmarkAppHelperExtensionServiceTest,
   helper.Create(base::Bind(&TestBookmarkAppHelper::CreationComplete,
                            base::Unretained(&helper)));
 
-  helper.CompleteGetManifest(kManifestUrl, content::Manifest());
+  helper.CompleteInstallableCheck(kManifestUrl, content::Manifest(), false);
   std::map<GURL, std::vector<SkBitmap>> icon_map;
   helper.CompleteIconDownload(true, icon_map);
   content::RunAllTasksUntilIdle();

chrome/browser/extensions/favicon_downloader.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/extensions/favicon_downloader.h"
 
 #include "base/bind.h"
+#include "base/message_loop/message_loop.h"
 #include "components/favicon/content/content_favicon_driver.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/web_contents.h"
@@ -31,9 +32,9 @@ void FaviconDownloader::SkipPageFavicons() {
 }
 
 void FaviconDownloader::Start() {
-  FetchIcons(extra_favicon_urls_);
   // If the candidates aren't loaded, icons will be fetched when
   // DidUpdateFaviconURL() is called.
+  FetchIcons(extra_favicon_urls_);
 
   if (need_favicon_urls_) {
     std::vector<content::FaviconURL> favicon_tab_helper_urls =
@@ -91,8 +92,10 @@ void FaviconDownloader::FetchIcons(const std::vector<GURL>& urls) {
 
   // If no downloads were initiated, we can proceed directly to running the
   // callback.
-  if (in_progress_requests_.empty() && !need_favicon_urls_)
-    callback_.Run(true, favicon_map_);
+  if (in_progress_requests_.empty() && !need_favicon_urls_) {
+    base::MessageLoop::current()->task_runner()->PostTask(
+        FROM_HERE, base::BindOnce(callback_, true, favicon_map_));
+  }
 }
 
 void FaviconDownloader::DidDownloadFavicon(

chrome/browser/extensions/favicon_downloader.h

@@ -10,6 +10,7 @@
 #include <vector>
 
 #include "base/callback.h"
+#include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "content/public/browser/web_contents_observer.h"
@@ -20,6 +21,11 @@ namespace content {
 struct FaviconURL;
 }
 
+namespace extensions {
+FORWARD_DECLARE_TEST(BookmarkAppHelperExtensionServiceTest,
+                     CreateBookmarkAppWithManifestIcons);
+}
+
 namespace gfx {
 class Size;
 }
@@ -49,6 +55,8 @@ class FaviconDownloader : public content::WebContentsObserver {
 
  private:
   friend class TestFaviconDownloader;
+  FRIEND_TEST_ALL_PREFIXES(extensions::BookmarkAppHelperExtensionServiceTest,
+                           CreateBookmarkAppWithManifestIcons);
 
   // Initiates a download of the image at |url| and returns the download id.
   // This is overridden in testing.

chrome/browser/ui/views/extensions/bookmark_app_confirmation_view.cc

@@ -5,12 +5,14 @@
 #include "chrome/browser/ui/views/extensions/bookmark_app_confirmation_view.h"
 
 #include "base/callback_helpers.h"
+#include "base/feature_list.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/string16.h"
 #include "base/strings/string_util.h"
 #include "build/build_config.h"
 #include "chrome/browser/extensions/extension_util.h"
 #include "chrome/browser/ui/views/harmony/chrome_layout_provider.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/grit/generated_resources.h"
 #include "components/constrained_window/constrained_window_views.h"
 #include "components/strings/grit/components_strings.h"
@@ -105,7 +107,9 @@ BookmarkAppConfirmationView::BookmarkAppConfirmationView(
 
   // When CanHostedAppsOpenInWindows() returns false, do not show the open as
   // window checkbox to avoid confusing users.
-  if (extensions::util::CanHostedAppsOpenInWindows()) {
+  // Desktop PWAs will choose the display mode automatically.
+  if (extensions::util::CanHostedAppsOpenInWindows() &&
+      !base::FeatureList::IsEnabled(features::kDesktopPWAWindowing)) {
     open_as_window_checkbox_ = new views::Checkbox(
         l10n_util::GetStringUTF16(IDS_BOOKMARK_APP_BUBBLE_OPEN_AS_WINDOW));
     open_as_window_checkbox_->SetChecked(web_app_info_.open_as_window);