Support uninstalling ARC app from Chrome launcher

chrome/browser/ui/app_list/arc/arc_app_utils.cc

Index: chrome/browser/ui/app_list/arc/arc_app_utils.cc
diff --git a/chrome/browser/ui/app_list/arc/arc_app_utils.cc b/chrome/browser/ui/app_list/arc/arc_app_utils.cc
index 3535fd77b1291f5be071671d43a4528f5a2dd95e..2ce1617e72ee8a57f8d1b875c17c77f75b5be6a5 100644
--- a/chrome/browser/ui/app_list/arc/arc_app_utils.cc
+++ b/chrome/browser/ui/app_list/arc/arc_app_utils.cc
@@ -6,6 +6,7 @@
 
 #include "chrome/browser/ui/app_list/arc/arc_app_list_prefs.h"
 #include "components/arc/arc_bridge_service.h"
+#include "mojo/public/cpp/bindings/string.h"
 
 namespace arc {
 
@@ -20,12 +21,12 @@ bool LaunchApp(content::BrowserContext* context, const std::string& app_id) {
 
   scoped_ptr<ArcAppListPrefs::AppInfo> app_info = prefs->GetApp(app_id);
   if (!app_info) {
-    VLOG(2) << "Cannot launch unavailable app:" << app_id << ".";
+    VLOG(2) << "Cannot launch unavailable app: " << app_id << ".";
     return false;
   }
 
   if (!app_info->ready) {
-    VLOG(2) << "Cannot launch not-ready app:" << app_id << ".";
+    VLOG(2) << "Cannot launch not-ready app: " << app_id << ".";
     return false;
   }
 
@@ -61,7 +62,7 @@ bool LaunchApp(content::BrowserContext* context, const std::string& app_id) {
 bool CanHandleResolution(content::BrowserContext* context,
     const std::string& app_id,
     const gfx::Rect& rect,
-    const CanHandleResolutionCallback callback) {
+    const CanHandleResolutionCallback& callback) {
   ArcAppListPrefs* prefs = ArcAppListPrefs::Get(context);
   scoped_ptr<ArcAppListPrefs::AppInfo> app_info = prefs->GetApp(app_id);
   if (!app_info) {
@@ -98,4 +99,37 @@ bool CanHandleResolution(content::BrowserContext* context,
   return true;
 }
 
+bool UninstallApp(const std::string& package_name,

[Luis Héctor Chávez, 2016/03/14 16:21:10]

Once you implement OnUninstallAppResponse correctly, all synchronous failures
(the bridge being unavilable, or not having the correct version) won't be
handled by the same path.

So, how about instead of making UninstallApp() bool, you make it void and
instead of logging errors here, you relay them to OnUninstallAppResponse? You'll
need to i18n the messages, but you can mark that as TODO. Alternatively if there
is already a pre-i18n'd message, you can relay that one and keep the verbose
logging here.

[victorhsieh0, 2016/03/15 17:26:33]

Per discussion, we won't report the error (which should really be internal error
since we should not add uninstall to menu at the first place) like chrome does
in many cases.

+                  const UninstallAppCallback& callback) {
+  VLOG(2) << "Uninstalling " << package_name;
+  arc::ArcBridgeService* bridge_service = arc::ArcBridgeService::Get();
+  if (!bridge_service) {
+    VLOG(2) << "Request to uninstall app when bridge service is not ready: "
+            << package_name << ".";
+    return false;
+  }
+  arc::AppInstance* app_instance = bridge_service->app_instance();
+  if (!app_instance) {
+    VLOG(2) << "Request to uninstall app when bridge service is not ready: "
+            << package_name << ".";
+    return false;
+  }
+
+  if (bridge_service->app_version() < 2) {
+    LOG(ERROR) << "Request to uninstall app when version "
+               << bridge_service->app_version() << " does not support it";
+    return false;
+  }
+
+  app_instance->UninstallApp(package_name, callback);
+  return true;
+}
+
+void OnUninstallAppResponse(const mojo::String& error_message) {
+  if (!error_message.is_null()) {
+    // TODO(victorhsieh): Notify user about the error.
+    LOG(ERROR) << "Failed to uninstall package: " << error_message.get();
+  }
+}
+
 }  // namespace arc