Make PWAs and Shortcut apps have the same WebKitPrefs scope

This CL removes one difference between promotable web apps and
"Create Shortcut" web apps where Blink's understanding of the app's
scope was disabled for shortcut apps.

This makes promotable web apps and shortcut apps behave the same for
autoplay policy, keyboard events in app windows and picture in picture
mode (see uses of Document::IsInWebAppScope()). This is desirable
because promotable/non-promotable web apps are effectively identical to
the user so the fewer hidden behavioural differences the better.

Bug: 910016
Change-Id: I4d1e29566520369436b58ecdd1ba2eaefa17de31
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2041379
Commit-Queue: Alan Cutter <alancutter@chromium.org>
Reviewed-by: Glen Robertson <glenrob@chromium.org>
Cr-Commit-Position: refs/heads/master@{#740528}

chrome/browser/chrome_content_browser_client.cc

@@ -160,7 +160,7 @@
 #include "chrome/browser/usb/usb_tab_helper.h"
 #include "chrome/browser/vr/vr_tab_helper.h"
 #include "chrome/browser/web_applications/components/app_registrar.h"
-#include "chrome/browser/web_applications/web_app_provider.h"
+#include "chrome/browser/web_applications/components/web_app_provider_base.h"
 #include "chrome/common/buildflags.h"
 #include "chrome/common/channel_info.h"
 #include "chrome/common/chrome_constants.h"
@@ -3159,20 +3159,12 @@ void ChromeContentBrowserClient::OverrideWebkitPrefs(
       // WebContents.
       Browser* browser = chrome::FindBrowserWithWebContents(contents);
       if (browser && browser->app_controller() &&
-          browser->app_controller()->CreatedForInstalledPwa()) {
-        // PWAs should be hosted apps.
-        DCHECK(browser->app_controller()->IsHostedApp());
-        // HostedApps that are PWAs are always created through WebAppProvider
-        // for profiles that support them, so we should always be able to
-        // retrieve a WebAppProvider from the Profile.
-        //
-        // Similarly, if a Hosted Apps is a PWA, it will always have a scope
-        // so there is no need to test for has_value().
-        web_prefs->web_app_scope =
-            web_app::WebAppProvider::Get(profile)
-                ->registrar()
-                .GetAppScopeInternal(browser->app_controller()->GetAppId())
-                .value();
+          browser->app_controller()->HasAppId()) {
+        const web_app::AppId& app_id = browser->app_controller()->GetAppId();
+        const web_app::AppRegistrar& registrar =
+            web_app::WebAppProviderBase::GetProviderBase(profile)->registrar();
+        if (registrar.IsLocallyInstalled(app_id))
+          web_prefs->web_app_scope = registrar.GetAppScope(app_id);
       }
     }
 #endif

chrome/browser/web_applications/extensions/bookmark_app_registrar.cc

@@ -42,13 +42,14 @@ bool BookmarkAppRegistrar::IsInstalled(const web_app::AppId& app_id) const {
 
 bool BookmarkAppRegistrar::IsLocallyInstalled(
     const web_app::AppId& app_id) const {
-  const Extension* extension = GetBookmarkApp(app_id);
-  return extension && BookmarkAppIsLocallyInstalled(profile(), extension);
+  const Extension* extension = GetEnabledExtension(app_id);
+  return extension && extension->from_bookmark() &&
+         BookmarkAppIsLocallyInstalled(profile(), extension);
 }
 
 bool BookmarkAppRegistrar::WasInstalledByUser(
     const web_app::AppId& app_id) const {
-  const Extension* extension = GetBookmarkApp(app_id);
+  const Extension* extension = GetBookmarkAppDchecked(app_id);
   return extension && !extension->was_installed_by_default();
 }
 
@@ -94,19 +95,19 @@ void BookmarkAppRegistrar::OnShutdown(ExtensionRegistry* registry) {
 
 std::string BookmarkAppRegistrar::GetAppShortName(
     const web_app::AppId& app_id) const {
-  const Extension* extension = GetBookmarkApp(app_id);
+  const Extension* extension = GetBookmarkAppDchecked(app_id);
   return extension ? extension->short_name() : std::string();
 }
 
 std::string BookmarkAppRegistrar::GetAppDescription(
     const web_app::AppId& app_id) const {
-  const Extension* extension = GetBookmarkApp(app_id);
+  const Extension* extension = GetBookmarkAppDchecked(app_id);
   return extension ? extension->description() : std::string();
 }
 
 base::Optional<SkColor> BookmarkAppRegistrar::GetAppThemeColor(
     const web_app::AppId& app_id) const {
-  const Extension* extension = GetBookmarkApp(app_id);
+  const Extension* extension = GetBookmarkAppDchecked(app_id);
   if (!extension)
     return base::nullopt;
 
@@ -120,18 +121,18 @@ base::Optional<SkColor> BookmarkAppRegistrar::GetAppThemeColor(
 
 const GURL& BookmarkAppRegistrar::GetAppLaunchURL(
     const web_app::AppId& app_id) const {
-  const Extension* extension = GetBookmarkApp(app_id);
+  const Extension* extension = GetBookmarkAppDchecked(app_id);
   return extension ? AppLaunchInfo::GetLaunchWebURL(extension)
                    : GURL::EmptyGURL();
 }
 
 base::Optional<GURL> BookmarkAppRegistrar::GetAppScopeInternal(
     const web_app::AppId& app_id) const {
-  const Extension* extension = GetBookmarkApp(app_id);
+  const Extension* extension = GetBookmarkAppDchecked(app_id);
   if (!extension)
     return base::nullopt;
 
-  GURL scope_url = GetScopeURLFromBookmarkApp(GetBookmarkApp(app_id));
+  GURL scope_url = GetScopeURLFromBookmarkApp(GetBookmarkAppDchecked(app_id));
   if (scope_url.is_valid())
     return scope_url;
 
@@ -140,7 +141,7 @@ base::Optional<GURL> BookmarkAppRegistrar::GetAppScopeInternal(
 
 DisplayMode BookmarkAppRegistrar::GetAppDisplayMode(
     const web_app::AppId& app_id) const {
-  const Extension* extension = GetBookmarkApp(app_id);
+  const Extension* extension = GetBookmarkAppDchecked(app_id);
   if (!extension)
     return DisplayMode::kUndefined;
 
@@ -149,7 +150,7 @@ DisplayMode BookmarkAppRegistrar::GetAppDisplayMode(
 
 DisplayMode BookmarkAppRegistrar::GetAppUserDisplayMode(
     const web_app::AppId& app_id) const {
-  const Extension* extension = GetBookmarkApp(app_id);
+  const Extension* extension = GetBookmarkAppDchecked(app_id);
   if (!extension)
     return DisplayMode::kStandalone;
 
@@ -169,7 +170,7 @@ DisplayMode BookmarkAppRegistrar::GetAppUserDisplayMode(
 std::vector<WebApplicationIconInfo> BookmarkAppRegistrar::GetAppIconInfos(
     const web_app::AppId& app_id) const {
   std::vector<WebApplicationIconInfo> result;
-  const Extension* extension = GetBookmarkApp(app_id);
+  const Extension* extension = GetBookmarkAppDchecked(app_id);
   if (!extension)
     return result;
   for (const LinkedAppIcons::IconInfo& icon_info :
@@ -184,7 +185,7 @@ std::vector<WebApplicationIconInfo> BookmarkAppRegistrar::GetAppIconInfos(
 
 std::vector<SquareSizePx> BookmarkAppRegistrar::GetAppDownloadedIconSizes(
     const web_app::AppId& app_id) const {
-  const Extension* extension = GetBookmarkApp(app_id);
+  const Extension* extension = GetBookmarkAppDchecked(app_id);
   return extension ? GetBookmarkAppDownloadedIconSizes(extension)
                    : std::vector<SquareSizePx>();
 }
@@ -218,7 +219,7 @@ const Extension* BookmarkAppRegistrar::FindExtension(
   return ExtensionRegistry::Get(profile())->GetInstalledExtension(app_id);
 }
 
-const Extension* BookmarkAppRegistrar::GetBookmarkApp(
+const Extension* BookmarkAppRegistrar::GetBookmarkAppDchecked(
     const web_app::AppId& app_id) const {
   const Extension* extension = GetEnabledExtension(app_id);
   DCHECK(!extension || extension->from_bookmark());

chrome/browser/web_applications/extensions/bookmark_app_registrar.h

@@ -69,7 +69,9 @@ class BookmarkAppRegistrar : public web_app::AppRegistrar,
   const Extension* FindExtension(const web_app::AppId& app_id) const;
 
  private:
-  const Extension* GetBookmarkApp(const web_app::AppId& app_id) const;
+  // DCHECKs that app_id isn't for a Chrome app to catch places where Chrome app
+  // UI accidentally starts using web_app::AppRegistrar when it shouldn't.
+  const Extension* GetBookmarkAppDchecked(const web_app::AppId& app_id) const;
   const Extension* GetEnabledExtension(const web_app::AppId& app_id) const;
 
   ScopedObserver<ExtensionRegistry, ExtensionRegistryObserver>