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

Issue 7215040: Fix relaunches on the Mac (Closed)

Created:
9 years, 6 months ago by Mark Mentovai
Modified:
9 years, 5 months ago
Reviewers:
Robert Sesek, Nico
CC:
chromium-reviews, brettw-cc_chromium.org
Visibility:
Public.

Description

Fix relaunches on the Mac. Relaunches that go through browser_shutdown and relaunches after install-from-disk-image should both be improved. browser_shutdown relaunches include those that occur when Chrome detects a new version and offers to relaunch itself, and those that occur when you change about:flags and click "Relaunch Now." When relaunching Chrome in the same location via browser_shutdown, the same Dock tile should now always be reused, provided that the tile is persistent, meaning that the user has chosen "Keep in Dock" from the tile's contextual menu. The relaunched process' activation status (foreground or background application) should now track the original process' activation status. BUG=42962, 85073 TEST=1a. Change something in about:flags and hit "Relaunch Now" at the bottom. If you had a persistent Dock tile, the same tile should be used for the relaunched browser every time, without fail. 1b. Same, but once you're running a version of Chrome (including a canary) with this fix, when an update is applied and you relaunch from within the application via the "Update Google Chrome" item in the wrench menu or the "Relaunch" button in the about window, the same tile should be used for the relaunched browser every time, without fail. 2. Relaunch after install from disk image. Remove Chrome from /Applications and double-click the Chrome icon in a read-only disk image carrying a version that has this fix. (Our official or canary disk images will work). Chrome will offer to install itself. Let it. Upon completion of the installation, Chrome should launch out of /Applications properly. If you had a persistent Dock tile pointing to a copy in /Applications, it will be used. Activation status (foreground or background) in the relaunched browser will match the status in the process that had run out of the disk image at the time that the installation completed. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=90371

Patch Set 1 : '' #

Patch Set 2 : '' #

Total comments: 19

Patch Set 3 : '' #

Patch Set 4 : '' #

Total comments: 1

Patch Set 5 : '' #

Total comments: 1

Patch Set 6 : '' #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+529 lines, -69 lines) Patch
M base/mac/mac_util.h View 1 2 3 4 5 1 chunk +5 lines, -0 lines 0 comments Download
M base/mac/mac_util.mm View 1 2 3 4 5 1 chunk +20 lines, -0 lines 0 comments Download
M chrome/app/chrome_main.cc View 1 2 3 4 5 5 chunks +11 lines, -2 lines 0 comments Download
M chrome/browser/app_controller_mac.mm View 1 2 3 4 5 1 chunk +0 lines, -11 lines 0 comments Download
M chrome/browser/browser_shutdown.cc View 1 2 3 4 5 1 chunk +0 lines, -8 lines 0 comments Download
M chrome/browser/first_run/upgrade_util.h View 1 2 3 4 5 2 chunks +14 lines, -4 lines 0 comments Download
A chrome/browser/first_run/upgrade_util_mac.cc View 1 1 chunk +14 lines, -0 lines 0 comments Download
M chrome/browser/mac/install_from_dmg.mm View 1 2 3 4 5 2 chunks +16 lines, -38 lines 0 comments Download
A chrome/browser/mac/relauncher.h View 1 2 3 4 1 chunk +73 lines, -0 lines 0 comments Download
A chrome/browser/mac/relauncher.cc View 1 2 3 4 1 chunk +368 lines, -0 lines 0 comments Download
M chrome/chrome_browser.gypi View 1 2 3 4 5 3 chunks +3 lines, -1 line 0 comments Download
M chrome/common/chrome_switches.h View 1 2 3 4 5 2 chunks +1 line, -1 line 2 comments Download
M chrome/common/chrome_switches.cc View 1 2 3 4 5 2 chunks +4 lines, -4 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
Mark Mentovai
9 years, 6 months ago (2011-06-22 20:40:04 UTC) #1
Robert Sesek
http://codereview.chromium.org/7215040/diff/8001/chrome/browser/mac/install_from_dmg.mm File chrome/browser/mac/install_from_dmg.mm (right): http://codereview.chromium.org/7215040/diff/8001/chrome/browser/mac/install_from_dmg.mm#newcode320 chrome/browser/mac/install_from_dmg.mm:320: FilePath helper_path = browser_path.Append("Contents/Versions"); Is there any way to ...
9 years, 6 months ago (2011-06-23 19:30:16 UTC) #2
Mark Mentovai
Ready. I made all of the changes you suggested except for the PLOG thing because ...
9 years, 6 months ago (2011-06-23 20:23:02 UTC) #3
Robert Sesek
LGTM http://codereview.chromium.org/7215040/diff/8001/chrome/common/chrome_switches.h File chrome/common/chrome_switches.h (left): http://codereview.chromium.org/7215040/diff/8001/chrome/common/chrome_switches.h#oldcode26 chrome/common/chrome_switches.h:26: extern const char kActivateOnLaunch[]; On 2011/06/23 20:23:03, Mark ...
9 years, 6 months ago (2011-06-24 14:30:28 UTC) #4
Mark Mentovai
rsesek@chromium.org wrote: > http://codereview.chromium.org/7215040/diff/17001/chrome/browser/mac/install_from_dmg.mm > File chrome/browser/mac/install_from_dmg.mm (right): > > http://codereview.chromium.org/7215040/diff/17001/chrome/browser/mac/install_from_dmg.mm#newcode320 > chrome/browser/mac/install_from_dmg.mm:320: FilePath helper_path ...
9 years, 6 months ago (2011-06-24 14:46:08 UTC) #5
Nico
Cool CL. http://codereview.chromium.org/7215040/diff/14003/chrome/common/chrome_switches.h File chrome/common/chrome_switches.h (left): http://codereview.chromium.org/7215040/diff/14003/chrome/common/chrome_switches.h#oldcode26 chrome/common/chrome_switches.h:26: extern const char kActivateOnLaunch[]; If I remember ...
9 years, 5 months ago (2011-06-29 17:22:15 UTC) #6
Mark Mentovai
9 years, 5 months ago (2011-06-29 17:52:51 UTC) #7
http://codereview.chromium.org/7215040/diff/14003/chrome/common/chrome_switch...
File chrome/common/chrome_switches.h (left):

http://codereview.chromium.org/7215040/diff/14003/chrome/common/chrome_switch...
chrome/common/chrome_switches.h:26: extern const char kActivateOnLaunch[];
Nico wrote:
> If I remember correctly, this was added by jrg by request of the WebDriver
team.
> Have you checked with them if they still depend on this?

No, but the thought was “anyone left who’s still using this should just run
‘open’ to get LS to handle the open and activation.”

Powered by Google App Engine
This is Rietveld 408576698