Skip to content

Commit

Permalink
[Lacros] Make Lacros extensions accessible to Ash via App Service.
Browse files Browse the repository at this point in the history
This CL adds the glue code needed to make Lacros extensions available to
Ash. The trickiest part is determining that Lacros needs to use
extension specific filtering code that previously was Ash only.

After this CL, Lacros extensions with fileBrowserHandler will show up
in ChromeOS File.app context menus for files. Icons also work. However,
clicking on these entries currently do nothing; the code to run Lacros
extensions and access files will be added in follow-up CLs.

Details:
* Add BrowserManager::Features::kExtensions (needed for |keep_alive_|).
* Add apps::StandaloneBrowserExtensionAppsFactoryForExtension as
  counterpart to apps::StandaloneBrowserExtensionApps.
* apps::StandaloneBrowserExtensionApps:
  * Rename RegisterChromeAppsCrosapiHost() to RegisterCrosapiHost(), to
    reflect generalized use by its 2 main instances.
  * For extensions: LoggedInStateChanged(): Assign |keep_alive_|.
* Add Crosapi::BindExtensionPublisher() as counterpart to
  Crosapi::BindChromeAppPublisher().
* Make apps_util::CreateIntentFiltersForExtension() available to Lacros.
  * Also make apps_util::CreateExtensionIntentFilters() (for mojom, and
    deprecated) available, for consistency.
  * Transitive inclusions:
    * (anon)::CreateFileURLFilter().
    * (anon)::URLPatternToFileSystemPattern().
* ChromeBrowserMainExtraPartsLacros: Add, instantiate, and initialize
  {|extensions_publisher_|, |extensions_controller_|}.
* LacrosExtensionAppsController:
  * Add MakeForExtensions().
  * For extensions: FinallyLaunch(): Add stub code main flow.
* LacrosExtensionAppsPublisher::ProfileTracker:
  * ShouldShow(): Return false for extensions, so in MakeApp() these
    won't be added to {launcer, shelf, search}.
  * MakeApp():
    * Force |handles_intents| to be true for extensions.
    * Call app_utils::CreateIntentFiltersForExtension() for extensions.
* LacrosExtensionAppsPublisher:
  * Add MakeForExtensions().
  * InitializeCrosapi(): Make crosapi version check and LacrosService
    binding that matches specialization to {Chrome Apps, Extensions}.

