Skip to content

Commit

Permalink
[Mac] Only create the App Shim Socket after taking process singleton …
Browse files Browse the repository at this point in the history
…lock.

Previously, Chrome processes initialized AppShimHostManager before taking
the singleton lock. This meant new processes would replace the App Shim
Socket symlink and then delete it as soon as they fail to become the
singleton process.

BUG=168080
TEST= Start Chrome.
Start Chrome again via the command line. I.e. Chromium.app/Contents/MacOS/Chromium
Start an app via its shim. It should work.

Review URL: https://codereview.chromium.org/268903002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@268469 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
jackhou@chromium.org committed May 6, 2014
1 parent 1a0f7c3 commit bdad6e6
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 7 deletions.
5 changes: 5 additions & 0 deletions apps/app_shim/app_shim_host_manager_mac.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ class AppShimHostManager
// Called on the IO thread to begin listening for connections from app shims.
void ListenOnIOThread();

// The AppShimHostManager is only initialized if the Chrome process
// successfully took the singleton lock. This prevents subsequent processes
// from deleting existing app shim socket files.
bool did_init_;

base::FilePath directory_in_tmp_;

scoped_ptr<IPC::ChannelFactory> factory_;
Expand Down
10 changes: 8 additions & 2 deletions apps/app_shim/app_shim_host_manager_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -50,19 +50,25 @@ void DeleteSocketFiles(const base::FilePath& directory_in_tmp,

} // namespace

AppShimHostManager::AppShimHostManager() {}
AppShimHostManager::AppShimHostManager()
: did_init_(false) {}

void AppShimHostManager::Init() {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
DCHECK(!did_init_);
did_init_ = true;
apps::AppShimHandler::SetDefaultHandler(&extension_app_shim_handler_);
BrowserThread::PostTask(
BrowserThread::FILE, FROM_HERE,
base::Bind(&AppShimHostManager::InitOnFileThread, this));
}

AppShimHostManager::~AppShimHostManager() {
apps::AppShimHandler::SetDefaultHandler(NULL);
factory_.reset();
if (!did_init_)
return;

apps::AppShimHandler::SetDefaultHandler(NULL);
base::FilePath symlink_path;
if (PathService::Get(chrome::DIR_USER_DATA, &symlink_path))
symlink_path = symlink_path.Append(app_mode::kAppShimSocketSymlinkName);
Expand Down
5 changes: 0 additions & 5 deletions chrome/browser/browser_process_platform_part_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
#include "chrome/browser/browser_process_platform_part_mac.h"

#include "chrome/browser/chrome_browser_application_mac.h"
#include "chrome/browser/ui/app_list/app_list_service.h"

BrowserProcessPlatformPart::BrowserProcessPlatformPart() {
}
Expand All @@ -29,10 +28,6 @@
// domain socket will cause the just-created socket to be unlinked.
DCHECK(!app_shim_host_manager_);
app_shim_host_manager_ = new AppShimHostManager;
// Init needs to be called after assigning to a scoped_refptr (i.e. after
// incrementing the refcount).
app_shim_host_manager_->Init();
AppListService::InitAll(NULL);
}

AppShimHostManager* BrowserProcessPlatformPart::app_shim_host_manager() {
Expand Down
1 change: 1 addition & 0 deletions chrome/browser/chrome_browser_main_mac.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class ChromeBrowserMainPartsMac : public ChromeBrowserMainPartsPosix {
// BrowserParts overrides.
virtual void PreEarlyInitialization() OVERRIDE;
virtual void PreMainMessageLoopStart() OVERRIDE;
virtual void PreProfileInit() OVERRIDE;
virtual void PostProfileInit() OVERRIDE;

// Perform platform-specific work that needs to be done after the main event
Expand Down
10 changes: 10 additions & 0 deletions chrome/browser/chrome_browser_main_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#import <Cocoa/Cocoa.h>
#include <sys/sysctl.h>

#include "apps/app_shim/app_shim_host_manager_mac.h"
#include "base/command_line.h"
#include "base/files/file_path.h"
#include "base/mac/bundle_locations.h"
Expand All @@ -20,6 +21,7 @@
#include "chrome/browser/mac/install_from_dmg.h"
#import "chrome/browser/mac/keystone_glue.h"
#include "chrome/browser/metrics/metrics_service.h"
#include "chrome/browser/ui/app_list/app_list_service.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/chrome_switches.h"
#include "components/breakpad/app/breakpad_mac.h"
Expand Down Expand Up @@ -243,6 +245,14 @@ void RecordCatSixtyFour() {
}];
}

void ChromeBrowserMainPartsMac::PreProfileInit() {
ChromeBrowserMainPartsPosix::PreProfileInit();
// This is called here so that the app shim socket is only created after
// taking the singleton lock.
g_browser_process->platform_part()->app_shim_host_manager()->Init();
AppListService::InitAll(NULL);
}

void ChromeBrowserMainPartsMac::PostProfileInit() {
ChromeBrowserMainPartsPosix::PostProfileInit();
g_browser_process->metrics_service()->RecordBreakpadRegistration(
Expand Down

0 comments on commit bdad6e6

Please sign in to comment.