Gives chrome://media-app a launcher icon with --enable-features=MediaApp.

Note that --enable-features=MediaApp and the added test still work even
when gn args has enable_cros_media_app = false, but the mock app is
loaded in this case (i.e. js/app_main.js is an empty file).

The manifest and html served are the same regardless of enable_cros_media_app.

Bug: 996088
Change-Id: I4b81f622efc874afdc53794eede8d04b69e3e3ef
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1824523Reviewed-by: calamity <calamity@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Trent Apted <tapted@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705746}

chrome/browser/web_applications/BUILD.gn

@@ -63,6 +63,7 @@ source_set("web_applications") {
   if (is_chromeos) {
     deps += [
       "//ash/public/cpp:cpp",
+      "//chromeos/components/media_app_ui",
       "//chromeos/constants",
       "//components/arc:arc_base",
       "//components/arc/mojom",

chrome/browser/web_applications/extensions/BUILD.gn

@@ -84,6 +84,7 @@ source_set("browser_tests") {
     "bookmark_app_registrar_browsertest.cc",
     "system_web_app_manager_browsertest.cc",
     "system_web_app_manager_browsertest.h",
+    "system_web_app_manager_browsertest_chromeos.cc",
     "web_app_audio_focus_browsertest.cc",
   ]
 
@@ -95,6 +96,7 @@ source_set("browser_tests") {
     "//chrome/app/theme:chrome_unscaled_resources_grit",
     "//chrome/browser",
     "//chrome/browser/ui",
+    "//chrome/browser/web_applications",
     "//chrome/browser/web_applications:web_app_test_group",
     "//chrome/browser/web_applications:web_applications_on_extensions",
     "//chrome/browser/web_applications:web_applications_on_extensions_test_support",
@@ -107,4 +109,11 @@ source_set("browser_tests") {
     "//extensions/common",
     "//net:test_support",
   ]
+
+  if (is_chromeos) {
+    deps += [
+      "//chromeos/components/media_app_ui",
+      "//chromeos/constants",
+    ]
+  }
 }

chrome/browser/web_applications/extensions/system_web_app_manager_browsertest.cc

@@ -126,17 +126,36 @@ class TestWebUIControllerFactory : public content::WebUIControllerFactory {
   DISALLOW_COPY_AND_ASSIGN(TestWebUIControllerFactory);
 };
 
-SystemWebAppManagerBrowserTest::SystemWebAppManagerBrowserTest()
-    : factory_(std::make_unique<TestWebUIControllerFactory>()),
-      test_web_app_provider_creator_(
-          base::BindOnce(&SystemWebAppManagerBrowserTest::CreateWebAppProvider,
-                         base::Unretained(this))) {
+SystemWebAppManagerBrowserTest::SystemWebAppManagerBrowserTest(
+    bool install_mock) {
   scoped_feature_list_.InitWithFeatures({features::kSystemWebApps}, {});
-  content::WebUIControllerFactory::RegisterFactory(factory_.get());
+  if (install_mock) {
+    factory_ = std::make_unique<TestWebUIControllerFactory>();
+    test_web_app_provider_creator_ =
+        std::make_unique<TestWebAppProviderCreator>(base::BindOnce(
+            &SystemWebAppManagerBrowserTest::CreateWebAppProvider,
+            base::Unretained(this)));
+    content::WebUIControllerFactory::RegisterFactory(factory_.get());
+  }
 }
 
 SystemWebAppManagerBrowserTest::~SystemWebAppManagerBrowserTest() {
-  content::WebUIControllerFactory::UnregisterFactoryForTesting(factory_.get());
+  if (factory_) {
+    content::WebUIControllerFactory::UnregisterFactoryForTesting(
+        factory_.get());
+  }
+}
+
+// static
+const extensions::Extension*
+SystemWebAppManagerBrowserTest::GetExtensionForAppBrowser(Browser* browser) {
+  return static_cast<extensions::HostedAppBrowserController*>(
+             browser->app_controller())
+      ->GetExtensionForTesting();
+}
+
+SystemWebAppManager& SystemWebAppManagerBrowserTest::GetManager() {
+  return WebAppProvider::Get(browser()->profile())->system_web_app_manager();
 }
 
 std::unique_ptr<KeyedService>
@@ -165,18 +184,20 @@ SystemWebAppManagerBrowserTest::CreateWebAppProvider(Profile* profile) {
 
 Browser* SystemWebAppManagerBrowserTest::WaitForSystemAppInstallAndLaunch(
     SystemAppType system_app_type) {
-  Profile* profile = browser()->profile();
-
   // Wait for the System Web Apps to install.
-  base::RunLoop run_loop;
-  test_system_web_app_manager_->on_apps_synchronized().Post(
-      FROM_HERE, run_loop.QuitClosure());
-  run_loop.Run();
+  if (factory_) {
+    base::RunLoop run_loop;
+    GetManager().on_apps_synchronized().Post(FROM_HERE, run_loop.QuitClosure());
+    run_loop.Run();
+  } else {
+    GetManager().InstallSystemAppsForTesting();
+  }
 
   base::Optional<AppId> app_id =
-      test_system_web_app_manager_->GetAppIdForSystemApp(system_app_type);
+      GetManager().GetAppIdForSystemApp(system_app_type);
   DCHECK(app_id.has_value());
 
+  Profile* profile = browser()->profile();
   const extensions::Extension* app =
       extensions::ExtensionRegistry::Get(profile)->enabled_extensions().GetByID(
           app_id.value());
@@ -187,11 +208,8 @@ Browser* SystemWebAppManagerBrowserTest::WaitForSystemAppInstallAndLaunch(
 
 // Test that System Apps install correctly with a manifest.
 IN_PROC_BROWSER_TEST_F(SystemWebAppManagerBrowserTest, Install) {
-  const extensions::Extension* app =
-      static_cast<extensions::HostedAppBrowserController*>(
-          WaitForSystemAppInstallAndLaunch(SystemAppType::SETTINGS)
-              ->app_controller())
-          ->GetExtensionForTesting();
+  const extensions::Extension* app = GetExtensionForAppBrowser(
+      WaitForSystemAppInstallAndLaunch(SystemAppType::SETTINGS));
   EXPECT_EQ("Test System App", app->name());
   EXPECT_EQ(SkColorSetRGB(0, 0xFF, 0),
             extensions::AppThemeColorInfo::GetThemeColor(app));
@@ -202,9 +220,7 @@ IN_PROC_BROWSER_TEST_F(SystemWebAppManagerBrowserTest, Install) {
   EXPECT_EQ(extensions::util::GetInstalledPwaForUrl(
                 browser()->profile(), content::GetWebUIURL("test-system-app/")),
             app);
-  EXPECT_TRUE(WebAppProvider::Get(browser()->profile())
-                  ->system_web_app_manager()
-                  .IsSystemWebApp(app->id()));
+  EXPECT_TRUE(GetManager().IsSystemWebApp(app->id()));
 }
 
 // Check the toolbar is not shown for system web apps for pages on the chrome://

chrome/browser/web_applications/extensions/system_web_app_manager_browsertest.h

@@ -15,6 +15,10 @@
 class Browser;
 class KeyedService;
 
+namespace extensions {
+class Extension;
+}
+
 namespace web_app {
 
 class TestSystemWebAppManager;
@@ -23,10 +27,23 @@ enum class SystemAppType;
 
 class SystemWebAppManagerBrowserTest : public InProcessBrowserTest {
  public:
-  SystemWebAppManagerBrowserTest();
+  // Performs common initialization for testing SystemWebAppManager features.
+  // If true, |install_mock| installs a WebUIController that serves a mock
+  // System PWA, and ensures the WebAppProvider associated with the startup
+  // profile is a TestWebAppProviderCreator.
+  explicit SystemWebAppManagerBrowserTest(bool install_mock = true);
+
   ~SystemWebAppManagerBrowserTest() override;
 
- protected:
+  // Gets the Extension* from the HostedAppBrowserController associated with
+  // |browser|.
+  static const extensions::Extension* GetExtensionForAppBrowser(
+      Browser* browser);
+
+  // Returns the SystemWebAppManager for browser()->profile(). This will be a
+  // TestSystemWebAppManager if initialized with |install_mock| true.
+  SystemWebAppManager& GetManager();
+
   Browser* WaitForSystemAppInstallAndLaunch(SystemAppType system_app_type);
 
  private:
@@ -34,7 +51,7 @@ class SystemWebAppManagerBrowserTest : public InProcessBrowserTest {
 
   base::test::ScopedFeatureList scoped_feature_list_;
   std::unique_ptr<TestWebUIControllerFactory> factory_;
-  TestWebAppProviderCreator test_web_app_provider_creator_;
+  std::unique_ptr<TestWebAppProviderCreator> test_web_app_provider_creator_;
   TestSystemWebAppManager* test_system_web_app_manager_ = nullptr;
 
   DISALLOW_COPY_AND_ASSIGN(SystemWebAppManagerBrowserTest);

chrome/browser/web_applications/extensions/system_web_app_manager_browsertest_chromeos.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/web_applications/extensions/system_web_app_manager_browsertest.h"
+
+#include "base/strings/utf_string_conversions.h"
+#include "base/test/scoped_feature_list.h"
+#include "chrome/browser/extensions/extension_util.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_window.h"
+#include "chrome/browser/ui/tabs/tab_strip_model.h"
+#include "chrome/browser/web_applications/system_web_app_manager.h"
+#include "chromeos/components/media_app_ui/url_constants.h"
+#include "chromeos/constants/chromeos_features.h"
+#include "content/public/browser/web_ui.h"
+#include "content/public/test/test_navigation_observer.h"
+#include "content/public/test/test_utils.h"
+#include "extensions/common/extension.h"
+#include "extensions/common/manifest.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "ui/aura/window.h"
+
+namespace web_app {
+
+namespace {
+
+class SystemWebAppManagerBrowserTestChromeos
+    : public SystemWebAppManagerBrowserTest {
+ public:
+  SystemWebAppManagerBrowserTestChromeos()
+      : SystemWebAppManagerBrowserTest(false /* install_mock */) {
+    scoped_feature_list_.InitAndEnableFeature(chromeos::features::kMediaApp);
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+}  // namespace
+
+// Test that the Media App installs and launches correctly. Runs some spot
+// checks on the manifest.
+IN_PROC_BROWSER_TEST_F(SystemWebAppManagerBrowserTestChromeos, MediaApp) {
+  Browser* app_browser = WaitForSystemAppInstallAndLaunch(SystemAppType::MEDIA);
+  const extensions::Extension* installed_app =
+      extensions::util::GetInstalledPwaForUrl(
+          browser()->profile(),
+          content::GetWebUIURL(chromeos::kChromeUIMediaAppHost));
+
+  EXPECT_TRUE(GetManager().IsSystemWebApp(installed_app->id()));
+  EXPECT_TRUE(installed_app->from_bookmark());
+
+  EXPECT_EQ("Media App", installed_app->name());
+  EXPECT_EQ(base::ASCIIToUTF16("Media App"),
+            app_browser->window()->GetNativeWindow()->GetTitle());
+  EXPECT_EQ(extensions::Manifest::EXTERNAL_COMPONENT,
+            installed_app->location());
+
+  // The installed app should match the opened app window.
+  EXPECT_EQ(installed_app, GetExtensionForAppBrowser(app_browser));
+  content::WebContents* web_contents =
+      app_browser->tab_strip_model()->GetActiveWebContents();
+
+  // The opened window should be showing chrome://media-app with attached WebUI.
+  EXPECT_EQ(chromeos::kChromeUIMediaAppURL, web_contents->GetVisibleURL());
+
+  content::TestNavigationObserver observer(web_contents);
+  observer.WaitForNavigationFinished();
+  EXPECT_EQ(chromeos::kChromeUIMediaAppURL,
+            web_contents->GetLastCommittedURL());
+
+  content::WebUI* web_ui = web_contents->GetCommittedWebUI();
+  ASSERT_TRUE(web_ui);
+  EXPECT_TRUE(web_ui->GetController());
+
+  // A completed navigation could change the window title. Check again.
+  EXPECT_EQ(base::ASCIIToUTF16("Media App"),
+            app_browser->window()->GetNativeWindow()->GetTitle());
+}
+
+}  // namespace web_app

chrome/browser/web_applications/system_web_app_manager.cc

@@ -31,6 +31,7 @@
 #if defined(OS_CHROMEOS)
 #include "ash/public/cpp/app_list/internal_app_id_constants.h"
 #include "chrome/browser/chromeos/extensions/default_web_app_ids.h"
+#include "chromeos/components/media_app_ui/url_constants.h"
 #include "chromeos/constants/chromeos_features.h"
 #endif  // defined(OS_CHROMEOS)
 
@@ -72,6 +73,10 @@ base::flat_map<SystemAppType, SystemAppInfo> CreateSystemWebApps() {
     constexpr char kChromeTerminalPWAURL[] = "chrome://terminal/html/pwa.html";
     infos[SystemAppType::TERMINAL].install_url = GURL(kChromeTerminalPWAURL);
   }
+  if (SystemWebAppManager::IsAppEnabled(SystemAppType::MEDIA)) {
+    constexpr char kChromeMediaAppURL[] = "chrome://media-app/pwa.html";
+    infos[SystemAppType::MEDIA].install_url = {GURL(kChromeMediaAppURL)};
+  }
 #endif  // OS_CHROMEOS
 
   return infos;
@@ -121,6 +126,8 @@ bool SystemWebAppManager::IsAppEnabled(SystemAppType type) {
           chromeos::features::kCameraSystemWebApp);
     case SystemAppType::TERMINAL:
       return base::FeatureList::IsEnabled(features::kTerminalSystemApp);
+    case SystemAppType::MEDIA:
+      return base::FeatureList::IsEnabled(chromeos::features::kMediaApp);
   }
 #else
   return false;

chrome/browser/web_applications/system_web_app_manager.h

@@ -41,6 +41,7 @@ enum class SystemAppType {
   DISCOVER,
   CAMERA,
   TERMINAL,
+  MEDIA,
 };
 
 // The configuration options for a System App.

chromeos/components/media_app_ui/media_app_ui.cc

@@ -18,6 +18,9 @@ content::WebUIDataSource* CreateHostDataSource() {
   content::WebUIDataSource* source =
       content::WebUIDataSource::Create(kChromeUIMediaAppHost);
   source->SetDefaultResource(IDR_MEDIA_APP_INDEX_HTML);
+  source->AddResourcePath("pwa.html", IDR_MEDIA_APP_PWA_HTML);
+  source->AddResourcePath("manifest.json", IDR_MEDIA_APP_MANIFEST);
+  source->AddResourcePath("assets/app_icon_256.png", IDR_MEDIA_APP_ICON_256);
   return source;
 }
 

chromeos/components/media_app_ui/resources/manifest.json

+{
+  "name": "Media App",
+  "short_name": "Media App",
+  "start_url": "/",
+  "display": "standalone",
+  "theme_color": "#202124",
+  "background_color": "#3c4043",
+  "icons": [
+    {
+      "src": "assets/app_icon_256.png",
+      "sizes": "256x256",
+      "type": "image/png"
+    }
+  ]
+}

chromeos/components/media_app_ui/resources/pwa.html

+<!-- Copyright 2019 The Chromium Authors. All rights reserved.
+     Use of this source code is governed by a BSD-style license that can be
+     found in the LICENSE file. -->
+<!-- TODO(crbug/896575): Delete when manifest URL installs are implemented. -->
+<!DOCTYPE html>
+<link rel="manifest" href="manifest.json">

chromeos/resources/media_app_resources.grdp

 <?xml version="1.0" encoding="utf-8"?>
 <grit-part>
+  <!-- Privileged app host contents. -->
   <include name="IDR_MEDIA_APP_INDEX_HTML"
       file="../components/media_app_ui/resources/index.html"
       type="BINDATA" />
+  <include name="IDR_MEDIA_APP_PWA_HTML"
+      file="../components/media_app_ui/resources/pwa.html"
+      type="BINDATA" />
+  <include name="IDR_MEDIA_APP_MANIFEST"
+      file="../components/media_app_ui/resources/manifest.json"
+      type="BINDATA" />
+
+  <!-- Unprivileged guest contents. -->
   <include name="IDR_MEDIA_APP_APP_HTML"
       file="../components/media_app_ui/resources/app.html"
       type="BINDATA" />
@@ -17,6 +26,11 @@
       <include name="IDR_MEDIA_APP_DROP_TARGET_MODULE_JS"
           file="../components/media_app_ui/resources/app/app/js/app_drop_target_module.js"
           type="BINDATA" />
+
+      <!-- Note the icon is fetched from the privileged origin. -->
+      <include name="IDR_MEDIA_APP_ICON_256"
+            file="../components/media_app_ui/resources/app/app/assets/app_icon_256.png"
+            type="BINDATA" />
     </then>
     <else>
       <include name="IDR_MEDIA_APP_APP_JS"
@@ -28,6 +42,11 @@
       <include name="IDR_MEDIA_APP_DROP_TARGET_MODULE_JS"
           file="../components/media_app_ui/resources/mock/js/mock_module.js"
           type="BINDATA" />
+
+      <!-- Use a placeholder icon when enable_cros_media_app = false. -->
+      <include name="IDR_MEDIA_APP_ICON_256"
+            file="../../ui/file_manager/gallery/images/icon256.png"
+            type="BINDATA" />
     </else>
   </if>
 </grit-part>