Bug: 1275654
Change-Id: I25d9069308289f921edc94780173c9a45d2da6d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3557792
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Nancy Wang <nancylingwang@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/main@{#986972}
  • Loading branch information
samuelhuang authored and Chromium LUCI CQ committed Mar 30, 2022
1 parent 786dcd5 commit 9308871
Show file tree
Hide file tree
Showing 17 changed files with 173 additions and 28 deletions.
25 changes: 14 additions & 11 deletions chrome/browser/apps/app_service/intent_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,17 @@
#include "chromeos/crosapi/mojom/app_service_types.mojom.h"
#endif

#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS)
#include "chrome/common/extensions/api/file_browser_handlers/file_browser_handler.h"
#endif

#if BUILDFLAG(IS_CHROMEOS_ASH)
#include "ash/components/arc/mojom/intent_common.mojom.h"
#include "ash/components/arc/mojom/intent_helper.mojom.h"
#include "ash/constants/ash_features.h"
#include "ash/webui/projector_app/public/cpp/projector_app_constants.h"
#include "chrome/browser/ui/app_list/arc/intent.h"
#include "chrome/browser/web_applications/web_app_id.h"
#include "chrome/common/extensions/api/file_browser_handlers/file_browser_handler.h"
#include "components/arc/intent_helper/arc_intent_helper_bridge.h"
#include "components/arc/intent_helper/intent_constants.h"
#include "components/arc/intent_helper/intent_filter.h"
Expand All @@ -56,7 +59,7 @@ namespace {

constexpr char kTextPlain[] = "text/plain";

#if BUILDFLAG(IS_CHROMEOS_ASH)
#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS)
apps::mojom::IntentFilterPtr CreateFileURLFilter(
const std::vector<std::string>& patterns,
const std::string& activity_name,
Expand Down Expand Up @@ -104,7 +107,7 @@ const std::string URLPatternToFileSystemPattern(const URLPattern& pattern,
base::ReplaceChars(path, "*", ".*", &path);
return path;
}
#endif
#endif // BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS)

apps::mojom::IntentFilterPtr CreateMimeTypeShareFilter(
const std::vector<std::string>& mime_types) {
Expand Down Expand Up @@ -534,9 +537,7 @@ std::vector<apps::mojom::IntentFilterPtr> CreateChromeAppIntentFilters(

apps::IntentFilters CreateIntentFiltersForExtension(
const extensions::Extension* extension) {
#if !BUILDFLAG(IS_CHROMEOS_ASH)
return {};
#else
#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS)
FileBrowserHandler::List* handler_list =
FileBrowserHandler::GetHandlers(extension);
if (!handler_list) {
Expand All @@ -554,14 +555,14 @@ apps::IntentFilters CreateIntentFiltersForExtension(
CreateFileURLFilter(patterns, handler->id(), handler->title())));
}
return filters;
#endif
#else
return {};
#endif // BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS)
}

std::vector<apps::mojom::IntentFilterPtr> CreateExtensionIntentFilters(
const extensions::Extension* extension) {
#if !BUILDFLAG(IS_CHROMEOS_ASH)
return {};
#else
#if BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS)
FileBrowserHandler::List* handler_list =
FileBrowserHandler::GetHandlers(extension);
if (!handler_list)
Expand All @@ -578,7 +579,9 @@ std::vector<apps::mojom::IntentFilterPtr> CreateExtensionIntentFilters(
CreateFileURLFilter(patterns, handler->id(), handler->title()));
}
return filters;
#endif
#else
return {};
#endif // BUILDFLAG(IS_CHROMEOS_ASH) || BUILDFLAG(IS_CHROMEOS_LACROS)
}

#if BUILDFLAG(IS_CHROMEOS_ASH)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,13 @@ StandaloneBrowserExtensionApps::StandaloneBrowserExtensionApps(

StandaloneBrowserExtensionApps::~StandaloneBrowserExtensionApps() = default;

void StandaloneBrowserExtensionApps::RegisterChromeAppsCrosapiHost(
void StandaloneBrowserExtensionApps::RegisterCrosapiHost(
mojo::PendingReceiver<crosapi::mojom::AppPublisher> receiver) {
RegisterPublisher(app_type_);
apps::AppPublisher::Publish(std::vector<AppPtr>{}, app_type_,
/*should_notify_initialized=*/true);

// At the moment the app service publisher will only accept one client
// At the moment the app service publisher will only accept one browser client
// publishing apps to ash chrome. Any extra clients will be ignored.
// TODO(crbug.com/1174246): Support SxS lacros.
if (receiver_.is_bound()) {
Expand Down Expand Up @@ -313,6 +313,9 @@ void StandaloneBrowserExtensionApps::LoggedInStateChanged() {
if (app_type_ == AppType::kStandaloneBrowserChromeApp) {
keep_alive_ = crosapi::BrowserManager::Get()->KeepAlive(
crosapi::BrowserManager::Feature::kChromeApps);
} else if (app_type_ == AppType::kStandaloneBrowserExtension) {
keep_alive_ = crosapi::BrowserManager::Get()->KeepAlive(
crosapi::BrowserManager::Feature::kExtensions);
}
}
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,9 @@ class StandaloneBrowserExtensionApps : public KeyedService,
StandaloneBrowserExtensionApps& operator=(
const StandaloneBrowserExtensionApps&) = delete;

// Register the chrome apps host from lacros-chrome to allow lacros-chrome
// to publish chrome apps to the app service in ash-chrome.
void RegisterChromeAppsCrosapiHost(
// Register the host (for Chrome Apps or Extensions) from Lacros to allow the
// matching publisher to publish to the App Service in Ash.
void RegisterCrosapiHost(
mojo::PendingReceiver<crosapi::mojom::AppPublisher> receiver);

private:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

namespace apps {

/******** StandaloneBrowserExtensionAppsFactoryForApp ********/

// static
StandaloneBrowserExtensionApps*
StandaloneBrowserExtensionAppsFactoryForApp::GetForProfile(Profile* profile) {
Expand Down Expand Up @@ -51,4 +53,47 @@ StandaloneBrowserExtensionAppsFactoryForApp::BuildServiceInstanceFor(
AppType::kStandaloneBrowserChromeApp);
}

/******** StandaloneBrowserExtensionAppsFactoryForExtension ********/

// static
StandaloneBrowserExtensionApps*
StandaloneBrowserExtensionAppsFactoryForExtension::GetForProfile(
Profile* profile) {
return static_cast<StandaloneBrowserExtensionApps*>(
StandaloneBrowserExtensionAppsFactoryForExtension::GetInstance()
->GetServiceForBrowserContext(profile, true /* create */));
}

// static
StandaloneBrowserExtensionAppsFactoryForExtension*
StandaloneBrowserExtensionAppsFactoryForExtension::GetInstance() {
return base::Singleton<
StandaloneBrowserExtensionAppsFactoryForExtension>::get();
}

// static
void StandaloneBrowserExtensionAppsFactoryForExtension::ShutDownForTesting(
content::BrowserContext* context) {
auto* factory = GetInstance();
factory->BrowserContextShutdown(context);
factory->BrowserContextDestroyed(context);
}

StandaloneBrowserExtensionAppsFactoryForExtension::
StandaloneBrowserExtensionAppsFactoryForExtension()
: BrowserContextKeyedServiceFactory(
"StandaloneBrowserExtensionAppsForExtension",
BrowserContextDependencyManager::GetInstance()) {
DependsOn(apps::AppServiceProxyFactory::GetInstance());
}

KeyedService*
StandaloneBrowserExtensionAppsFactoryForExtension::BuildServiceInstanceFor(
content::BrowserContext* context) const {
return new StandaloneBrowserExtensionApps(
AppServiceProxyFactory::GetForProfile(
Profile::FromBrowserContext(context)),
AppType::kStandaloneBrowserExtension);
}

} // namespace apps
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,33 @@ class StandaloneBrowserExtensionAppsFactoryForApp
content::BrowserContext* context) const override;
};

// Singleton that owns all StandaloneBrowserExtensionApps publisher for
// Extensions and associates them with Profiles.
class StandaloneBrowserExtensionAppsFactoryForExtension
: public BrowserContextKeyedServiceFactory {
public:
static StandaloneBrowserExtensionApps* GetForProfile(Profile* profile);

static StandaloneBrowserExtensionAppsFactoryForExtension* GetInstance();

static void ShutDownForTesting(content::BrowserContext* context);

private:
friend struct base::DefaultSingletonTraits<
StandaloneBrowserExtensionAppsFactoryForExtension>;

StandaloneBrowserExtensionAppsFactoryForExtension();
StandaloneBrowserExtensionAppsFactoryForExtension(
const StandaloneBrowserExtensionAppsFactoryForExtension&) = delete;
StandaloneBrowserExtensionAppsFactoryForExtension& operator=(
const StandaloneBrowserExtensionAppsFactoryForExtension&) = delete;
~StandaloneBrowserExtensionAppsFactoryForExtension() override = default;

// BrowserContextKeyedServiceFactory overrides.
KeyedService* BuildServiceInstanceFor(
content::BrowserContext* context) const override;
};

} // namespace apps

#endif // CHROME_BROWSER_APPS_APP_SERVICE_PUBLISHERS_STANDALONE_BROWSER_EXTENSION_APPS_FACTORY_H_
1 change: 1 addition & 0 deletions chrome/browser/ash/crosapi/browser_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,7 @@ class BrowserManager : public session_manager::SessionManagerObserver,
kTestOnly,
kAppService,
kChromeApps,
kExtensions,
};

// Any instance of this class will ensure that the Lacros browser will stay
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ash/crosapi/browser_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ Channel GetStatefulLacrosChannel() {
}

static_assert(
crosapi::mojom::Crosapi::Version_ == 69,
crosapi::mojom::Crosapi::Version_ == 70,
"if you add a new crosapi, please add it to kInterfaceVersionEntries");

} // namespace
Expand Down
11 changes: 10 additions & 1 deletion chrome/browser/ash/crosapi/crosapi_ash.cc
Original file line number Diff line number Diff line change
Expand Up @@ -288,14 +288,23 @@ void CrosapiAsh::BindChromeAppPublisher(
Profile* profile = ProfileManager::GetPrimaryUserProfile();
apps::StandaloneBrowserExtensionApps* chrome_apps =
apps::StandaloneBrowserExtensionAppsFactoryForApp::GetForProfile(profile);
chrome_apps->RegisterChromeAppsCrosapiHost(std::move(receiver));
chrome_apps->RegisterCrosapiHost(std::move(receiver));
}

void CrosapiAsh::BindChromeAppWindowTracker(
mojo::PendingReceiver<mojom::AppWindowTracker> receiver) {
chrome_app_window_tracker_ash_->BindReceiver(std::move(receiver));
}

void CrosapiAsh::BindExtensionPublisher(
mojo::PendingReceiver<mojom::AppPublisher> receiver) {
Profile* profile = ProfileManager::GetPrimaryUserProfile();
apps::StandaloneBrowserExtensionApps* extensions =
apps::StandaloneBrowserExtensionAppsFactoryForExtension::GetForProfile(
profile);
extensions->RegisterCrosapiHost(std::move(receiver));
}

void CrosapiAsh::BindFieldTrialService(
mojo::PendingReceiver<crosapi::mojom::FieldTrialService> receiver) {
field_trial_service_ash_->BindReceiver(std::move(receiver));
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/ash/crosapi/crosapi_ash.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,8 @@ class CrosapiAsh : public mojom::Crosapi {
mojo::PendingReceiver<mojom::DownloadController> receiver) override;
void BindDriveIntegrationService(
mojo::PendingReceiver<mojom::DriveIntegrationService> receiver) override;
void BindExtensionPublisher(
mojo::PendingReceiver<mojom::AppPublisher> receiver) override;
void BindFieldTrialService(
mojo::PendingReceiver<mojom::FieldTrialService> receiver) override;
void BindFileManager(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,11 @@ void ChromeBrowserMainExtraPartsLacros::PostBrowserStart() {
chrome_apps_controller_ =
LacrosExtensionAppsController::MakeForChromeApps();
chrome_apps_controller_->Initialize(chrome_apps_publisher_->publisher());

extensions_publisher_ = LacrosExtensionAppsPublisher::MakeForExtensions();
extensions_publisher_->Initialize();
extensions_controller_ = LacrosExtensionAppsController::MakeForExtensions();
extensions_controller_->Initialize(extensions_publisher_->publisher());
}

if (chromeos::LacrosService::Get()->init_params()->web_apps_enabled) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,12 @@ class ChromeBrowserMainExtraPartsLacros : public ChromeBrowserMainExtraParts {
// Sends Chrome app (AKA extension app) events to ash.
std::unique_ptr<LacrosExtensionAppsPublisher> chrome_apps_publisher_;

// Receives extension events from ash.
std::unique_ptr<LacrosExtensionAppsController> extensions_controller_;

// Sends extension events to ash.
std::unique_ptr<LacrosExtensionAppsPublisher> extensions_publisher_;

// A test controller that is registered with the ash-chrome's test controller
// service over crosapi to let tests running in ash-chrome control this Lacros
// instance. It is only instantiated in Linux builds AND only when Ash's test
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/lacros/for_which_extension_type.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class Extension;
} // namespace extensions

// A class to embody instance-specific specialization for one of {Chrome Apps,
// Extension}".
// Extension}.
class ForWhichExtensionType {
public:
ForWhichExtensionType(const ForWhichExtensionType&);
Expand Down
16 changes: 16 additions & 0 deletions chrome/browser/lacros/lacros_extension_apps_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ LacrosExtensionAppsController::MakeForChromeApps() {
return std::make_unique<LacrosExtensionAppsController>(InitForChromeApps());
}

// static
std::unique_ptr<LacrosExtensionAppsController>
LacrosExtensionAppsController::MakeForExtensions() {
return std::make_unique<LacrosExtensionAppsController>(InitForExtensions());
}

LacrosExtensionAppsController::LacrosExtensionAppsController(
const ForWhichExtensionType& which_type)
: which_type_(which_type), controller_{this} {}
Expand Down Expand Up @@ -267,5 +273,15 @@ void LacrosExtensionAppsController::FinallyLaunch(
// which will be used to close the instance at a later point in time.
result->instance_id = base::UnguessableToken::Create();
std::move(callback).Run(std::move(result));

} else if (which_type_.IsExtensions()) {
// TODO(crbug.com/1275654): Add flow to run extensions. This will involve
// an asynchronous step,
std::move(callback).Run(std::move(result));

} else {
// Unknown case: Just reply with vacuous results (instead of failing) since
// the call is over crosapi.
std::move(callback).Run(std::move(result));
}
}
1 change: 1 addition & 0 deletions chrome/browser/lacros/lacros_extension_apps_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class ExtensionAppsEnableFlow;
class LacrosExtensionAppsController : public crosapi::mojom::AppController {
public:
static std::unique_ptr<LacrosExtensionAppsController> MakeForChromeApps();
static std::unique_ptr<LacrosExtensionAppsController> MakeForExtensions();

// Should not be directly called. Normally this should be private, but then
// this would require friending std::make_unique.
Expand Down

0 comments on commit 9308871

Please sign in to comment.