Desktop PWAs: SetAppPrefsForWebContents supports BMO

HostedAppBrowserController static methods SetAppPrefsForWebContents
and ClearAppPrefsForWebContents move out to become standalone
methods in web_app_launch_utils.h/cc

They are now also called by WebAppBrowserController and
WebAppLaunchManager.

The test PWAMixedContentBrowserTest, MixedContentInPWA now passes
with the DesktopPWAsUnifiedUiController flag enabled.

The test previously failed as SetAppPrefsForWebContents was
not called when the DesktopPWAsUnifiedUiController flag and
the DesktopPWAsUnifiedUiController flag were used.

TBR=benwells@chromium.org

Bug: 1034361
Change-Id: Id388122efa147ae8ae4ee26ba87a3c7c5101e5ea
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2035819
Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#738505}

chrome/browser/apps/launch_service/launch_service.cc

@@ -45,6 +45,10 @@ LaunchService::~LaunchService() {}
 
 LaunchManager& LaunchService::GetLaunchManagerForApp(
     const std::string& app_id) {
+  // --app old-style app shortcuts
+  if (app_id.empty())
+    return *extension_app_launch_manager_;
+
   const extensions::Extension* extension =
       extensions::ExtensionRegistry::Get(profile_)->GetInstalledExtension(
           app_id);

chrome/browser/ui/extensions/application_launch.cc

@@ -30,8 +30,8 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/extensions/extension_enable_flow.h"
 #include "chrome/browser/ui/extensions/extension_enable_flow_delegate.h"
-#include "chrome/browser/ui/extensions/hosted_app_browser_controller.h"
 #include "chrome/browser/ui/web_applications/system_web_app_ui_utils.h"
+#include "chrome/browser/ui/web_applications/web_app_launch_utils.h"
 #include "chrome/browser/web_applications/components/file_handler_manager.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "chrome/browser/web_applications/components/web_app_provider_base.h"
@@ -436,13 +436,13 @@ WebContents* NavigateApplicationWindow(Browser* browser,
   nav_params.disposition = disposition;
   Navigate(&nav_params);
 
-  WebContents* web_contents = nav_params.navigated_or_inserted_contents;
+  WebContents* const web_contents = nav_params.navigated_or_inserted_contents;
 
-  // TODO(https://crbug.com/1034361):
-  // Once WebAppBrowserController implements TabInserted and TabRemoved, remove
-  // the following call.
-  extensions::HostedAppBrowserController::SetAppPrefsForWebContents(
-      browser->app_controller(), web_contents);
+  if (extension && !extension->from_bookmark()) {
+    extensions::TabHelper::FromWebContents(web_contents)
+        ->SetExtensionApp(extension);
+  }
+  web_app::SetAppPrefsForWebContents(web_contents);
 
   // TODO(https://crbug.com/1032443):
   // Eventually move this to browser_navigator.cc: CreateTargetContents().

chrome/browser/ui/extensions/hosted_app_browser_controller.cc

@@ -15,6 +15,7 @@
 #include "chrome/browser/ui/browser_window_state.h"
 #include "chrome/browser/ui/location_bar/location_bar.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/ui/web_applications/web_app_launch_utils.h"
 #include "chrome/browser/web_applications/components/app_registrar.h"
 #include "chrome/browser/web_applications/components/install_finalizer.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
@@ -56,38 +57,6 @@ bool IsSameHostAndPort(const GURL& app_url, const GURL& page_url) {
 
 }  // namespace
 
-// static
-void HostedAppBrowserController::SetAppPrefsForWebContents(
-    web_app::AppBrowserController* controller,
-    content::WebContents* web_contents) {
-  web_contents->GetMutableRendererPrefs()->can_accept_load_drops = false;
-  web_contents->SyncRendererPrefs();
-
-  if (!controller)
-    return;
-
-  // All hosted apps should specify an app ID.
-  DCHECK(controller->HasAppId());
-  extensions::TabHelper::FromWebContents(web_contents)
-      ->SetExtensionApp(ExtensionRegistry::Get(controller->browser()->profile())
-                            ->GetExtensionById(controller->GetAppId(),
-                                               ExtensionRegistry::EVERYTHING));
-
-  web_contents->NotifyPreferencesChanged();
-}
-
-// static
-void HostedAppBrowserController::ClearAppPrefsForWebContents(
-    content::WebContents* web_contents) {
-  web_contents->GetMutableRendererPrefs()->can_accept_load_drops = true;
-  web_contents->SyncRendererPrefs();
-
-  extensions::TabHelper::FromWebContents(web_contents)
-      ->SetExtensionApp(nullptr);
-
-  web_contents->NotifyPreferencesChanged();
-}
-
 HostedAppBrowserController::HostedAppBrowserController(Browser* browser)
     : AppBrowserController(
           browser,
@@ -275,13 +244,19 @@ void HostedAppBrowserController::OnReceivedInitialURL() {
 
 void HostedAppBrowserController::OnTabInserted(content::WebContents* contents) {
   AppBrowserController::OnTabInserted(contents);
-  extensions::HostedAppBrowserController::SetAppPrefsForWebContents(this,
-                                                                    contents);
+
+  const Extension* extension = GetExtension();
+  if (extension && extension->from_bookmark())
+    extension = nullptr;
+  extensions::TabHelper::FromWebContents(contents)->SetExtensionApp(extension);
+  web_app::SetAppPrefsForWebContents(contents);
 }
 
 void HostedAppBrowserController::OnTabRemoved(content::WebContents* contents) {
   AppBrowserController::OnTabRemoved(contents);
-  extensions::HostedAppBrowserController::ClearAppPrefsForWebContents(contents);
+
+  extensions::TabHelper::FromWebContents(contents)->SetExtensionApp(nullptr);
+  web_app::ClearAppPrefsForWebContents(contents);
 }
 
 }  // namespace extensions

chrome/browser/ui/extensions/hosted_app_browser_controller.h

@@ -31,14 +31,6 @@ class Extension;
 class HostedAppBrowserController : public web_app::AppBrowserController,
                                    public ExtensionUninstallDialog::Delegate {
  public:
-  // Functions to set preferences that are unique to app windows.
-  static void SetAppPrefsForWebContents(
-      web_app::AppBrowserController* controller,
-      content::WebContents* web_contents);
-
-  // Clear preferences that are unique to app windows.
-  static void ClearAppPrefsForWebContents(content::WebContents* web_contents);
-
   explicit HostedAppBrowserController(Browser* browser);
   ~HostedAppBrowserController() override;
 

chrome/browser/ui/web_applications/pwa_mixed_content_browsertest.cc

@@ -64,12 +64,12 @@ IN_PROC_BROWSER_TEST_P(PWAMixedContentBrowserTest,
   EXPECT_EQ(GetAppMenuCommandState(IDC_INSTALL_PWA, browser()), kNotPresent);
 }
 
-// TODO(crbug.com/1026080): Also test kUnifiedControllerWithWebApp.
 INSTANTIATE_TEST_SUITE_P(
     All,
     PWAMixedContentBrowserTest,
     ::testing::Values(ControllerType::kHostedAppController,
-                      ControllerType::kUnifiedControllerWithBookmarkApp),
+                      ControllerType::kUnifiedControllerWithBookmarkApp,
+                      ControllerType::kUnifiedControllerWithWebApp),
     ControllerTypeParamToString);
 
 }  // namespace web_app

chrome/browser/ui/web_applications/web_app_browser_controller.cc

@@ -8,6 +8,7 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/web_applications/web_app_dialog_manager.h"
+#include "chrome/browser/ui/web_applications/web_app_launch_utils.h"
 #include "chrome/browser/ui/web_applications/web_app_ui_manager_impl.h"
 #include "chrome/browser/web_applications/components/app_icon_manager.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
@@ -134,6 +135,16 @@ bool WebAppBrowserController::IsInstalled() const {
   return registrar().IsInstalled(GetAppId());
 }
 
+void WebAppBrowserController::OnTabInserted(content::WebContents* contents) {
+  AppBrowserController::OnTabInserted(contents);
+  web_app::SetAppPrefsForWebContents(contents);
+}
+
+void WebAppBrowserController::OnTabRemoved(content::WebContents* contents) {
+  AppBrowserController::OnTabRemoved(contents);
+  web_app::ClearAppPrefsForWebContents(contents);
+}
+
 const AppRegistrar& WebAppBrowserController::registrar() const {
   return provider_.registrar();
 }

chrome/browser/ui/web_applications/web_app_browser_controller.h

@@ -62,6 +62,11 @@ class WebAppBrowserController : public AppBrowserController,
 
   void SetReadIconCallbackForTesting(base::OnceClosure callback);
 
+ protected:
+  // web_app::AppBrowserController:
+  void OnTabInserted(content::WebContents* contents) override;
+  void OnTabRemoved(content::WebContents* contents) override;
+
  private:
   const AppRegistrar& registrar() const;
 

chrome/browser/ui/web_applications/web_app_launch_manager.cc

@@ -16,6 +16,7 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/web_applications/system_web_app_ui_utils.h"
+#include "chrome/browser/ui/web_applications/web_app_launch_utils.h"
 #include "chrome/browser/web_applications/components/file_handler_manager.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
 #include "chrome/browser/web_applications/components/web_app_helpers.h"
@@ -80,10 +81,12 @@ content::WebContents* NavigateWebApplicationWindow(
   nav_params.disposition = disposition;
   Navigate(&nav_params);
 
-  content::WebContents* web_contents =
+  content::WebContents* const web_contents =
       nav_params.navigated_or_inserted_contents;
 
   SetTabHelperAppId(web_contents, app_id);
+  web_app::SetAppPrefsForWebContents(web_contents);
+
   return web_contents;
 }
 

chrome/browser/ui/web_applications/web_app_launch_utils.cc

@@ -6,6 +6,7 @@
 
 #include "base/feature_list.h"
 #include "base/strings/string_util.h"
+#include "chrome/browser/extensions/tab_helper.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
@@ -21,6 +22,9 @@
 #include "content/public/browser/navigation_controller.h"
 #include "content/public/browser/navigation_entry.h"
 #include "content/public/browser/web_contents.h"
+#include "extensions/browser/extension_registry.h"
+#include "extensions/common/extension.h"
+#include "third_party/blink/public/mojom/renderer_preferences.mojom.h"
 #include "url/gurl.h"
 
 namespace {
@@ -146,4 +150,18 @@ Browser* ReparentWebContentsForFocusMode(content::WebContents* contents) {
   return ReparentWebContentsWithBrowserCreateParams(contents, browser_params);
 }
 
+void SetAppPrefsForWebContents(content::WebContents* web_contents) {
+  web_contents->GetMutableRendererPrefs()->can_accept_load_drops = false;
+  web_contents->SyncRendererPrefs();
+
+  web_contents->NotifyPreferencesChanged();
+}
+
+void ClearAppPrefsForWebContents(content::WebContents* web_contents) {
+  web_contents->GetMutableRendererPrefs()->can_accept_load_drops = true;
+  web_contents->SyncRendererPrefs();
+
+  web_contents->NotifyPreferencesChanged();
+}
+
 }  // namespace web_app

chrome/browser/ui/web_applications/web_app_launch_utils.h

@@ -37,6 +37,12 @@ Browser* ReparentWebContentsIntoAppBrowser(content::WebContents* contents,
 // Reparents contents to a new app browser when entering the Focus Mode.
 Browser* ReparentWebContentsForFocusMode(content::WebContents* contents);
 
+// Set preferences that are unique to app windows.
+void SetAppPrefsForWebContents(content::WebContents* web_contents);
+
+// Clear preferences that are unique to app windows.
+void ClearAppPrefsForWebContents(content::WebContents* web_contents);
+
 }  // namespace web_app
 
 #endif  // CHROME_BROWSER_UI_WEB_APPLICATIONS_WEB_APP_LAUNCH_UTILS_H_

chrome/common/chrome_features.cc

@@ -274,8 +274,8 @@ const base::Feature kDesktopPWAsUnifiedUiController{
 // on extensions unconditionally use the new launch manager.)
 // TODO(crbug.com/877898): Enable and delete this feature flag before
 // kDesktopPWAsWithoutExtensions launch.
-const base::Feature kDesktopPWAsUnifiedLaunch{
-    "DesktopPWAsUnifiedLaunch", base::FEATURE_DISABLED_BY_DEFAULT};
+const base::Feature kDesktopPWAsUnifiedLaunch{"DesktopPWAsUnifiedLaunch",
+                                              base::FEATURE_ENABLED_BY_DEFAULT};
 
 // Enables or disables usage of shared LevelDB instance (ModelTypeStoreService).
 // If this flag is disabled, the new Web Apps system uses its own isolated