Update pinned item on shelf for app updates between Android and PWA.

When an app is pinned on the shelf and that app is updated from Android
app to PWA or from PWA to Android app, the shelf icon now updates to
a shortcut of the new app. This behaviour is fulfilled by the new
ReplacePinnedItem function in the ShelfModel.

BUG=986132

Change-Id: Ib8057c63ea257fc59017e2c906949f6481ac7db6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1781906Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Melissa Zhang <melzhang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#695942}

chrome/browser/chromeos/apps/apk_web_app_service.cc

@@ -12,8 +12,10 @@
 #include "chrome/browser/chromeos/apps/apk_web_app_service_factory.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 #include "chrome/browser/web_applications/components/externally_installed_web_app_prefs.h"
 #include "chrome/browser/web_applications/components/web_app_constants.h"
+#include "chrome/browser/web_applications/components/web_app_helpers.h"
 #include "chrome/common/chrome_features.h"
 #include "components/arc/mojom/app.mojom.h"
 #include "components/arc/session/connection_holder.h"
@@ -23,6 +25,7 @@
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/uninstall_reason.h"
 #include "extensions/common/extension.h"
+#include "url/gurl.h"
 
 namespace {
 
@@ -45,6 +48,8 @@ namespace {
 const char kWebAppToApkDictPref[] = "web_app_apks";
 const char kPackageNameKey[] = "package_name";
 const char kShouldRemoveKey[] = "should_remove";
+constexpr char kLastAppId[] = "last_app_id";
+constexpr char kPinIndex[] = "pin_index";
 
 // Default icon size in pixels to request from ARC for an icon.
 const int kDefaultIconSize = 192;
@@ -116,6 +121,60 @@ void ApkWebAppService::UninstallWebApp(const web_app::AppId& web_app_id) {
   }
 }
 
+void ApkWebAppService::UpdateShelfPin(
+    const arc::mojom::ArcPackageInfo* package_info) {
+  std::string new_app_id;
+  // Compute the current app id. It may have changed if the package has been
+  // updated from an Android app to a web app, or vice versa.
+  if (!package_info->web_app_info.is_null()) {
+    new_app_id = web_app::GenerateAppIdFromURL(
+        GURL(package_info->web_app_info->start_url));
+  } else {
+    // Get the first app in the package. If there are multiple apps in the
+    // package there is no way to determine which app is more suitable to
+    // replace the previous web app shortcut. For simplicity we will just use
+    // the first one.
+    std::unordered_set<std::string> apps =
+        arc_app_list_prefs_->GetAppsForPackage(package_info->package_name);
+    if (!apps.empty())
+      new_app_id = *apps.begin();
+  }
+
+  // Query for the old app id, which is cached in the package dict to ensure it
+  // isn't overwritten before this method can run.
+  const base::Value* last_app_id_value = arc_app_list_prefs_->GetPackagePrefs(
+      package_info->package_name, kLastAppId);
+
+  std::string last_app_id;
+  if (last_app_id_value && last_app_id_value->is_string())
+    last_app_id = last_app_id_value->GetString();
+
+  if (new_app_id != last_app_id && !new_app_id.empty()) {
+    arc_app_list_prefs_->SetPackagePrefs(package_info->package_name, kLastAppId,
+                                         base::Value(new_app_id));
+    if (!last_app_id.empty()) {
+      auto* launcher_controller = ChromeLauncherController::instance();
+      if (!launcher_controller)
+        return;
+      int index = launcher_controller->PinnedItemIndexByAppID(last_app_id);
+      // The previously installed app has been uninstalled or hidden, in this
+      // instance get the saved pin index and pin at that place.
+      if (index == ChromeLauncherController::kInvalidIndex) {
+        const base::Value* saved_index = arc_app_list_prefs_->GetPackagePrefs(
+            package_info->package_name, kPinIndex);
+        if (!(saved_index && saved_index->is_int()))
+          return;
+        launcher_controller->PinAppAtIndex(new_app_id, saved_index->GetInt());
+        arc_app_list_prefs_->SetPackagePrefs(
+            package_info->package_name, kPinIndex,
+            base::Value(ChromeLauncherController::kInvalidIndex));
+      } else {
+        launcher_controller->ReplacePinnedItem(last_app_id, new_app_id);
+      }
+    }
+  }
+}
+
 void ApkWebAppService::Shutdown() {
   // Can be null in tests.
   if (arc_app_list_prefs_) {
@@ -156,6 +215,10 @@ void ApkWebAppService::OnPackageInstalled(
   if (is_now_web_app == was_previously_web_app)
     return;
 
+  // Only call this function if there has been a state change from web app to
+  // Android app or vice-versa.
+  UpdateShelfPin(&package_info);
+
   if (was_previously_web_app) {
     // The package was a web app, but now isn't. Remove the web app.
     OnPackageRemoved(package_info.package_name, true /* uninstalled */);

chrome/browser/chromeos/apps/apk_web_app_service.h

@@ -61,6 +61,11 @@ class ApkWebAppService : public KeyedService,
   // ApkWebAppInstaller::Install().
   void UninstallWebApp(const web_app::AppId& web_app_id);
 
+  // If the app has updated from a web app to Android app or vice-versa,
+  // this function pins the new app in the old app's place on the shelf if it
+  // was pinned prior to the update.
+  void UpdateShelfPin(const arc::mojom::ArcPackageInfo* package_info);
+
   // KeyedService:
   void Shutdown() override;
 

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

@@ -32,6 +32,7 @@
 #include "chrome/browser/ui/app_list/arc/arc_default_app_list.h"
 #include "chrome/browser/ui/app_list/arc/arc_package_syncable_service.h"
 #include "chrome/browser/ui/app_list/arc/arc_pai_starter.h"
+#include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 #include "chrome/grit/generated_resources.h"
 #include "chromeos/constants/chromeos_features.h"
 #include "components/arc/arc_prefs.h"
@@ -65,6 +66,7 @@ constexpr char kName[] = "name";
 constexpr char kNotificationsEnabled[] = "notifications_enabled";
 constexpr char kPackageName[] = "package_name";
 constexpr char kPackageVersion[] = "package_version";
+constexpr char kPinIndex[] = "pin_index";
 constexpr char kPermissionStates[] = "permission_states";
 constexpr char kSticky[] = "sticky";
 constexpr char kShortcut[] = "shortcut";
@@ -1493,6 +1495,18 @@ void ArcAppListPrefs::OnPackageAppListRefreshed(
     AddApp(*app);
   }
 
+  arc::ArcAppScopedPrefUpdate update(prefs_, package_name,
+                                     arc::prefs::kArcPackages);
+  base::DictionaryValue* package_dict = update.Get();
+  if (!apps_to_remove.empty()) {
+    auto* launcher_controller = ChromeLauncherController::instance();
+    if (launcher_controller) {
+      int pin_index =
+          launcher_controller->PinnedItemIndexByAppID(*apps_to_remove.begin());
+      package_dict->SetInteger(kPinIndex, pin_index);
+    }
+  }
+
   for (const auto& app_id : apps_to_remove)
     RemoveApp(app_id);
 }

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

@@ -13,6 +13,7 @@
 #include <vector>
 
 #include "ash/public/cpp/app_list/app_list_config.h"
+#include "ash/public/cpp/shelf_model.h"
 #include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
@@ -51,6 +52,7 @@
 #include "chrome/browser/ui/app_list/test/fake_app_list_model_updater.h"
 #include "chrome/browser/ui/app_list/test/test_app_list_controller_delegate.h"
 #include "chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.h"
+#include "chrome/browser/ui/ash/launcher/chrome_launcher_controller.h"
 #include "chrome/test/base/testing_profile.h"
 #include "chromeos/constants/chromeos_switches.h"
 #include "components/arc/arc_prefs.h"
@@ -227,6 +229,8 @@ class ArcAppModelBuilderTest : public extensions::ExtensionServiceTestBase,
       arc::SetArcAlwaysStartWithoutPlayStoreForTesting();
     }
 
+    model_ = std::make_unique<ash::ShelfModel>();
+
     extensions::ExtensionServiceTestBase::SetUp();
     InitializeExtensionService(ExtensionServiceInitParams());
     service_->Init();
@@ -235,16 +239,25 @@ class ArcAppModelBuilderTest : public extensions::ExtensionServiceTestBase,
     arc_test_.SetUp(profile_.get());
     CreateBuilder();
 
+    CreateLauncherController()->Init();
+
     // Validating decoded content does not fit well for unit tests.
     ArcAppIcon::DisableSafeDecodingForTesting();
   }
 
   void TearDown() override {
+    launcher_controller_.reset();
     arc_test_.TearDown();
     ResetBuilder();
     extensions::ExtensionServiceTestBase::TearDown();
   }
 
+  ChromeLauncherController* CreateLauncherController() {
+    launcher_controller_ = std::make_unique<ChromeLauncherController>(
+        profile_.get(), model_.get());
+    return launcher_controller_.get();
+  }
+
  protected:
   // Notifies that initial preparation is done, profile is ready and it is time
   // to initialize ARC subsystem.
@@ -520,6 +533,8 @@ class ArcAppModelBuilderTest : public extensions::ExtensionServiceTestBase,
   std::unique_ptr<FakeAppListModelUpdater> model_updater_;
   std::unique_ptr<test::TestAppListControllerDelegate> controller_;
   std::unique_ptr<ArcAppModelBuilder> builder_;
+  std::unique_ptr<ChromeLauncherController> launcher_controller_;
+  std::unique_ptr<ash::ShelfModel> model_;
 
   DISALLOW_COPY_AND_ASSIGN(ArcAppModelBuilderTest);
 };

chrome/browser/ui/ash/launcher/chrome_launcher_controller.cc

@@ -789,6 +789,45 @@ void ChromeLauncherController::UnpinAppWithID(const std::string& app_id) {
   model_->UnpinAppWithID(app_id);
 }
 
+void ChromeLauncherController::ReplacePinnedItem(
+    const std::string& old_app_id,
+    const std::string& new_app_id) {
+  if (!model_->IsAppPinned(old_app_id))
+    return;
+  const int index = model_->ItemIndexByAppID(old_app_id);
+
+  const ash::ShelfID new_shelf_id(new_app_id);
+  ash::ShelfItem item;
+  item.type = ash::TYPE_PINNED_APP;
+  item.id = new_shelf_id;
+
+  // Remove old_app at index and replace with new app.
+  model_->RemoveItemAt(index);
+  model_->AddAt(index, item);
+}
+
+void ChromeLauncherController::PinAppAtIndex(const std::string& app_id,
+                                             int target_index) {
+  if (target_index < 0)
+    return;
+
+  const ash::ShelfID new_shelf_id(app_id);
+  ash::ShelfItem item;
+  item.type = ash::TYPE_PINNED_APP;
+  item.id = new_shelf_id;
+
+  model_->AddAt(target_index, item);
+}
+
+int ChromeLauncherController::PinnedItemIndexByAppID(
+    const std::string& app_id) {
+  if (model_->IsAppPinned(app_id)) {
+    ash::ShelfID shelf_id(app_id);
+    return model_->ItemIndexByID(shelf_id);
+  }
+  return kInvalidIndex;
+}
+
 AppIconLoader* ChromeLauncherController::GetAppIconLoaderForApp(
     const std::string& app_id) {
   for (const auto& app_icon_loader : app_icon_loaders_) {

chrome/browser/ui/ash/launcher/chrome_launcher_controller.h

@@ -67,6 +67,9 @@ class ChromeLauncherController
       private app_list::AppListSyncableService::Observer,
       private sync_preferences::PrefServiceSyncableObserver {
  public:
+  // The value used for indicating that an index position doesn't exist.
+  static const int kInvalidIndex = -1;
+
   // Returns the single ChromeLauncherController instance.
   static ChromeLauncherController* instance() { return instance_; }
 
@@ -247,6 +250,17 @@ class ChromeLauncherController
   bool IsAppPinned(const std::string& app_id);
   void UnpinAppWithID(const std::string& app_id);
 
+  // Unpins app item with |old_app_id| and pins app |new_app_id| in its place.
+  void ReplacePinnedItem(const std::string& old_app_id,
+                         const std::string& new_app_id);
+
+  // Pins app with |app_id| at |target_index|.
+  void PinAppAtIndex(const std::string& app_id, int target_index);
+
+  // Converts |app_id| to shelf_id and calls ShelfModel function ItemIndexbyID
+  // to get index of item with id |app_id| or -1 if it's not pinned.
+  int PinnedItemIndexByAppID(const std::string& app_id);
+
   // Whether the controller supports a Show App Info flow.
   bool CanDoShowAppInfoFlow();