From 959bbb345b4af70293943f8e8fda7c042b32fa73 Mon Sep 17 00:00:00 2001 From: Alexander Bolodurin Date: Fri, 8 Oct 2021 04:05:19 +0000 Subject: [PATCH] [lacros shelf] check WebAppsCrosapi in Lacros the right way Ash flags aren't propagated to Lacros automatically, but some are passed as init parameters. WebAppsCrosapi is one of them, so check it instead of using the flag directly. Also remove factory methods, as they can't be shared between Ash and Lacros anymore. Bug: 1203992 Bug: 1257872 Change-Id: Idcd656d05a646ec79e2c91acbe0ae3ba23750261 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3212272 Reviewed-by: Dominick Ng Commit-Queue: Alexander Bolodurin Cr-Commit-Position: refs/heads/main@{#929561} --- .../app_service/app_service_proxy_chromeos.cc | 14 +++++++++----- .../apps/app_service/app_service_proxy_lacros.cc | 15 ++++++++++----- .../app_service/browser_app_instance_forwarder.cc | 9 --------- .../app_service/browser_app_instance_forwarder.h | 9 --------- .../app_service/browser_app_instance_registry.cc | 9 --------- .../app_service/browser_app_instance_registry.h | 7 ------- .../app_service/browser_app_instance_tracker.cc | 11 ----------- .../app_service/browser_app_instance_tracker.h | 8 -------- 8 files changed, 19 insertions(+), 63 deletions(-) diff --git a/chrome/browser/apps/app_service/app_service_proxy_chromeos.cc b/chrome/browser/apps/app_service/app_service_proxy_chromeos.cc index 76eb3d7cedf308..dcb089f7aedb4e 100644 --- a/chrome/browser/apps/app_service/app_service_proxy_chromeos.cc +++ b/chrome/browser/apps/app_service/app_service_proxy_chromeos.cc @@ -48,11 +48,15 @@ bool g_omit_plugin_vm_apps_for_testing_ = false; } // anonymous namespace AppServiceProxyChromeOs::AppServiceProxyChromeOs(Profile* profile) - : AppServiceProxyBase(profile), - browser_app_instance_tracker_( - BrowserAppInstanceTracker::Create(profile_, app_registry_cache_)), - browser_app_instance_registry_(BrowserAppInstanceRegistry::Create( - browser_app_instance_tracker_.get())) { + : AppServiceProxyBase(profile) { + if (features::IsBrowserAppInstanceTrackingEnabled()) { + browser_app_instance_tracker_ = + std::make_unique(profile_, + app_registry_cache_); + browser_app_instance_registry_ = + std::make_unique( + *browser_app_instance_tracker_); + } Initialize(); } diff --git a/chrome/browser/apps/app_service/app_service_proxy_lacros.cc b/chrome/browser/apps/app_service/app_service_proxy_lacros.cc index 0017b4b8dc5e0e..3038a0b9e9699d 100644 --- a/chrome/browser/apps/app_service/app_service_proxy_lacros.cc +++ b/chrome/browser/apps/app_service/app_service_proxy_lacros.cc @@ -96,11 +96,16 @@ AppServiceProxyLacros::AppServiceProxyLacros(Profile* profile) icon_coalescer_(&inner_icon_loader_), outer_icon_loader_(&icon_coalescer_, apps::IconCache::GarbageCollectionPolicy::kEager), - profile_(profile), - browser_app_instance_tracker_( - BrowserAppInstanceTracker::Create(profile_, app_registry_cache_)), - browser_app_instance_forwarder_(BrowserAppInstanceForwarder::Create( - browser_app_instance_tracker_.get())) { + profile_(profile) { + auto* service = chromeos::LacrosService::Get(); + if (service && service->init_params()->web_apps_enabled) { + browser_app_instance_tracker_ = + std::make_unique(profile_, + app_registry_cache_); + browser_app_instance_forwarder_ = + std::make_unique( + *browser_app_instance_tracker_); + } Initialize(); } diff --git a/chrome/browser/apps/app_service/browser_app_instance_forwarder.cc b/chrome/browser/apps/app_service/browser_app_instance_forwarder.cc index 73b569d0499892..711ab1469fe51d 100644 --- a/chrome/browser/apps/app_service/browser_app_instance_forwarder.cc +++ b/chrome/browser/apps/app_service/browser_app_instance_forwarder.cc @@ -12,7 +12,6 @@ #include "chrome/browser/lacros/window_utility.h" #include "chrome/browser/ui/browser.h" #include "chrome/browser/ui/browser_window.h" -#include "chrome/common/chrome_features.h" #include "chromeos/crosapi/mojom/app_service.mojom.h" #include "chromeos/crosapi/mojom/browser_app_instance_registry.mojom.h" #include "chromeos/lacros/lacros_service.h" @@ -30,14 +29,6 @@ BrowserAppInstanceForwarder::BrowserAppInstanceForwarder( } BrowserAppInstanceForwarder::~BrowserAppInstanceForwarder() = default; -std::unique_ptr -BrowserAppInstanceForwarder::Create(BrowserAppInstanceTracker* tracker) { - if (!features::IsBrowserAppInstanceTrackingEnabled()) { - return nullptr; - } - return std::make_unique(*tracker); -} - void BrowserAppInstanceForwarder::OnBrowserWindowAdded( const apps::BrowserWindowInstance& instance) { registry_->OnBrowserWindowAdded(instance.ToUpdate()); diff --git a/chrome/browser/apps/app_service/browser_app_instance_forwarder.h b/chrome/browser/apps/app_service/browser_app_instance_forwarder.h index 4a368b99037f18..d2cc486cfb3bc0 100644 --- a/chrome/browser/apps/app_service/browser_app_instance_forwarder.h +++ b/chrome/browser/apps/app_service/browser_app_instance_forwarder.h @@ -5,8 +5,6 @@ #ifndef CHROME_BROWSER_APPS_APP_SERVICE_BROWSER_APP_INSTANCE_FORWARDER_H_ #define CHROME_BROWSER_APPS_APP_SERVICE_BROWSER_APP_INSTANCE_FORWARDER_H_ -#include - #include "base/scoped_observation.h" #include "chrome/browser/apps/app_service/browser_app_instance_observer.h" #include "chrome/browser/apps/app_service/browser_app_instance_tracker.h" @@ -25,13 +23,6 @@ class BrowserAppInstanceForwarder : public apps::BrowserAppInstanceObserver, public crosapi::mojom::BrowserAppInstanceController { public: - // A factory method to make the creation of the forwarder optional to keep it - // behind a flag. - // TODO(crbug.com/1203992): Remove this when the |kBrowserAppInstanceTracking| - // flag is removed. - static std::unique_ptr Create( - BrowserAppInstanceTracker* tracker); - explicit BrowserAppInstanceForwarder(BrowserAppInstanceTracker& tracker); ~BrowserAppInstanceForwarder() override; diff --git a/chrome/browser/apps/app_service/browser_app_instance_registry.cc b/chrome/browser/apps/app_service/browser_app_instance_registry.cc index 8afeff7b20884d..ab2f781989e067 100644 --- a/chrome/browser/apps/app_service/browser_app_instance_registry.cc +++ b/chrome/browser/apps/app_service/browser_app_instance_registry.cc @@ -11,7 +11,6 @@ #include "chrome/browser/apps/app_service/browser_app_instance_map.h" #include "chrome/browser/ash/crosapi/browser_util.h" #include "chrome/browser/ui/browser_window.h" -#include "chrome/common/chrome_features.h" #include "components/exo/shell_surface_util.h" #include "ui/views/widget/widget.h" #include "ui/wm/public/activation_change_observer.h" @@ -46,14 +45,6 @@ BrowserAppInstanceRegistry::BrowserAppInstanceRegistry( BrowserAppInstanceRegistry::~BrowserAppInstanceRegistry() = default; -std::unique_ptr BrowserAppInstanceRegistry::Create( - BrowserAppInstanceTracker* ash_instance_tracker) { - if (!features::IsBrowserAppInstanceTrackingEnabled()) { - return nullptr; - } - return std::make_unique(*ash_instance_tracker); -} - std::set BrowserAppInstanceRegistry::GetAppInstancesByAppId( const std::string& app_id) const { diff --git a/chrome/browser/apps/app_service/browser_app_instance_registry.h b/chrome/browser/apps/app_service/browser_app_instance_registry.h index 428acb7e7c8848..4e75ab62887282 100644 --- a/chrome/browser/apps/app_service/browser_app_instance_registry.h +++ b/chrome/browser/apps/app_service/browser_app_instance_registry.h @@ -50,13 +50,6 @@ class BrowserAppInstanceRegistry BrowserAppInstanceTracker& ash_instance_tracker); ~BrowserAppInstanceRegistry() override; - // A factory method to make the creation of the registry optional to keep it - // behind a flag. - // TODO(crbug.com/1203992): Remove this when the |kBrowserAppInstanceTracking| - // flag is removed. - static std::unique_ptr Create( - BrowserAppInstanceTracker* ash_instance_tracker); - // Get all instances by app ID. Returns a set of unowned pointers. std::set GetAppInstancesByAppId( const std::string& app_id) const; diff --git a/chrome/browser/apps/app_service/browser_app_instance_tracker.cc b/chrome/browser/apps/app_service/browser_app_instance_tracker.cc index c8436eb1f2f671..83b1d72eac4c94 100644 --- a/chrome/browser/apps/app_service/browser_app_instance_tracker.cc +++ b/chrome/browser/apps/app_service/browser_app_instance_tracker.cc @@ -21,7 +21,6 @@ #include "chrome/browser/ui/browser_window.h" #include "chrome/browser/ui/tabs/tab_strip_model.h" #include "chrome/browser/web_applications/web_app_tab_helper.h" -#include "chrome/common/chrome_features.h" #include "components/services/app_service/public/cpp/types_util.h" #include "content/public/browser/browser_context.h" #include "content/public/browser/navigation_controller.h" @@ -139,16 +138,6 @@ BrowserAppInstanceTracker::~BrowserAppInstanceTracker() { } } -std::unique_ptr BrowserAppInstanceTracker::Create( - Profile* profile, - AppRegistryCache& app_registry_cache) { - if (!features::IsBrowserAppInstanceTrackingEnabled()) { - return nullptr; - } - return std::make_unique(profile, - app_registry_cache); -} - std::set BrowserAppInstanceTracker::GetAppInstancesByAppId( const std::string& app_id) const { diff --git a/chrome/browser/apps/app_service/browser_app_instance_tracker.h b/chrome/browser/apps/app_service/browser_app_instance_tracker.h index ed23b068776fcb..8dbbcdde3c5c80 100644 --- a/chrome/browser/apps/app_service/browser_app_instance_tracker.h +++ b/chrome/browser/apps/app_service/browser_app_instance_tracker.h @@ -55,14 +55,6 @@ class BrowserAppInstanceTracker : public TabStripModelObserver, BrowserAppInstanceTracker& operator=(const BrowserAppInstanceTracker&) = delete; - // A factory method to make the creation of the tracker optional to keep it - // behind a flag. - // TODO(crbug.com/1203992): Remove this when the - // |kBrowserAppInstanceTracking| flag is removed. - static std::unique_ptr Create( - Profile* profile, - AppRegistryCache& app_registry_cache); - // Get all instances by app ID. Returns a set of unowned pointers. std::set GetAppInstancesByAppId( const std::string& app_id) const;