Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Commit

Permalink
[Mac] Increment app shortcut version numbers.
Browse files Browse the repository at this point in the history
https://crrev.com/1413863003 statically links libc++.a on Mac. This
causes app shims created before the static linking to fail to launch on
Chromium built after the static linking. The launch failures are caused
by substantial memory corruptions in the app_mode::ChromeAppModeInfo
struct when it is passed from the shim launcher to the Chromium dylib at
startup, as the shim is linking against the system C++ library while
Chromium is now linked to its own libc++.

This CL increments the shortcut version numbers, which will cause
Chromium to rebuild shims when it is launched. However, shims which are
launched without Chromium running will continue to fail until Chromium
is launched, at which point the shims will be rebuilt. To counter this,
the ChromeAppModeStart entry point for shims is now versioned; by
incrementing the function version number, all old shims will
automatically be rebuilt on their next launch.

BUG=561205

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

Cr-Commit-Position: refs/heads/master@{#362634}
(cherry picked from commit d7f8dad)

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

Cr-Commit-Position: refs/branch-heads/2564@{#227}
Cr-Branched-From: 1283eca-refs/heads/master@{#359700}
  • Loading branch information
Dominick Ng committed Dec 4, 2015
1 parent 9bef822 commit 68f6a69
Show file tree
Hide file tree
Showing 5 changed files with 19 additions and 9 deletions.
2 changes: 1 addition & 1 deletion chrome/app/framework.order
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ __ZdlPvS_
___asan_default_options

# Entry point from the app mode loader.
_ChromeAppModeStart
_ChromeAppModeStart_v4

# _ChromeMain must be listed last. That's the whole point of this file.
_ChromeMain
8 changes: 7 additions & 1 deletion chrome/app_shim/app_mode_loader_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@

typedef int (*StartFun)(const app_mode::ChromeAppModeInfo*);

// The name of the entry point in the Framework. This name is dynamically
// queried at shim launch to allow the shim to connect and run.
// The function is versioned in case we need to obsolete and rebuild the shim
// before it loads, e.g. see https://crbug.com/561205.
const char kStartFunName[] = "ChromeAppModeStart_v4";

int LoadFrameworkAndStart(app_mode::ChromeAppModeInfo* info) {
using base::SysNSStringToUTF8;
using base::SysNSStringToUTF16;
Expand Down Expand Up @@ -138,7 +144,7 @@ int LoadFrameworkAndStart(app_mode::ChromeAppModeInfo* info) {
void* cr_dylib = dlopen(framework_shlib_path.value().c_str(), RTLD_LAZY);
if (cr_dylib) {
// Find the entry point.
ChromeAppModeStart = (StartFun)dlsym(cr_dylib, "ChromeAppModeStart");
ChromeAppModeStart = (StartFun)dlsym(cr_dylib, kStartFunName);
if (!ChromeAppModeStart)
LOG(ERROR) << "Couldn't get entry point: " << dlerror();
} else {
Expand Down
12 changes: 9 additions & 3 deletions chrome/app_shim/chrome_main_app_mode_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -565,13 +565,19 @@ - (void)closeWithSuccess:(bool)success {
extern "C" {

// |ChromeAppModeStart()| is the point of entry into the framework from the app
// mode loader.
// mode loader. There are cases where the Chromium framework may have changed in
// a way that is incompatible with an older shim (e.g. change to libc++ library
// linking). The function name is versioned to provide a way to force shim
// upgrades if they are launched before an updated version of Chromium can
// upgrade them; the old shim will not be able to dyload the new
// ChromeAppModeStart, so it will fall back to the upgrade path. See
// https://crbug.com/561205.
__attribute__((visibility("default")))
int ChromeAppModeStart(const app_mode::ChromeAppModeInfo* info);
int ChromeAppModeStart_v4(const app_mode::ChromeAppModeInfo* info);

} // extern "C"

int ChromeAppModeStart(const app_mode::ChromeAppModeInfo* info) {
int ChromeAppModeStart_v4(const app_mode::ChromeAppModeInfo* info) {
base::CommandLine::Init(info->argc, info->argv);

base::mac::ScopedNSAutoreleasePool scoped_pool;
Expand Down
4 changes: 1 addition & 3 deletions chrome/browser/apps/shortcut_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,7 @@ namespace {
// need to be recreated. This might happen when we change various aspects of app
// shortcuts like command-line flags or associated icons, binaries, etc.
#if defined(OS_MACOSX)
// This was changed to 3 at r316520, but reverted again. Next time we need to
// trigger a recreate, increment this to 4.
const int kCurrentAppShortcutsVersion = 2;
const int kCurrentAppShortcutsVersion = 4;
#else
const int kCurrentAppShortcutsVersion = 0;
#endif
Expand Down
2 changes: 1 addition & 1 deletion chrome/browser/ui/app_list/app_list_service_mac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ - (void)cleanupOnUIThread;
namespace {

// Version of the app list shortcut version installed.
const int kShortcutVersion = 2;
const int kShortcutVersion = 4;

// Duration of show and hide animations.
const NSTimeInterval kAnimationDuration = 0.2;
Expand Down

0 comments on commit 68f6a69

Please sign in to comment.