Commit c2d4a844 authored by Christopher Cameron's avatar Christopher Cameron Committed by Commit Bot

MacPWAs: Use RemoteCocoa for some Google apps

Change YouTube, GMail, and Google Drive (non-bookmark) apps to use
RemoteCocoa. Do this just by inspecting their app id.

These apps exist in a strange corner where they are not bookmark apps,
but largely behave like bookmark apps. It's an open question if they
should be multi-profile or not. In this patch I err on the side of
caution and treat them as old-style non-multi-profile apps, because much
of the multi-profile handling code is now PWA-only (post-BMO).

In working on this, I found a bug wherein we assume when generating app
shims, that any app that has a start URL is a bookmark or PWA app. This
is not a valid assumption. Add a new field to web_app::ShortcutInfo to
track if an app should be multi-profile, and populate it accordingly.

There will be one bug here that I won't be able to fix without more
effort and risk than it's worth. If someone created a multi-profile app
shim for any non-PWA app that happened to have a valid URL, that
shortcut will no longer "work" in that it will open the app in a new
Chrome window instead of an app. Launching through chrome://apps will
repair the situation. This is documented in crbug.com/1091318.

R=dmurph
TBR=rdevlin.cronin

Bug: 1086824
Change-Id: Ibd829b07ed806c3e3c45b67075c5e25e368b9341
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2229663
Commit-Queue: ccameron <ccameron@chromium.org>
Reviewed-by: default avatarDaniel Murphy <dmurph@chromium.org>
Cr-Commit-Position: refs/heads/master@{#775221}
parent d2144c46
...@@ -156,7 +156,15 @@ bool ExtensionAppShimManagerDelegate::AppUsesRemoteCocoa( ...@@ -156,7 +156,15 @@ bool ExtensionAppShimManagerDelegate::AppUsesRemoteCocoa(
const Extension* extension = MaybeGetAppExtension(profile, app_id); const Extension* extension = MaybeGetAppExtension(profile, app_id);
if (!profile || !extension) if (!profile || !extension)
return false; return false;
return extension->is_hosted_app() && extension->from_bookmark(); if (!extension->is_hosted_app())
return false;
// The Gmail, Google Drive, and YouTube apps behave like bookmark apps.
// https://crbug.com/1086824
return extension->from_bookmark() ||
extension->id() == extension_misc::kYoutubeAppId ||
extension->id() == extension_misc::kGoogleDriveAppId ||
extension->id() == extension_misc::kGMailAppId;
} }
void ExtensionAppShimManagerDelegate::EnableExtension( void ExtensionAppShimManagerDelegate::EnableExtension(
......
...@@ -48,6 +48,11 @@ struct ShortcutInfo { ...@@ -48,6 +48,11 @@ struct ShortcutInfo {
std::set<std::string> file_handler_extensions; std::set<std::string> file_handler_extensions;
std::set<std::string> file_handler_mime_types; std::set<std::string> file_handler_mime_types;
// An app is multi-profile if there is a single shortcut and single app shim
// for all profiles. The app itself has a profile switcher that may be used
// to open windows for the various profiles. This is relevant only on macOS.
bool is_multi_profile = false;
private: private:
// Since gfx::ImageFamily |favicon| has a non-thread-safe reference count in // Since gfx::ImageFamily |favicon| has a non-thread-safe reference count in
// its member and is bound to current thread, always destroy ShortcutInfo // its member and is bound to current thread, always destroy ShortcutInfo
......
...@@ -1252,8 +1252,7 @@ std::vector<base::FilePath> WebAppShortcutCreator::GetAppBundlesById() const { ...@@ -1252,8 +1252,7 @@ std::vector<base::FilePath> WebAppShortcutCreator::GetAppBundlesById() const {
} }
bool WebAppShortcutCreator::IsMultiProfile() const { bool WebAppShortcutCreator::IsMultiProfile() const {
// Only PWAs and bookmark apps are multi-profile capable. return info_->is_multi_profile;
return info_->url.is_valid();
} }
void WebAppShortcutCreator::RevealAppShimInFinder( void WebAppShortcutCreator::RevealAppShimInFinder(
......
...@@ -76,6 +76,7 @@ std::unique_ptr<ShortcutInfo> GetShortcutInfo() { ...@@ -76,6 +76,7 @@ std::unique_ptr<ShortcutInfo> GetShortcutInfo() {
info->profile_path = base::FilePath("user_data_dir").Append("Profile 1"); info->profile_path = base::FilePath("user_data_dir").Append("Profile 1");
info->profile_name = "profile name"; info->profile_name = "profile name";
info->version_for_display = "stable 1.0"; info->version_for_display = "stable 1.0";
info->is_multi_profile = true;
return info; return info;
} }
...@@ -430,7 +431,7 @@ TEST_F(WebAppShortcutCreatorTest, UpdateBookmarkAppShortcut) { ...@@ -430,7 +431,7 @@ TEST_F(WebAppShortcutCreatorTest, UpdateBookmarkAppShortcut) {
} }
TEST_F(WebAppShortcutCreatorTest, DeleteShortcutsSingleProfile) { TEST_F(WebAppShortcutCreatorTest, DeleteShortcutsSingleProfile) {
info_->url = GURL(); info_->is_multi_profile = false;
base::FilePath other_shim_path = base::FilePath other_shim_path =
shim_path_.DirName().Append("Copy of Shim.app"); shim_path_.DirName().Append("Copy of Shim.app");
...@@ -486,7 +487,7 @@ TEST_F(WebAppShortcutCreatorTest, DeleteShortcuts) { ...@@ -486,7 +487,7 @@ TEST_F(WebAppShortcutCreatorTest, DeleteShortcuts) {
} }
TEST_F(WebAppShortcutCreatorTest, DeleteAllShortcutsForProfile) { TEST_F(WebAppShortcutCreatorTest, DeleteAllShortcutsForProfile) {
info_->url = GURL(); info_->is_multi_profile = false;
NiceMock<WebAppShortcutCreatorMock> shortcut_creator(app_data_dir_, NiceMock<WebAppShortcutCreatorMock> shortcut_creator(app_data_dir_,
info_.get()); info_.get());
......
...@@ -188,6 +188,7 @@ std::unique_ptr<ShortcutInfo> ShortcutInfoForExtensionAndProfile( ...@@ -188,6 +188,7 @@ std::unique_ptr<ShortcutInfo> ShortcutInfoForExtensionAndProfile(
// File Handlers should only be included in bookmark apps. // File Handlers should only be included in bookmark apps.
if (app->from_bookmark()) { if (app->from_bookmark()) {
shortcut_info->is_multi_profile = true;
FileHandlerManager& file_handler_manager = FileHandlerManager& file_handler_manager =
WebAppProviderBase::GetProviderBase(profile)->file_handler_manager(); WebAppProviderBase::GetProviderBase(profile)->file_handler_manager();
if (const auto* file_handlers = if (const auto* file_handlers =
......
...@@ -120,6 +120,7 @@ std::unique_ptr<ShortcutInfo> WebAppShortcutManager::BuildShortcutInfoForWebApp( ...@@ -120,6 +120,7 @@ std::unique_ptr<ShortcutInfo> WebAppShortcutManager::BuildShortcutInfoForWebApp(
shortcut_info->profile_path = profile()->GetPath(); shortcut_info->profile_path = profile()->GetPath();
shortcut_info->profile_name = shortcut_info->profile_name =
profile()->GetPrefs()->GetString(prefs::kProfileName); profile()->GetPrefs()->GetString(prefs::kProfileName);
shortcut_info->is_multi_profile = true;
if (const apps::FileHandlers* file_handlers = if (const apps::FileHandlers* file_handlers =
file_handler_manager_->GetEnabledFileHandlers(app->app_id())) { file_handler_manager_->GetEnabledFileHandlers(app->app_id())) {
......
...@@ -124,7 +124,9 @@ const char kLacrosAppId[] = "jaimifaeiicidiikhmjedcgdimealfbh"; ...@@ -124,7 +124,9 @@ const char kLacrosAppId[] = "jaimifaeiicidiikhmjedcgdimealfbh";
const char kFilesManagerAppId[] = "hhaomjibdihmijegdhdafkllkbggdgoj"; const char kFilesManagerAppId[] = "hhaomjibdihmijegdhdafkllkbggdgoj";
const char kCalculatorAppId[] = "joodangkbfjnajiiifokapkpmhfnpleo"; const char kCalculatorAppId[] = "joodangkbfjnajiiifokapkpmhfnpleo";
const char kCalendarDemoAppId[] = "fpgfohogebplgnamlafljlcidjedbdeb"; const char kCalendarDemoAppId[] = "fpgfohogebplgnamlafljlcidjedbdeb";
const char kGMailAppId[] = "pjkljhegncpnkpknbcohdijeoejaedia";
const char kGoogleDocsDemoAppId[] = "chdaoodbokekbiiphekbfjdmiodccljl"; const char kGoogleDocsDemoAppId[] = "chdaoodbokekbiiphekbfjdmiodccljl";
const char kGoogleDriveAppId[] = "apdfllckaahabafndbhieahigkjlhalf";
const char kGoogleSheetsDemoAppId[] = "nifkmgcdokhkjghdlgflonppnefddien"; const char kGoogleSheetsDemoAppId[] = "nifkmgcdokhkjghdlgflonppnefddien";
const char kGoogleSlidesDemoAppId[] = "hdmobeajeoanbanmdlabnbnlopepchip"; const char kGoogleSlidesDemoAppId[] = "hdmobeajeoanbanmdlabnbnlopepchip";
const char kGoogleKeepAppId[] = "hmjkmjkepdijhoojdojkdfohbdgmmhki"; const char kGoogleKeepAppId[] = "hmjkmjkepdijhoojdojkdfohbdgmmhki";
......
...@@ -216,9 +216,15 @@ extern const char kCalculatorAppId[]; ...@@ -216,9 +216,15 @@ extern const char kCalculatorAppId[];
// The extension id of the demo Calendar application. // The extension id of the demo Calendar application.
extern const char kCalendarDemoAppId[]; extern const char kCalendarDemoAppId[];
// The extension id of the GMail application.
extern const char kGMailAppId[];
// The extension id of the demo Google Docs application. // The extension id of the demo Google Docs application.
extern const char kGoogleDocsDemoAppId[]; extern const char kGoogleDocsDemoAppId[];
// The extension id of the Google Drive application.
extern const char kGoogleDriveAppId[];
// The extension id of the demo Google Sheets application. // The extension id of the demo Google Sheets application.
extern const char kGoogleSheetsDemoAppId[]; extern const char kGoogleSheetsDemoAppId[];
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment