[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. (cherry picked from commit 959bbb3) Bug: 1203992 Bug: 1257872 Change-Id: Idcd656d05a646ec79e2c91acbe0ae3ba23750261 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3212272 Reviewed-by: Dominick Ng <dominickn@chromium.org> Commit-Queue: Alexander Bolodurin <alexbn@chromium.org> Cr-Original-Commit-Position: refs/heads/main@{#929561} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3215844 Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com> Reviewed-by: Maggie Cai <mxcai@chromium.org> Cr-Commit-Position: refs/branch-heads/4664@{#18} Cr-Branched-From: 24dc4ee-refs/heads/main@{#929512}
blueboxd · Oct 11, 2021 · 9add906 · 9add906
1 parent b873e8a
commit 9add906
Show file tree

Hide file tree

Showing 8 changed files with 19 additions and 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
@@ -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<apps::BrowserAppInstanceTracker>(profile_,
+                                                          app_registry_cache_);
+    browser_app_instance_registry_ =
+        std::make_unique<apps::BrowserAppInstanceRegistry>(
+            *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
@@ -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<apps::BrowserAppInstanceTracker>(profile_,
+                                                          app_registry_cache_);
+    browser_app_instance_forwarder_ =
+        std::make_unique<apps::BrowserAppInstanceForwarder>(
+            *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
@@ -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>
-BrowserAppInstanceForwarder::Create(BrowserAppInstanceTracker* tracker) {
-  if (!features::IsBrowserAppInstanceTrackingEnabled()) {
-    return nullptr;
-  }
-  return std::make_unique<BrowserAppInstanceForwarder>(*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
@@ -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 <memory>
-
 #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<BrowserAppInstanceForwarder> 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
@@ -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> BrowserAppInstanceRegistry::Create(
-    BrowserAppInstanceTracker* ash_instance_tracker) {
-  if (!features::IsBrowserAppInstanceTrackingEnabled()) {
-    return nullptr;
-  }
-  return std::make_unique<BrowserAppInstanceRegistry>(*ash_instance_tracker);
-}
-
 std::set<const BrowserAppInstance*>
 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
@@ -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<BrowserAppInstanceRegistry> Create(
-      BrowserAppInstanceTracker* ash_instance_tracker);
-
   // Get all instances by app ID. Returns a set of unowned pointers.
   std::set<const BrowserAppInstance*> 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
@@ -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> BrowserAppInstanceTracker::Create(
-    Profile* profile,
-    AppRegistryCache& app_registry_cache) {
-  if (!features::IsBrowserAppInstanceTrackingEnabled()) {
-    return nullptr;
-  }
-  return std::make_unique<BrowserAppInstanceTracker>(profile,
-                                                     app_registry_cache);
-}
-
 std::set<const BrowserAppInstance*>
 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
@@ -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<BrowserAppInstanceTracker> Create(
-      Profile* profile,
-      AppRegistryCache& app_registry_cache);
-
   // Get all instances by app ID. Returns a set of unowned pointers.
   std::set<const BrowserAppInstance*> GetAppInstancesByAppId(
       const std::string& app_id) const;