Skip to content

Commit

Permalink
[lacros shelf] check WebAppsCrosapi in Lacros the right way
Browse files Browse the repository at this point in the history
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 <dominickn@chromium.org>
Commit-Queue: Alexander Bolodurin <alexbn@chromium.org>
Cr-Commit-Position: refs/heads/main@{#929561}
  • Loading branch information
Alexander Bolodurin authored and Chromium LUCI CQ committed Oct 8, 2021
1 parent 362234f commit 959bbb3
Show file tree
Hide file tree
Showing 8 changed files with 19 additions and 63 deletions.
14 changes: 9 additions & 5 deletions chrome/browser/apps/app_service/app_service_proxy_chromeos.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
15 changes: 10 additions & 5 deletions chrome/browser/apps/app_service/app_service_proxy_lacros.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
11 changes: 0 additions & 11 deletions chrome/browser/apps/app_service/browser_app_instance_tracker.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 959bbb3

Please sign in to comment.