desktop-pwas: Allow clients to set the location and extra install flags

Changes BookmarkAppHelper, CrxInstaller, and ConvertWebAppToExtension
so that clients can set the manifest location and extra install flags
that will be used when creating the extension.

Bug: 852244

Change-Id: I59077e015dc80d9461218f3722942e58e9bc5281
Reviewed-on: https://chromium-review.googlesource.com/1170152Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582513}

chrome/browser/extensions/bookmark_app_helper.cc

@@ -546,6 +546,15 @@ void BookmarkAppHelper::OnBubbleCompleted(
     const WebApplicationInfo& web_app_info) {
   if (user_accepted) {
     web_app_info_ = web_app_info;
+
+    if (is_policy_installed_app_)
+      crx_installer_->set_install_source(Manifest::EXTERNAL_POLICY);
+
+    // InstallWebApp will OR the creation flags with FROM_BOOKMARK.
+    crx_installer_->set_creation_flags(is_default_app_
+                                           ? Extension::WAS_INSTALLED_BY_DEFAULT
+                                           : Extension::NO_FLAGS);
+
     crx_installer_->InstallWebApp(web_app_info_);
 
     if (InstallableMetrics::IsReportableInstallSource(install_source_) &&

chrome/browser/extensions/bookmark_app_helper.h

@@ -19,6 +19,7 @@
 #include "chrome/common/web_application_info.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
+#include "extensions/common/extension.h"
 #include "third_party/blink/public/common/manifest/manifest.h"
 
 class WebAppIconDownloader;
@@ -102,6 +103,12 @@ class BookmarkAppHelper : public content::NotificationObserver {
   // Begins the asynchronous bookmark app creation.
   void Create(const CreateBookmarkAppCallback& callback);
 
+  // If called, the installed extension will be considered policy installed.
+  void set_is_policy_installed_app() { is_policy_installed_app_ = true; }
+
+  // If called, the installed extension will be considered default installed.
+  void set_is_default_app() { is_default_app_ = true; }
+
  protected:
   // Protected methods for testing.
 
@@ -157,6 +164,10 @@ class BookmarkAppHelper : public content::NotificationObserver {
 
   ForInstallableSite for_installable_site_ = ForInstallableSite::kUnknown;
 
+  bool is_policy_installed_app_ = false;
+
+  bool is_default_app_ = false;
+
   // The mechanism via which the app creation was triggered.
   WebappInstallSource install_source_;
 

chrome/browser/extensions/bookmark_app_helper_unittest.cc

@@ -79,6 +79,8 @@ class BookmarkAppHelperExtensionServiceTest
     InitializeEmptyExtensionService();
     service_->Init();
     EXPECT_EQ(0u, registry()->enabled_extensions().size());
+    web_contents_ =
+        content::WebContentsTester::CreateTestWebContents(profile(), nullptr);
   }
 
   void TearDown() override {
@@ -94,7 +96,11 @@ class BookmarkAppHelperExtensionServiceTest
     }
   }
 
+  content::WebContents* web_contents() { return web_contents_.get(); }
+
  private:
+  std::unique_ptr<content::WebContents> web_contents_;
+
   DISALLOW_COPY_AND_ASSIGN(BookmarkAppHelperExtensionServiceTest);
 };
 
@@ -155,6 +161,17 @@ class TestBookmarkAppHelper : public BookmarkAppHelper {
     BookmarkAppHelper::OnIconsDownloaded(success, bitmaps);
   }
 
+  void FinishCreationForNonInstallableSite() {
+    CompleteInstallableCheck("", blink::Manifest(), ForInstallableSite::kNo);
+
+    std::map<GURL, std::vector<SkBitmap>> icon_map;
+    icon_map[GURL(kAppUrl)].push_back(
+        CreateSquareBitmapWithColor(kIconSizeSmall, SK_ColorRED));
+    CompleteIconDownload(true, icon_map);
+
+    content::RunAllTasksUntilIdle();
+  }
+
   const Extension* extension() { return extension_; }
 
   const WebAppIconDownloader* web_app_icon_downloader() {
@@ -176,27 +193,20 @@ TEST_F(BookmarkAppHelperExtensionServiceTest, CreateBookmarkApp) {
   web_app_info.title = base::UTF8ToUTF16(kAppTitle);
   web_app_info.description = base::UTF8ToUTF16(kAppDescription);
 
-  std::unique_ptr<content::WebContents> contents(
-      content::WebContentsTester::CreateTestWebContents(profile(), nullptr));
-  TestBookmarkAppHelper helper(service_, web_app_info, contents.get());
+  TestBookmarkAppHelper helper(service_, web_app_info, web_contents());
   helper.Create(base::Bind(&TestBookmarkAppHelper::CreationComplete,
                            base::Unretained(&helper)));
+  helper.FinishCreationForNonInstallableSite();
 
-  helper.CompleteInstallableCheck(kManifestUrl, blink::Manifest(),
-                                  ForInstallableSite::kNo);
-
-  std::map<GURL, std::vector<SkBitmap> > icon_map;
-  icon_map[GURL(kAppUrl)].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_FALSE(extension->was_installed_by_default());
+  EXPECT_FALSE(Manifest::IsPolicyLocation(helper.extension()->location()));
+
   EXPECT_EQ(kAppTitle, extension->name());
   EXPECT_EQ(kAppDescription, extension->description());
   EXPECT_EQ(GURL(kAppUrl), AppLaunchInfo::GetLaunchWebURL(extension));
@@ -205,11 +215,48 @@ TEST_F(BookmarkAppHelperExtensionServiceTest, CreateBookmarkApp) {
           extension, kIconSizeSmall, ExtensionIconSet::MATCH_EXACTLY).empty());
   EXPECT_FALSE(
       AppBannerSettingsHelper::GetSingleBannerEvent(
-          contents.get(), web_app_info.app_url, web_app_info.app_url.spec(),
+          web_contents(), web_app_info.app_url, web_app_info.app_url.spec(),
           AppBannerSettingsHelper::APP_BANNER_EVENT_DID_ADD_TO_HOMESCREEN)
           .is_null());
 }
 
+TEST_F(BookmarkAppHelperExtensionServiceTest, CreateBookmarkAppDefaultApp) {
+  WebApplicationInfo web_app_info;
+  web_app_info.app_url = GURL(kAppUrl);
+  web_app_info.title = base::UTF8ToUTF16(kAppTitle);
+  web_app_info.description = base::UTF8ToUTF16(kAppDescription);
+
+  TestBookmarkAppHelper helper(service_, web_app_info, web_contents());
+  helper.set_is_default_app();
+  helper.Create(base::Bind(&TestBookmarkAppHelper::CreationComplete,
+                           base::Unretained(&helper)));
+  helper.FinishCreationForNonInstallableSite();
+
+  ASSERT_TRUE(helper.extension());
+  EXPECT_TRUE(helper.extension()->from_bookmark());
+  EXPECT_TRUE(helper.extension()->was_installed_by_default());
+  EXPECT_FALSE(Manifest::IsPolicyLocation(helper.extension()->location()));
+}
+
+TEST_F(BookmarkAppHelperExtensionServiceTest,
+       CreateBookmarkAppPolicyInstalled) {
+  WebApplicationInfo web_app_info;
+  web_app_info.app_url = GURL(kAppUrl);
+  web_app_info.title = base::UTF8ToUTF16(kAppTitle);
+  web_app_info.description = base::UTF8ToUTF16(kAppDescription);
+
+  TestBookmarkAppHelper helper(service_, web_app_info, web_contents());
+  helper.set_is_policy_installed_app();
+  helper.Create(base::Bind(&TestBookmarkAppHelper::CreationComplete,
+                           base::Unretained(&helper)));
+  helper.FinishCreationForNonInstallableSite();
+
+  ASSERT_TRUE(helper.extension());
+  EXPECT_TRUE(helper.extension()->from_bookmark());
+  EXPECT_FALSE(helper.extension()->was_installed_by_default());
+  EXPECT_TRUE(Manifest::IsPolicyLocation(helper.extension()->location()));
+}
+
 class BookmarkAppHelperExtensionServiceInstallableSiteTest
     : public BookmarkAppHelperExtensionServiceTest,
       public ::testing::WithParamInterface<ForInstallableSite> {
@@ -226,9 +273,7 @@ TEST_P(BookmarkAppHelperExtensionServiceInstallableSiteTest,
        CreateBookmarkAppWithManifest) {
   WebApplicationInfo web_app_info;
 
-  std::unique_ptr<content::WebContents> contents(
-      content::WebContentsTester::CreateTestWebContents(profile(), nullptr));
-  TestBookmarkAppHelper helper(service_, web_app_info, contents.get());
+  TestBookmarkAppHelper helper(service_, web_app_info, web_contents());
   helper.Create(base::Bind(&TestBookmarkAppHelper::CreationComplete,
                            base::Unretained(&helper)));
 
@@ -254,7 +299,7 @@ TEST_P(BookmarkAppHelperExtensionServiceInstallableSiteTest,
   EXPECT_EQ(SK_ColorBLUE, AppThemeColorInfo::GetThemeColor(extension).value());
   EXPECT_FALSE(
       AppBannerSettingsHelper::GetSingleBannerEvent(
-          contents.get(), manifest.start_url, manifest.start_url.spec(),
+          web_contents(), manifest.start_url, manifest.start_url.spec(),
           AppBannerSettingsHelper::APP_BANNER_EVENT_DID_ADD_TO_HOMESCREEN)
           .is_null());
 
@@ -268,9 +313,7 @@ TEST_P(BookmarkAppHelperExtensionServiceInstallableSiteTest,
 TEST_P(BookmarkAppHelperExtensionServiceInstallableSiteTest,
        CreateBookmarkAppWithManifestIcons) {
   WebApplicationInfo web_app_info;
-  std::unique_ptr<content::WebContents> contents(
-      content::WebContentsTester::CreateTestWebContents(profile(), nullptr));
-  TestBookmarkAppHelper helper(service_, web_app_info, contents.get());
+  TestBookmarkAppHelper helper(service_, web_app_info, web_contents());
   helper.Create(base::Bind(&TestBookmarkAppHelper::CreationComplete,
                            base::Unretained(&helper)));
 
@@ -318,9 +361,7 @@ TEST_P(BookmarkAppHelperExtensionServiceInstallableSiteTest,
        CreateBookmarkAppWithManifestNoScope) {
   WebApplicationInfo web_app_info;
 
-  std::unique_ptr<content::WebContents> contents(
-      content::WebContentsTester::CreateTestWebContents(profile(), nullptr));
-  TestBookmarkAppHelper helper(service_, web_app_info, contents.get());
+  TestBookmarkAppHelper helper(service_, web_app_info, web_contents());
   helper.Create(base::Bind(&TestBookmarkAppHelper::CreationComplete,
                            base::Unretained(&helper)));
 
@@ -356,14 +397,12 @@ TEST_F(BookmarkAppHelperExtensionServiceTest,
   WebApplicationInfo web_app_info;
   std::map<GURL, std::vector<SkBitmap>> icon_map;
 
-  std::unique_ptr<content::WebContents> contents(
-      content::WebContentsTester::CreateTestWebContents(profile(), nullptr));
   blink::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());
+    TestBookmarkAppHelper helper(service_, web_app_info, web_contents());
     helper.Create(base::Bind(&TestBookmarkAppHelper::CreationComplete,
                              base::Unretained(&helper)));
 
@@ -380,7 +419,7 @@ TEST_F(BookmarkAppHelperExtensionServiceTest,
               GetLaunchContainer(ExtensionPrefs::Get(profile()), extension));
   }
   {
-    TestBookmarkAppHelper helper(service_, web_app_info, contents.get());
+    TestBookmarkAppHelper helper(service_, web_app_info, web_contents());
     helper.Create(base::Bind(&TestBookmarkAppHelper::CreationComplete,
                              base::Unretained(&helper)));
 
@@ -405,9 +444,7 @@ TEST_F(BookmarkAppHelperExtensionServiceTest,
   web_app_info.description = base::UTF8ToUTF16(kAppDescription);
   web_app_info.app_url = GURL(kAppUrl);
 
-  std::unique_ptr<content::WebContents> contents(
-      content::WebContentsTester::CreateTestWebContents(profile(), nullptr));
-  TestBookmarkAppHelper helper(service_, web_app_info, contents.get());
+  TestBookmarkAppHelper helper(service_, web_app_info, web_contents());
   helper.Create(base::Bind(&TestBookmarkAppHelper::CreationComplete,
                            base::Unretained(&helper)));
 

chrome/browser/extensions/convert_web_app.cc

@@ -141,7 +141,9 @@ std::string ConvertTimeToExtensionVersion(const base::Time& create_time) {
 scoped_refptr<Extension> ConvertWebAppToExtension(
     const WebApplicationInfo& web_app,
     const base::Time& create_time,
-    const base::FilePath& extensions_dir) {
+    const base::FilePath& extensions_dir,
+    int extra_creation_flags,
+    Manifest::Location install_source) {
   base::FilePath install_temp_dir =
       file_util::GetInstallTempDir(extensions_dir);
   if (install_temp_dir.empty()) {
@@ -236,9 +238,9 @@ scoped_refptr<Extension> ConvertWebAppToExtension(
 
   // Finally, create the extension object to represent the unpacked directory.
   std::string error;
-  scoped_refptr<Extension> extension =
-      Extension::Create(temp_dir.GetPath(), Manifest::INTERNAL, *root,
-                        Extension::FROM_BOOKMARK, &error);
+  scoped_refptr<Extension> extension = Extension::Create(
+      temp_dir.GetPath(), install_source, *root,
+      Extension::FROM_BOOKMARK | extra_creation_flags, &error);
   if (!extension.get()) {
     LOG(ERROR) << error;
     return NULL;

chrome/browser/extensions/convert_web_app.h

@@ -8,6 +8,7 @@
 #include <string>
 
 #include "base/memory/ref_counted.h"
+#include "extensions/common/manifest.h"
 
 namespace base {
 class DictionaryValue;
@@ -71,7 +72,9 @@ std::string ConvertTimeToExtensionVersion(const base::Time& time);
 scoped_refptr<Extension> ConvertWebAppToExtension(
     const WebApplicationInfo& web_app_info,
     const base::Time& create_time,
-    const base::FilePath& extensions_dir);
+    const base::FilePath& extensions_dir,
+    int extra_creation_flags,
+    Manifest::Location install_source);
 
 }  // namespace extensions
 

chrome/browser/extensions/convert_web_app_unittest.cc

@@ -258,7 +258,8 @@ TEST(ExtensionFromWebApp, Basic) {
   }
 
   scoped_refptr<Extension> extension = ConvertWebAppToExtension(
-      web_app, GetTestTime(1978, 12, 11, 0, 0, 0, 0), extensions_dir.GetPath());
+      web_app, GetTestTime(1978, 12, 11, 0, 0, 0, 0), extensions_dir.GetPath(),
+      Extension::NO_FLAGS, Manifest::INTERNAL);
   ASSERT_TRUE(extension.get());
 
   base::ScopedTempDir extension_dir;
@@ -266,8 +267,14 @@ TEST(ExtensionFromWebApp, Basic) {
 
   EXPECT_TRUE(extension->is_app());
   EXPECT_TRUE(extension->is_hosted_app());
+  EXPECT_TRUE(extension->from_bookmark());
   EXPECT_FALSE(extension->is_legacy_packaged_app());
 
+  EXPECT_FALSE(extension->was_installed_by_default());
+  EXPECT_FALSE(extension->was_installed_by_oem());
+  EXPECT_FALSE(extension->from_webstore());
+  EXPECT_EQ(Manifest::INTERNAL, extension->location());
+
   EXPECT_EQ("zVvdNZy3Mp7CFU8JVSyXNlDuHdVLbP7fDO3TGVzj/0w=",
             extension->public_key());
   EXPECT_EQ("oplhagaaipaimkjlbekcdjkffijdockj", extension->id());
@@ -304,7 +311,8 @@ TEST(ExtensionFromWebApp, Minimal) {
   web_app.app_url = GURL("http://aaronboodman.com/gearpad/");
 
   scoped_refptr<Extension> extension = ConvertWebAppToExtension(
-      web_app, GetTestTime(1978, 12, 11, 0, 0, 0, 0), extensions_dir.GetPath());
+      web_app, GetTestTime(1978, 12, 11, 0, 0, 0, 0), extensions_dir.GetPath(),
+      Extension::NO_FLAGS, Manifest::INTERNAL);
   ASSERT_TRUE(extension.get());
 
   base::ScopedTempDir extension_dir;
@@ -312,8 +320,14 @@ TEST(ExtensionFromWebApp, Minimal) {
 
   EXPECT_TRUE(extension->is_app());
   EXPECT_TRUE(extension->is_hosted_app());
+  EXPECT_TRUE(extension->from_bookmark());
   EXPECT_FALSE(extension->is_legacy_packaged_app());
 
+  EXPECT_FALSE(extension->was_installed_by_default());
+  EXPECT_FALSE(extension->was_installed_by_oem());
+  EXPECT_FALSE(extension->from_webstore());
+  EXPECT_EQ(Manifest::INTERNAL, extension->location());
+
   EXPECT_EQ("zVvdNZy3Mp7CFU8JVSyXNlDuHdVLbP7fDO3TGVzj/0w=",
             extension->public_key());
   EXPECT_EQ("oplhagaaipaimkjlbekcdjkffijdockj", extension->id());
@@ -328,6 +342,52 @@ TEST(ExtensionFromWebApp, Minimal) {
   ASSERT_EQ(0u, extension->web_extent().patterns().size());
 }
 
+TEST(ExtensionFromWebApp, ExtraInstallationFlags) {
+  base::ScopedTempDir extensions_dir;
+  ASSERT_TRUE(extensions_dir.CreateUniqueTempDir());
+
+  WebApplicationInfo web_app;
+  web_app.title = base::ASCIIToUTF16("Gearpad");
+  web_app.app_url = GURL("http://aaronboodman.com/gearpad/");
+
+  scoped_refptr<Extension> extension = ConvertWebAppToExtension(
+      web_app, GetTestTime(1978, 12, 11, 0, 0, 0, 0), extensions_dir.GetPath(),
+      Extension::FROM_WEBSTORE | Extension::WAS_INSTALLED_BY_OEM,
+      Manifest::INTERNAL);
+  ASSERT_TRUE(extension.get());
+
+  EXPECT_TRUE(extension->is_app());
+  EXPECT_TRUE(extension->is_hosted_app());
+  EXPECT_TRUE(extension->from_bookmark());
+  EXPECT_FALSE(extension->is_legacy_packaged_app());
+
+  EXPECT_TRUE(extension->was_installed_by_oem());
+  EXPECT_TRUE(extension->from_webstore());
+  EXPECT_FALSE(extension->was_installed_by_default());
+  EXPECT_EQ(Manifest::INTERNAL, extension->location());
+}
+
+TEST(ExtensionFromWebApp, ExternalPolicyLocation) {
+  base::ScopedTempDir extensions_dir;
+  ASSERT_TRUE(extensions_dir.CreateUniqueTempDir());
+
+  WebApplicationInfo web_app;
+  web_app.title = base::ASCIIToUTF16("Gearpad");
+  web_app.app_url = GURL("http://aaronboodman.com/gearpad/");
+
+  scoped_refptr<Extension> extension = ConvertWebAppToExtension(
+      web_app, GetTestTime(1978, 12, 11, 0, 0, 0, 0), extensions_dir.GetPath(),
+      Extension::NO_FLAGS, Manifest::EXTERNAL_POLICY);
+  ASSERT_TRUE(extension.get());
+
+  EXPECT_TRUE(extension->is_app());
+  EXPECT_TRUE(extension->is_hosted_app());
+  EXPECT_TRUE(extension->from_bookmark());
+  EXPECT_FALSE(extension->is_legacy_packaged_app());
+
+  EXPECT_EQ(Manifest::EXTERNAL_POLICY, extension->location());
+}
+
 // Tests that a scope not ending in "/" works correctly.
 // The tested behavior is unexpected but is working correctly according
 // to the Web Manifest spec. https://github.com/w3c/manifest/issues/554
@@ -343,7 +403,8 @@ TEST(ExtensionFromWebApp, ScopeDoesNotEndInSlash) {
   web_app.scope = GURL("http://aaronboodman.com/gear");
 
   scoped_refptr<Extension> extension = ConvertWebAppToExtension(
-      web_app, GetTestTime(1978, 12, 11, 0, 0, 0, 0), extensions_dir.GetPath());
+      web_app, GetTestTime(1978, 12, 11, 0, 0, 0, 0), extensions_dir.GetPath(),
+      Extension::NO_FLAGS, Manifest::INTERNAL);
   ASSERT_TRUE(extension.get());
   EXPECT_EQ(web_app.scope, GetScopeURLFromBookmarkApp(extension.get()));
 }

chrome/browser/extensions/crx_installer.cc

@@ -293,8 +293,9 @@ void CrxInstaller::UpdateExtensionFromUnpackedCrx(
 
 void CrxInstaller::ConvertWebAppOnFileThread(
     const WebApplicationInfo& web_app) {
-  scoped_refptr<Extension> extension(ConvertWebAppToExtension(
-      web_app, base::Time::Now(), install_directory_));
+  scoped_refptr<Extension> extension(
+      ConvertWebAppToExtension(web_app, base::Time::Now(), install_directory_,
+                               creation_flags_, install_source_));
   if (!extension.get()) {
     // Validation should have stopped any potential errors before getting here.
     NOTREACHED() << "Could not convert web app to extension.";