cros: Replace app id with package name in arcAppsPrivate API

The app ID is calculated from the package name and activity, but
activity may change over time. Given that the caller may hard code the
app IDs and thus not aware of the ID change, we should change the API
to use package name, which remains unchanged.

Bug: 819404
Change-Id: If87351ae609ecaf82442adcfb3ea98a3922f0bda
Reviewed-on: https://chromium-review.googlesource.com/1164524
Commit-Queue: Wenzhao (Colin) Zang <wzang@chromium.org>
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581432}

chrome/browser/chromeos/extensions/arc_apps_private_api.cc

@@ -57,7 +57,7 @@ void ArcAppsPrivateAPI::OnAppRegistered(
   if (!app_info.launchable)
     return;
   api::arc_apps_private::AppInfo app_info_result;
-  app_info_result.id = app_id;
+  app_info_result.package_name = app_info.package_name;
   auto event = std::make_unique<Event>(
       events::ARC_APPS_PRIVATE_ON_INSTALLED,
       api::arc_apps_private::OnInstalled::kEventName,
@@ -84,7 +84,7 @@ ArcAppsPrivateGetLaunchableAppsFunction::Run() {
     std::unique_ptr<ArcAppListPrefs::AppInfo> app_info = prefs->GetApp(app_id);
     if (app_info && app_info->launchable) {
       api::arc_apps_private::AppInfo app_info_result;
-      app_info_result.id = app_id;
+      app_info_result.package_name = app_info->package_name;
       result.push_back(std::move(app_info_result));
     }
   }
@@ -100,11 +100,16 @@ ExtensionFunction::ResponseAction ArcAppsPrivateLaunchAppFunction::Run() {
   std::unique_ptr<api::arc_apps_private::LaunchApp::Params> params(
       api::arc_apps_private::LaunchApp::Params::Create(*args_));
   EXTENSION_FUNCTION_VALIDATE(params.get());
-  if (!ArcAppListPrefs::Get(Profile::FromBrowserContext(browser_context())))
+  ArcAppListPrefs* prefs =
+      ArcAppListPrefs::Get(Profile::FromBrowserContext(browser_context()));
+  if (!prefs)
     return RespondNow(Error("Not available"));
 
+  const std::string app_id = prefs->GetAppIdByPackageName(params->package_name);
+  if (app_id.empty())
+    return RespondNow(Error("App not found"));
   if (!arc::LaunchApp(
-          browser_context(), params->app_id, ui::EF_NONE,
+          browser_context(), app_id, ui::EF_NONE,
           arc::UserInteractionType::APP_STARTED_FROM_EXTENSION_API)) {
     return RespondNow(Error("Launch failed"));
   }

chrome/browser/chromeos/extensions/arc_apps_private_apitest.cc

@@ -66,7 +66,7 @@ class ArcAppsPrivateApiTest : public extensions::ExtensionApiTest {
   DISALLOW_COPY_AND_ASSIGN(ArcAppsPrivateApiTest);
 };
 
-IN_PROC_BROWSER_TEST_F(ArcAppsPrivateApiTest, GetAppIdAndLaunchApp) {
+IN_PROC_BROWSER_TEST_F(ArcAppsPrivateApiTest, GetPackageNameAndLaunchApp) {
   ArcAppListPrefs* prefs = ArcAppListPrefs::Get(browser()->profile());
   ASSERT_TRUE(prefs);
   std::unique_ptr<arc::FakeAppInstance> app_instance = CreateAppInstance(prefs);
@@ -81,11 +81,10 @@ IN_PROC_BROWSER_TEST_F(ArcAppsPrivateApiTest, GetAppIdAndLaunchApp) {
   arc::ArcServiceManager::Get()->arc_bridge_service()->app()->CloseInstance(
       app_instance.get());
   EXPECT_EQ(0u, app_instance->launch_requests().size());
-  // Verify |chrome.arcAppsPrivate.getLaunchableApps| returns the id of
-  // the launchable app only. The JS test will attempt to launch the app.
-  EXPECT_TRUE(RunPlatformAppTestWithArg(
-      "arc_app_launcher/launch_app",
-      ArcAppListPrefs::GetAppId("Package_0", "Dummy_activity_0").c_str()))
+  // Verify |chrome.arcAppsPrivate.getLaunchableApps| returns the package name
+  // of the launchable app only. The JS test will attempt to launch the app.
+  EXPECT_TRUE(
+      RunPlatformAppTestWithArg("arc_app_launcher/launch_app", "Package_0"))
       << message_;
 
   // Verify the app is not launched because it's not ready.
@@ -106,7 +105,7 @@ IN_PROC_BROWSER_TEST_F(ArcAppsPrivateApiTest, OnInstalled) {
 
   // The JS test will observe the onInstalled event and attempt to launch the
   // newly installed app.
-  SetCustomArg(ArcAppListPrefs::GetAppId("Package_0", "Dummy_activity_0"));
+  SetCustomArg("Package_0");
   extensions::ResultCatcher catcher;
   ExtensionTestMessageListener ready_listener("ready", false);
 

chrome/common/extensions/api/arc_apps_private.idl

@@ -9,23 +9,23 @@
 namespace arcAppsPrivate {
 
   dictionary AppInfo {
-    // The app id.
-    DOMString id;
+    // The app package name.
+    DOMString packageName;
   };
 
   callback VoidCallback = void ();
-  callback GetLaunchableAppsCallback = void (AppInfo[] apps_info);
+  callback GetLaunchableAppsCallback = void (AppInfo[] appsInfo);
 
   interface Functions {
     // Returns info of the installed ARC apps that are launchable, including
     // ready and non-ready apps.
     static void getLaunchableApps(GetLaunchableAppsCallback callback);
 
-    // Launches the ARC app with |app_id|, which must be returned by
-    // |getLaunchableApps|. The app is launched immediately if it's ready,
-    // otherwise it will be launched when it becomes ready. The callback is
-    // called as soon as the launch is scheduled.
-    static void launchApp(DOMString app_id, optional VoidCallback callback);
+    // Launches the ARC app with its package name. The app is launched
+    // immediately if it's ready, otherwise it will be launched when it becomes
+    // ready. The callback is called as soon as the launch is scheduled.
+    static void launchApp(DOMString packageName,
+                          optional VoidCallback callback);
   };
 
   interface Events {

chrome/test/data/extensions/api_test/arc_app_launcher/install_event/test.js

@@ -2,21 +2,22 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-var EXPECTED_APP_ID = null;
+var expectedPackageName = null;
 
 chrome.test.runTests([
   function getConfig() {
     chrome.test.getConfig(chrome.test.callbackPass(config => {
-      EXPECTED_APP_ID = config.customArg;
+      expectedPackageName = config.customArg;
     }));
   },
 
   function onInstalled() {
-    chrome.test.assertTrue(!!EXPECTED_APP_ID);
+    chrome.test.assertTrue(!!expectedPackageName);
     chrome.test.listenOnce(chrome.arcAppsPrivate.onInstalled, appInfo => {
-      chrome.test.assertEq({id: EXPECTED_APP_ID}, appInfo);
-      chrome.arcAppsPrivate.launchApp(appInfo.id, chrome.test.callbackPass());
+      chrome.test.assertEq({packageName: expectedPackageName}, appInfo);
+      chrome.arcAppsPrivate.launchApp(
+          appInfo.packageName, chrome.test.callbackPass());
     });
     chrome.test.sendMessage('ready');
   }
-]);
+]);
\ No newline at end of file

chrome/test/data/extensions/api_test/arc_app_launcher/launch_app/test.js

@@ -3,24 +3,25 @@
 // found in the LICENSE file.
 
 chrome.app.runtime.onLaunched.addListener(function() {
-  var EXPECTED_APP_ID = null;
+  var expectedPackageName = null;
 
   chrome.test.runTests([
     function getConfig() {
       chrome.test.getConfig(chrome.test.callbackPass(config => {
-        EXPECTED_APP_ID = config.customArg;
+        expectedPackageName = config.customArg;
       }));
     },
 
-    function getAppIdAndLaunchApp() {
+    function getPackageNameAndLaunchApp() {
       chrome.arcAppsPrivate.launchApp(
-          'invalid app id', chrome.test.callbackFail('Launch failed'));
-      chrome.test.assertTrue(!!EXPECTED_APP_ID);
+          'invalid package name', chrome.test.callbackFail('App not found'));
+      chrome.test.assertTrue(!!expectedPackageName);
       chrome.arcAppsPrivate.getLaunchableApps(
           chrome.test.callbackPass(appsInfo => {
-            chrome.test.assertEq([{id: EXPECTED_APP_ID}], appsInfo);
+            chrome.test.assertEq(
+                [{packageName: expectedPackageName}], appsInfo);
             chrome.arcAppsPrivate.launchApp(
-                appsInfo[0].id, chrome.test.callbackPass());
+                appsInfo[0].packageName, chrome.test.callbackPass());
           }));
     }
   ]);