Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1946)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chrome/browser/web_applications/web_app_mac.mm

Issue 1038573002: Fixed thread-unsafe use of gfx::Image in app shortcut creation. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Rebase. Created 5 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« chrome/browser/web_applications/web_app_mac.h ('K') | « chrome/browser/web_applications/web_app_mac.h ('k') | chrome/browser/web_applications/web_app_mac_unittest.mm » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/web_applications/web_app_mac.mm
diff --git a/chrome/browser/web_applications/web_app_mac.mm b/chrome/browser/web_applications/web_app_mac.mm
index 5c04775cd1eba2e971ef131c58fe8918d622fecc..24b95e6335db4121d06b730987e6c4edddb26508 100644
--- a/chrome/browser/web_applications/web_app_mac.mm
+++ b/chrome/browser/web_applications/web_app_mac.mm
@@ -219,10 +219,10 @@ bool HasSameUserDataDir(const base::FilePath& bundle_path) {
true /* case_sensitive */);
}
-void LaunchShimOnFileThread(const web_app::ShortcutInfo& shortcut_info,
+void LaunchShimOnFileThread(scoped_ptr<web_app::ShortcutInfo> shortcut_info,
bool launched_after_rebuild) {
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
- base::FilePath shim_path = web_app::GetAppInstallPath(shortcut_info);
+ base::FilePath shim_path = web_app::GetAppInstallPath(*shortcut_info);
if (shim_path.empty() ||
!base::PathExists(shim_path) ||
@@ -230,7 +230,7 @@ void LaunchShimOnFileThread(const web_app::ShortcutInfo& shortcut_info,
// The user may have deleted the copy in the Applications folder, use the
// one in the web app's |app_data_dir_|.
base::FilePath app_data_dir = web_app::GetWebAppDataDirectory(
- shortcut_info.profile_path, shortcut_info.extension_id, GURL());
+ shortcut_info->profile_path, shortcut_info->extension_id, GURL());
shim_path = app_data_dir.Append(shim_path.BaseName());
}
@@ -253,29 +253,44 @@ base::FilePath GetAppLoaderPath() {
base::mac::NSToCFCast(@"app_mode_loader.app"));
}
-void UpdateAndLaunchShimOnFileThread(
+void UpdatePlatformShortcutsInternal(
+ const base::FilePath& app_data_path,
+ const base::string16& old_app_title,
const web_app::ShortcutInfo& shortcut_info,
const extensions::FileHandlersInfo& file_handlers_info) {
+ DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
+ if (AppShimsDisabledForTest() &&
+ !g_app_shims_allow_update_and_launch_in_tests) {
+ return;
+ }
+
+ web_app::WebAppShortcutCreator shortcut_creator(app_data_path, shortcut_info,
+ file_handlers_info);
+ shortcut_creator.UpdateShortcuts();
+}
+
+void UpdateAndLaunchShimOnFileThread(
+ scoped_ptr<web_app::ShortcutInfo> shortcut_info,
+ const extensions::FileHandlersInfo& file_handlers_info) {
base::FilePath shortcut_data_dir = web_app::GetWebAppDataDirectory(
- shortcut_info.profile_path, shortcut_info.extension_id, GURL());
- web_app::internals::UpdatePlatformShortcuts(
- shortcut_data_dir, base::string16(), shortcut_info, file_handlers_info);
- LaunchShimOnFileThread(shortcut_info, true);
+ shortcut_info->profile_path, shortcut_info->extension_id, GURL());
+ UpdatePlatformShortcutsInternal(shortcut_data_dir, base::string16(),
+ *shortcut_info, file_handlers_info);
+ LaunchShimOnFileThread(shortcut_info.Pass(), true);
}
void UpdateAndLaunchShim(
- const web_app::ShortcutInfo& shortcut_info,
+ scoped_ptr<web_app::ShortcutInfo> shortcut_info,
const extensions::FileHandlersInfo& file_handlers_info) {
content::BrowserThread::PostTask(
- content::BrowserThread::FILE,
- FROM_HERE,
- base::Bind(
- &UpdateAndLaunchShimOnFileThread, shortcut_info, file_handlers_info));
+ content::BrowserThread::FILE, FROM_HERE,
+ base::Bind(&UpdateAndLaunchShimOnFileThread, base::Passed(&shortcut_info),
+ file_handlers_info));
}
-void RebuildAppAndLaunch(const web_app::ShortcutInfo& shortcut_info) {
+void RebuildAppAndLaunch(scoped_ptr<web_app::ShortcutInfo> shortcut_info) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
- if (shortcut_info.extension_id == app_mode::kAppListModeId) {
+ if (shortcut_info->extension_id == app_mode::kAppListModeId) {
AppListService* app_list_service =
AppListService::Get(chrome::HOST_DESKTOP_TYPE_NATIVE);
app_list_service->CreateShortcut();
@@ -285,14 +300,14 @@ void RebuildAppAndLaunch(const web_app::ShortcutInfo& shortcut_info) {
ProfileManager* profile_manager = g_browser_process->profile_manager();
Profile* profile =
- profile_manager->GetProfileByPath(shortcut_info.profile_path);
+ profile_manager->GetProfileByPath(shortcut_info->profile_path);
if (!profile || !profile_manager->IsValidProfile(profile))
return;
extensions::ExtensionRegistry* registry =
extensions::ExtensionRegistry::Get(profile);
const extensions::Extension* extension = registry->GetExtensionById(
- shortcut_info.extension_id, extensions::ExtensionRegistry::ENABLED);
+ shortcut_info->extension_id, extensions::ExtensionRegistry::ENABLED);
if (!extension || !extension->is_platform_app())
return;
@@ -468,19 +483,19 @@ std::vector<base::FilePath> GetAllAppBundlesInPath(
return bundle_paths;
}
-web_app::ShortcutInfo BuildShortcutInfoFromBundle(
+scoped_ptr<web_app::ShortcutInfo> BuildShortcutInfoFromBundle(
const base::FilePath& bundle_path) {
NSDictionary* plist = ReadPlist(GetPlistPath(bundle_path));
- web_app::ShortcutInfo shortcut_info;
- shortcut_info.extension_id = base::SysNSStringToUTF8(
+ scoped_ptr<web_app::ShortcutInfo> shortcut_info(new web_app::ShortcutInfo);
+ shortcut_info->extension_id = base::SysNSStringToUTF8(
[plist valueForKey:app_mode::kCrAppModeShortcutIDKey]);
- shortcut_info.is_platform_app = true;
- shortcut_info.url = GURL(base::SysNSStringToUTF8(
+ shortcut_info->is_platform_app = true;
+ shortcut_info->url = GURL(base::SysNSStringToUTF8(
[plist valueForKey:app_mode::kCrAppModeShortcutURLKey]));
- shortcut_info.title = base::SysNSStringToUTF16(
+ shortcut_info->title = base::SysNSStringToUTF16(
[plist valueForKey:app_mode::kCrAppModeShortcutNameKey]);
- shortcut_info.profile_name = base::SysNSStringToUTF8(
+ shortcut_info->profile_name = base::SysNSStringToUTF8(
[plist valueForKey:app_mode::kCrAppModeProfileNameKey]);
// Figure out the profile_path. Since the user_data_dir could contain the
@@ -490,14 +505,14 @@ web_app::ShortcutInfo BuildShortcutInfoFromBundle(
base::FilePath profile_base_name = base::mac::NSStringToFilePath(
[plist valueForKey:app_mode::kCrAppModeProfileDirKey]);
if (user_data_dir.DirName().DirName().BaseName() == profile_base_name)
- shortcut_info.profile_path = user_data_dir.DirName().DirName();
+ shortcut_info->profile_path = user_data_dir.DirName().DirName();
else
- shortcut_info.profile_path = user_data_dir.Append(profile_base_name);
+ shortcut_info->profile_path = user_data_dir.Append(profile_base_name);
return shortcut_info;
}
-web_app::ShortcutInfo RecordAppShimErrorAndBuildShortcutInfo(
+scoped_ptr<web_app::ShortcutInfo> RecordAppShimErrorAndBuildShortcutInfo(
const base::FilePath& bundle_path) {
NSDictionary* plist = ReadPlist(GetPlistPath(bundle_path));
NSString* version_string = [plist valueForKey:app_mode::kCrBundleVersionKey];
@@ -559,10 +574,10 @@ void UpdateFileTypes(NSMutableDictionary* plist,
}
void RevealAppShimInFinderForAppOnFileThread(
- const web_app::ShortcutInfo& shortcut_info,
+ scoped_ptr<web_app::ShortcutInfo> shortcut_info,
const base::FilePath& app_path) {
web_app::WebAppShortcutCreator shortcut_creator(
- app_path, shortcut_info, extensions::FileHandlersInfo());
+ app_path, *shortcut_info, extensions::FileHandlersInfo());
shortcut_creator.RevealAppShimInFinder();
}
@@ -1011,16 +1026,15 @@ base::FilePath GetAppInstallPath(const ShortcutInfo& shortcut_info) {
return shortcut_creator.GetApplicationsShortcutPath();
}
-void MaybeLaunchShortcut(const ShortcutInfo& shortcut_info) {
+void MaybeLaunchShortcut(scoped_ptr<ShortcutInfo> shortcut_info) {
if (AppShimsDisabledForTest() &&
!g_app_shims_allow_update_and_launch_in_tests) {
return;
}
content::BrowserThread::PostTask(
- content::BrowserThread::FILE,
- FROM_HERE,
- base::Bind(&LaunchShimOnFileThread, shortcut_info, false));
+ content::BrowserThread::FILE, FROM_HERE,
+ base::Bind(&LaunchShimOnFileThread, base::Passed(&shortcut_info), false));
}
bool MaybeRebuildShortcut(const base::CommandLine& command_line) {
@@ -1042,7 +1056,7 @@ void CreateAppShortcutInfoLoaded(
Profile* profile,
const extensions::Extension* app,
const base::Callback<void(bool)>& close_callback,
- const ShortcutInfo& shortcut_info) {
+ scoped_ptr<ShortcutInfo> shortcut_info) {
base::scoped_nsobject<NSAlert> alert([[NSAlert alloc] init]);
NSButton* continue_button = [alert
@@ -1074,7 +1088,7 @@ void CreateAppShortcutInfoLoaded(
const int kIconPreviewSizePixels = 128;
const int kIconPreviewTargetSize = 64;
- const gfx::Image* icon = shortcut_info.favicon.GetBest(
+ const gfx::Image* icon = shortcut_info->favicon.GetBest(
kIconPreviewSizePixels, kIconPreviewSizePixels);
if (icon && !icon->IsEmpty()) {
@@ -1123,19 +1137,19 @@ void UpdateShortcutsForAllApps(Profile* profile,
void RevealAppShimInFinderForApp(Profile* profile,
const extensions::Extension* app) {
- const web_app::ShortcutInfo shortcut_info =
+ scoped_ptr<web_app::ShortcutInfo> shortcut_info =
ShortcutInfoForExtensionAndProfile(app, profile);
content::BrowserThread::PostTask(
content::BrowserThread::FILE, FROM_HERE,
- base::Bind(&RevealAppShimInFinderForAppOnFileThread, shortcut_info,
- app->path()));
+ base::Bind(&RevealAppShimInFinderForAppOnFileThread,
+ base::Passed(&shortcut_info), app->path()));
}
namespace internals {
bool CreatePlatformShortcuts(
const base::FilePath& app_data_path,
- const ShortcutInfo& shortcut_info,
+ scoped_ptr<ShortcutInfo> shortcut_info,
const extensions::FileHandlersInfo& file_handlers_info,
const ShortcutLocations& creation_locations,
ShortcutCreationReason creation_reason) {
@@ -1143,33 +1157,26 @@ bool CreatePlatformShortcuts(
if (AppShimsDisabledForTest())
return true;
- WebAppShortcutCreator shortcut_creator(
- app_data_path, shortcut_info, file_handlers_info);
+ WebAppShortcutCreator shortcut_creator(app_data_path, *shortcut_info,
+ file_handlers_info);
return shortcut_creator.CreateShortcuts(creation_reason, creation_locations);
}
void DeletePlatformShortcuts(const base::FilePath& app_data_path,
- const ShortcutInfo& shortcut_info) {
+ scoped_ptr<ShortcutInfo> shortcut_info) {
DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
- WebAppShortcutCreator shortcut_creator(
- app_data_path, shortcut_info, extensions::FileHandlersInfo());
+ WebAppShortcutCreator shortcut_creator(app_data_path, *shortcut_info,
+ extensions::FileHandlersInfo());
shortcut_creator.DeleteShortcuts();
}
void UpdatePlatformShortcuts(
const base::FilePath& app_data_path,
const base::string16& old_app_title,
- const ShortcutInfo& shortcut_info,
+ scoped_ptr<ShortcutInfo> shortcut_info,
const extensions::FileHandlersInfo& file_handlers_info) {
- DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::FILE));
- if (AppShimsDisabledForTest() &&
- !g_app_shims_allow_update_and_launch_in_tests) {
- return;
- }
-
- WebAppShortcutCreator shortcut_creator(
- app_data_path, shortcut_info, file_handlers_info);
- shortcut_creator.UpdateShortcuts();
+ UpdatePlatformShortcutsInternal(app_data_path, old_app_title, *shortcut_info,
+ file_handlers_info);
}
void DeleteAllShortcutsForProfile(const base::FilePath& profile_path) {
@@ -1179,10 +1186,10 @@ void DeleteAllShortcutsForProfile(const base::FilePath& profile_path) {
for (std::vector<base::FilePath>::const_iterator it = bundles.begin();
it != bundles.end(); ++it) {
- web_app::ShortcutInfo shortcut_info =
+ scoped_ptr<web_app::ShortcutInfo> shortcut_info =
BuildShortcutInfoFromBundle(*it);
- WebAppShortcutCreator shortcut_creator(
- it->DirName(), shortcut_info, extensions::FileHandlersInfo());
+ WebAppShortcutCreator shortcut_creator(it->DirName(), *shortcut_info,
+ extensions::FileHandlersInfo());
shortcut_creator.DeleteShortcuts();
}
}
« chrome/browser/web_applications/web_app_mac.h ('K') | « chrome/browser/web_applications/web_app_mac.h ('k') | chrome/browser/web_applications/web_app_mac_unittest.mm » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698