Modify ArcApps to call CreateBlockDialog.

Update related unit tests, and not call CreateBlockDialog for unit
tests, because below unit tests are broken if create CreateBlockDialog
due to BubbleDialogDelegateView related error.
All/ArcAppModelBuilderTest.IconLoaderForSuspendedApps/*
AppServiceWrapperTest.ArcAppDisabled

So workaround it by not showing CreateBlockDialog.

BUG=1035136

Change-Id: Id5e4af8cc575f25c1edcdce576b59d46ff1e667b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2061576Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Trent Apted <tapted@chromium.org>
Reviewed-by: Aga Wronska <agawronska@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Commit-Queue: Nancy Wang <nancylingwang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743868}

chrome/browser/apps/app_service/app_service_proxy.cc

@@ -351,6 +351,11 @@ void AppServiceProxy::ReInitializeCrostiniForTesting(Profile* profile) {
 #endif
 }
 
+void AppServiceProxy::SetDialogCreatedCallbackForTesting(
+    base::OnceClosure callback) {
+  dialog_created_callback_ = std::move(callback);
+}
+
 std::vector<std::string> AppServiceProxy::GetAppIdsForUrl(const GURL& url) {
   return GetAppIdsForIntent(apps_util::CreateIntentFromUrl(url));
 }
@@ -511,6 +516,10 @@ void AppServiceProxy::OnLoadIconForPauseDialog(
       app_name, icon_value->uncompressed, pause_data,
       base::BindOnce(&AppServiceProxy::OnPauseDialogClosed,
                      weak_ptr_factory_.GetWeakPtr(), app_type, app_id));
+
+  if (!dialog_created_callback_.is_null()) {
+    std::move(dialog_created_callback_).Run();
+  }
 #endif  // OS_CHROMEOS
 }
 

chrome/browser/apps/app_service/app_service_proxy.h

@@ -9,6 +9,7 @@
 #include <memory>
 #include <set>
 
+#include "base/callback.h"
 #include "base/containers/unique_ptr_adapters.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
@@ -147,6 +148,7 @@ class AppServiceProxy : public KeyedService,
   apps::IconLoader* OverrideInnerIconLoaderForTesting(
       apps::IconLoader* icon_loader);
   void ReInitializeCrostiniForTesting(Profile* profile);
+  void SetDialogCreatedCallbackForTesting(base::OnceClosure callback);
 
   // Returns a list of apps (represented by their ids) which can handle |url|.
   std::vector<std::string> GetAppIdsForUrl(const GURL& url);
@@ -331,6 +333,8 @@ class AppServiceProxy : public KeyedService,
                                     base::UniquePtrComparator>;
   UninstallDialogs uninstall_dialogs_;
 
+  base::OnceClosure dialog_created_callback_;
+
   base::WeakPtrFactory<AppServiceProxy> weak_ptr_factory_{this};
 
   DISALLOW_COPY_AND_ASSIGN(AppServiceProxy);

chrome/browser/apps/app_service/app_service_test.cc

@@ -8,6 +8,11 @@
 #include "chrome/browser/apps/app_service/app_service_proxy.h"
 #include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
 
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/apps/app_service/arc_apps.h"
+#include "chrome/browser/apps/app_service/arc_apps_factory.h"
+#endif  // OS_CHROMEOS
+
 namespace apps {
 
 AppServiceTest::AppServiceTest() = default;
@@ -15,6 +20,7 @@ AppServiceTest::AppServiceTest() = default;
 AppServiceTest::~AppServiceTest() = default;
 
 void AppServiceTest::SetUp(Profile* profile) {
+  profile_ = profile;
   app_service_proxy_ = apps::AppServiceProxyFactory::GetForProfile(profile);
   DCHECK(app_service_proxy_);
   app_service_proxy_->ReInitializeForTesting(profile);
@@ -61,4 +67,12 @@ void AppServiceTest::FlushMojoCalls() {
   }
 }
 
+#if defined(OS_CHROMEOS)
+void AppServiceTest::SetArcAppsUseTestingProfile() {
+  ArcAppsFactory::GetInstance()
+      ->GetForProfile(profile_)
+      ->SetUseTestingProfile();
+}
+#endif  // OS_CHROMEOS
+
 }  // namespace apps

chrome/browser/apps/app_service/app_service_test.h

@@ -35,9 +35,17 @@ class AppServiceTest {
   // Flush mojo calls to allow AppService async callbacks to run.
   void FlushMojoCalls();
 
+#if defined(OS_CHROMEOS)
+  // Set the flag in ArcApps to indicate the test environment and not create the
+  // dialog.
+  void SetArcAppsUseTestingProfile();
+#endif  // OS_CHROMEOS
+
  private:
   AppServiceProxy* app_service_proxy_ = nullptr;
 
+  Profile* profile_ = nullptr;
+
   DISALLOW_COPY_AND_ASSIGN(AppServiceTest);
 };
 

chrome/browser/apps/app_service/arc_apps.cc

@@ -9,7 +9,6 @@
 
 #include "ash/public/cpp/app_menu_constants.h"
 #include "base/bind.h"
-#include "base/callback.h"
 #include "base/containers/flat_map.h"
 #include "base/feature_list.h"
 #include "base/metrics/histogram_macros.h"
@@ -276,6 +275,14 @@ ArcApps::ArcApps(Profile* profile, apps::AppServiceProxy* proxy)
 
 ArcApps::~ArcApps() = default;
 
+void ArcApps::SetUseTestingProfile() {
+  is_using_testing_profile_ = true;
+}
+
+void ArcApps::SetDialogCreatedCallbackForTesting(base::OnceClosure callback) {
+  dialog_created_callback_ = std::move(callback);
+}
+
 void ArcApps::Shutdown() {
   // Disconnect the observee-observer connections that we made during the
   // constructor.
@@ -636,13 +643,30 @@ void ArcApps::OnAppRegistered(const std::string& app_id,
 void ArcApps::OnAppStatesChanged(const std::string& app_id,
                                  const ArcAppListPrefs::AppInfo& app_info) {
   ArcAppListPrefs* prefs = ArcAppListPrefs::Get(profile_);
-  if (prefs) {
-    Publish(Convert(prefs, app_id, app_info));
+  if (!prefs) {
+    return;
+  }
+
+  if (!app_info.suspended) {
+    blocked_apps_.erase(app_id);
+  }
+
+  if (!base::Contains(blocked_apps_, app_id) && app_info.suspended &&
+      !is_using_testing_profile_) {
+    LoadIconForBlockDialog(app_id, app_info);
   }
+
+  if (app_info.suspended) {
+    blocked_apps_.insert(app_id);
+  }
+
+  Publish(Convert(prefs, app_id, app_info));
 }
 
 void ArcApps::OnAppRemoved(const std::string& app_id) {
   paused_apps_.MaybeRemoveApp(app_id);
+  blocked_apps_.erase(app_id);
+
   if (base::Contains(app_id_to_task_ids_, app_id)) {
     for (int task_id : app_id_to_task_ids_[app_id]) {
       task_id_to_app_id_.erase(task_id);
@@ -1028,4 +1052,30 @@ void ArcApps::UpdateAppIntentFilters(
   }
 }
 
+void ArcApps::LoadIconForBlockDialog(const std::string& app_id,
+                                     const ArcAppListPrefs::AppInfo& app_info) {
+  apps::mojom::IconKeyPtr icon_key =
+      icon_key_factory_.MakeIconKey(IconEffects::kNone);
+  constexpr bool kAllowPlaceholderIcon = false;
+  constexpr int32_t kPauseIconSize = 48;
+  LoadIcon(app_id, icon_key_factory_.MakeIconKey(IconEffects::kNone),
+           apps::mojom::IconCompression::kUncompressed, kPauseIconSize,
+           kAllowPlaceholderIcon,
+           base::BindOnce(&ArcApps::OnLoadIconForBlockDialog,
+                          weak_ptr_factory_.GetWeakPtr(), app_info.name));
+}
+
+void ArcApps::OnLoadIconForBlockDialog(const std::string& app_name,
+                                       apps::mojom::IconValuePtr icon_value) {
+  if (icon_value->icon_compression !=
+      apps::mojom::IconCompression::kUncompressed) {
+    return;
+  }
+  CreateBlockDialog(app_name, icon_value->uncompressed, profile_);
+
+  if (!dialog_created_callback_.is_null()) {
+    std::move(dialog_created_callback_).Run();
+  }
+}
+
 }  // namespace apps

chrome/browser/apps/app_service/arc_apps.h

@@ -10,7 +10,7 @@
 #include <string>
 #include <vector>
 
-#include "base/callback_forward.h"
+#include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
 #include "base/optional.h"
@@ -51,9 +51,13 @@ class ArcApps : public KeyedService,
 
   ~ArcApps() override;
 
+  void SetUseTestingProfile();
+  void SetDialogCreatedCallbackForTesting(base::OnceClosure callback);
+
  private:
   using AppIdToTaskIds = std::map<std::string, std::set<int>>;
   using TaskIdToAppId = std::map<int, std::string>;
+  using BlockedApps = std::set<std::string>;
 
   static void CreateBlockDialog(const std::string& app_name,
                                 const gfx::ImageSkia& image,
@@ -145,6 +149,13 @@ class ArcApps : public KeyedService,
       arc::ArcIntentHelperBridge* intent_helper_bridge,
       std::vector<apps::mojom::IntentFilterPtr>* intent_filters);
 
+  void LoadIconForBlockDialog(const std::string& app_id,
+                              const ArcAppListPrefs::AppInfo& app_info);
+
+  // Callback invoked when the icon is loaded.
+  void OnLoadIconForBlockDialog(const std::string& app_name,
+                                apps::mojom::IconValuePtr icon_value);
+
   mojo::Receiver<apps::mojom::Publisher> receiver_{this};
   mojo::RemoteSet<apps::mojom::Subscriber> subscribers_;
 
@@ -155,9 +166,14 @@ class ArcApps : public KeyedService,
 
   PausedApps paused_apps_;
 
+  BlockedApps blocked_apps_;
+
   AppIdToTaskIds app_id_to_task_ids_;
   TaskIdToAppId task_id_to_app_id_;
 
+  bool is_using_testing_profile_ = false;
+  base::OnceClosure dialog_created_callback_;
+
   ScopedObserver<arc::ArcIntentHelperBridge, arc::ArcIntentHelperObserver>
       arc_intent_helper_observer_{this};
 

chrome/browser/chromeos/child_accounts/time_limits/app_service_wrapper_unittest.cc

@@ -97,6 +97,7 @@ class AppServiceWrapperTest : public testing::Test {
 
     app_service_test_.SetUp(&profile_);
     arc_test_.SetUp(&profile_);
+    app_service_test_.SetArcAppsUseTestingProfile();
     task_environment_.RunUntilIdle();
 
     tested_wrapper_.AddObserver(&test_listener_);

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

@@ -28,6 +28,7 @@
 #include "build/build_config.h"
 #include "chrome/browser/apps/app_service/app_service_proxy.h"
 #include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
+#include "chrome/browser/apps/app_service/arc_apps.h"
 #include "chrome/browser/apps/app_service/arc_apps_factory.h"
 #include "chrome/browser/apps/app_service/arc_icon_once_loader.h"
 #include "chrome/browser/chromeos/arc/arc_optin_uma.h"
@@ -2029,6 +2030,9 @@ TEST_P(ArcAppModelBuilderTest, IconLoaderForShelfGroup) {
 
 // Test that icon is correctly updated for suspended/non-suspended app.
 TEST_P(ArcAppModelBuilderTest, IconLoaderForSuspendedApps) {
+  apps::ArcAppsFactory::GetInstance()
+      ->GetForProfile(profile_.get())
+      ->SetUseTestingProfile();
   arc::mojom::AppInfo app = fake_apps()[0];
   const std::string app_id = ArcAppTest::GetAppId(app);
 

chrome/browser/ui/views/apps/app_block_dialog_view.cc

@@ -11,6 +11,12 @@
 #include "ui/gfx/image/image_skia.h"
 #include "ui/views/window/dialog_delegate.h"
 
+namespace {
+
+AppBlockDialogView* g_app_block_dialog_view = nullptr;
+
+}  // namespace
+
 namespace apps {
 
 // static
@@ -24,6 +30,11 @@ void ArcApps::CreateBlockDialog(const std::string& app_name,
 
 }  // namespace apps
 
+// static
+AppBlockDialogView* AppBlockDialogView::GetActiveViewForTesting() {
+  return g_app_block_dialog_view;
+}
+
 AppBlockDialogView::AppBlockDialogView(const std::string& app_name,
                                        const gfx::ImageSkia& image,
                                        Profile* profile) {
@@ -33,9 +44,13 @@ AppBlockDialogView::AppBlockDialogView(const std::string& app_name,
       base::UTF8ToUTF16(app_name));
 
   InitializeView(image, heading_text);
+
+  g_app_block_dialog_view = this;
 }
 
-AppBlockDialogView::~AppBlockDialogView() = default;
+AppBlockDialogView::~AppBlockDialogView() {
+  g_app_block_dialog_view = nullptr;
+}
 
 base::string16 AppBlockDialogView::GetWindowTitle() const {
   return l10n_util::GetStringUTF16(IDS_APP_BLOCK_PROMPT_TITLE);

chrome/browser/ui/views/apps/app_block_dialog_view.h

@@ -20,6 +20,8 @@ class ImageSkia;
 // launched.
 class AppBlockDialogView : public AppDialogView {
  public:
+  static AppBlockDialogView* GetActiveViewForTesting();
+
   AppBlockDialogView(const std::string& app_name,
                      const gfx::ImageSkia& image,
                      Profile* profile);

chrome/browser/ui/views/apps/app_dialog_view_browsertest.cc

+// Copyright 2020 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 <memory>
+
+#include "base/bind.h"
+#include "base/command_line.h"
+#include "base/run_loop.h"
+#include "chrome/browser/apps/app_service/app_service_proxy.h"
+#include "chrome/browser/apps/app_service/app_service_proxy_factory.h"
+#include "chrome/browser/apps/app_service/arc_apps.h"
+#include "chrome/browser/apps/app_service/arc_apps_factory.h"
+#include "chrome/browser/chromeos/arc/arc_util.h"
+#include "chrome/browser/chromeos/arc/session/arc_session_manager.h"
+#include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/ui/app_list/arc/arc_app_icon.h"
+#include "chrome/browser/ui/app_list/arc/arc_app_list_prefs.h"
+#include "chrome/browser/ui/app_list/arc/arc_app_utils.h"
+#include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/test/test_browser_dialog.h"
+#include "chrome/browser/ui/views/apps/app_block_dialog_view.h"
+#include "chrome/browser/ui/views/apps/app_pause_dialog_view.h"
+#include "chrome/test/base/in_process_browser_test.h"
+#include "components/arc/arc_util.h"
+#include "components/arc/mojom/app.mojom.h"
+#include "components/arc/test/connection_holder_util.h"
+#include "components/arc/test/fake_app_instance.h"
+
+class AppDialogViewBrowserTest : public DialogBrowserTest {
+ public:
+  void SetUpCommandLine(base::CommandLine* command_line) override {
+    arc::SetArcAvailableCommandLineForTesting(command_line);
+  }
+
+  void SetUpInProcessBrowserTestFixture() override {
+    arc::ArcSessionManager::SetUiEnabledForTesting(false);
+  }
+
+  void SetUpOnMainThread() override {
+    profile_ = browser()->profile();
+
+    arc::SetArcPlayStoreEnabledForProfile(profile_, true);
+
+    // Validating decoded content does not fit well for unit tests.
+    ArcAppIcon::DisableSafeDecodingForTesting();
+
+    arc_app_list_pref_ = ArcAppListPrefs::Get(profile_);
+    DCHECK(arc_app_list_pref_);
+    base::RunLoop run_loop;
+    arc_app_list_pref_->SetDefaultAppsReadyCallback(run_loop.QuitClosure());
+    run_loop.Run();
+
+    app_instance_ = std::make_unique<arc::FakeAppInstance>(arc_app_list_pref_);
+    arc_app_list_pref_->app_connection_holder()->SetInstance(
+        app_instance_.get());
+    WaitForInstanceReady(arc_app_list_pref_->app_connection_holder());
+  }
+
+  void TearDownOnMainThread() override {
+    arc_app_list_pref_->app_connection_holder()->CloseInstance(
+        app_instance_.get());
+    app_instance_.reset();
+    arc::ArcSessionManager::Get()->Shutdown();
+  }
+
+  AppDialogView* ActiveView(const std::string& name) {
+    if (name == "block")
+      return AppBlockDialogView::GetActiveViewForTesting();
+
+    return AppPauseDialogView::GetActiveViewForTesting();
+  }
+
+  void ShowUi(const std::string& name) override {
+    arc::mojom::AppInfo app;
+    app.name = "Fake App 0";
+    app.package_name = "fake.package.0";
+    app.activity = "fake.app.0.activity";
+    app.sticky = false;
+    app_instance_->SendRefreshAppList(std::vector<arc::mojom::AppInfo>(1, app));
+    base::RunLoop().RunUntilIdle();
+
+    EXPECT_EQ(1u, arc_app_list_pref_->GetAppIds().size());
+    EXPECT_EQ(nullptr, ActiveView(name));
+
+    auto* app_service_proxy =
+        apps::AppServiceProxyFactory::GetForProfile(profile_);
+    ASSERT_TRUE(app_service_proxy);
+
+    if (name == "block") {
+      app.suspended = true;
+      app_instance_->SendRefreshAppList(
+          std::vector<arc::mojom::AppInfo>(1, app));
+    } else {
+      std::string app_id =
+          arc_app_list_pref_->GetAppId(app.package_name, app.activity);
+      std::map<std::string, apps::PauseData> pause_data;
+      pause_data[app_id].hours = 3;
+      pause_data[app_id].minutes = 30;
+      pause_data[app_id].should_show_pause_dialog = true;
+      app_service_proxy->PauseApps(pause_data);
+    }
+    base::RunLoop run_loop;
+    if (name == "block") {
+      apps::ArcAppsFactory::GetForProfile(profile_)
+          ->SetDialogCreatedCallbackForTesting(run_loop.QuitClosure());
+    } else {
+      app_service_proxy->SetDialogCreatedCallbackForTesting(
+          run_loop.QuitClosure());
+    }
+    run_loop.Run();
+
+    ASSERT_NE(nullptr, ActiveView(name));
+    EXPECT_EQ(ui::DIALOG_BUTTON_OK, ActiveView(name)->GetDialogButtons());
+
+    app_service_proxy->FlushMojoCallsForTesting();
+    std::string app_id =
+        arc_app_list_pref_->GetAppId(app.package_name, app.activity);
+    bool state_is_set = false;
+    app_service_proxy->AppRegistryCache().ForOneApp(
+        app_id, [&state_is_set, name](const apps::AppUpdate& update) {
+          if (name == "block") {
+            state_is_set = (update.Readiness() ==
+                            apps::mojom::Readiness::kDisabledByPolicy);
+          } else {
+            state_is_set =
+                (update.Paused() == apps::mojom::OptionalBool::kTrue);
+          }
+        });
+
+    EXPECT_TRUE(state_is_set);
+  }
+
+ private:
+  Profile* profile_ = nullptr;
+  ArcAppListPrefs* arc_app_list_pref_ = nullptr;
+  std::unique_ptr<arc::FakeAppInstance> app_instance_;
+};
+
+IN_PROC_BROWSER_TEST_F(AppDialogViewBrowserTest, InvokeUi_block) {
+  ShowAndVerifyUi();
+}
+
+IN_PROC_BROWSER_TEST_F(AppDialogViewBrowserTest, InvokeUi_pause) {
+  ShowAndVerifyUi();
+}

chrome/browser/ui/views/apps/app_pause_dialog_view.cc

@@ -12,6 +12,12 @@
 #include "ui/gfx/image/image_skia.h"
 #include "ui/views/window/dialog_delegate.h"
 
+namespace {
+
+AppPauseDialogView* g_app_pause_dialog_view = nullptr;
+
+}  // namespace
+
 // static
 void apps::AppServiceProxy::CreatePauseDialog(
     const std::string& app_name,
@@ -25,6 +31,11 @@ void apps::AppServiceProxy::CreatePauseDialog(
       ->Show();
 }
 
+// static
+AppPauseDialogView* AppPauseDialogView::GetActiveViewForTesting() {
+  return g_app_pause_dialog_view;
+}
+
 AppPauseDialogView::AppPauseDialogView(
     const std::string& app_name,
     const gfx::ImageSkia& image,
@@ -41,9 +52,13 @@ AppPauseDialogView::AppPauseDialogView(
               base::TimeDelta::FromMinutes(pause_data.minutes)));
 
   InitializeView(image, heading_text);
+
+  g_app_pause_dialog_view = this;
 }
 
-AppPauseDialogView::~AppPauseDialogView() = default;
+AppPauseDialogView::~AppPauseDialogView() {
+  g_app_pause_dialog_view = nullptr;
+}
 
 bool AppPauseDialogView::Accept() {
   std::move(closed_callback_).Run();

chrome/browser/ui/views/apps/app_pause_dialog_view.h

@@ -16,6 +16,8 @@ class ImageSkia;
 // the callback to notify AppService, which pauses the app.
 class AppPauseDialogView : public AppDialogView {
  public:
+  static AppPauseDialogView* GetActiveViewForTesting();
+
   AppPauseDialogView(
       const std::string& app_name,
       const gfx::ImageSkia& image,

chrome/test/BUILD.gn

@@ -2459,6 +2459,7 @@ if (!is_android) {
         "../browser/ui/ash/tablet_mode_page_behavior_browsertest.cc",
         "../browser/ui/ash/volume_controller_browsertest.cc",
         "../browser/ui/browser_finder_chromeos_browsertest.cc",
+        "../browser/ui/views/apps/app_dialog_view_browsertest.cc",
         "../browser/ui/views/apps/chrome_native_app_window_views_aura_ash_browsertest.cc",
         "../browser/ui/views/arc_app_dialog_view_browsertest.cc",
         "../browser/ui/views/crostini/crostini_ansible_software_config_view_browsertest.cc",