Turn on session restore by default for mac on Lion

Turn on session restore by default for mac on Lion

When the user has not explicitly set a preference in Chrome for session restore, the default behavior under Lion becomes sensitive to the System preference for window restore.

BUG=75814
TEST=SessionStartupPrefTest.*, and manual testing of WebUI: clean prefs, Launch Chrome, toggle "System Preferences > General > Restore windows..." and observe effect on Chrome preferences "Basics > On Startup".

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=94605

[dhollowa, 2011-07-28 17:55:08]

csilv: WebUI changes
rsesek: All else

csliv: A note on the WebUI changes.  They are motivated by the fact that the
standard preference mechanism assumes that the standard preferences controller
is sufficient.  In this case it is not.  WebUI needs to treat
|SessionStartupPref| as the controller, which I've done here.

Thanks!

[Robert Sesek, 2011-07-28 18:30:21]

http://codereview.chromium.org/7522025/diff/1/chrome/browser/prefs/session_st...
File chrome/browser/prefs/session_startup_pref.cc (right):

http://codereview.chromium.org/7522025/diff/1/chrome/browser/prefs/session_st...
chrome/browser/prefs/session_startup_pref.cc:16: #ifdef OS_MACOSX
Break this out to be in its own #include block

http://codereview.chromium.org/7522025/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/browser_options.js (right):

http://codereview.chromium.org/7522025/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/browser_options.js:67: for (i=0; i <
items.length; i++) {
nit: spaces around operators

http://codereview.chromium.org/7522025/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/browser_options.js:248: items[i].checked = true
nit: missing semicolon

http://codereview.chromium.org/7522025/diff/1/chrome/browser/ui/cocoa/window_...
File chrome/browser/ui/cocoa/window_restore_utils.mm (right):

http://codereview.chromium.org/7522025/diff/1/chrome/browser/ui/cocoa/window_...
chrome/browser/ui/cocoa/window_restore_utils.mm:15: NSUserDefaults* defaults =
[NSUserDefaults standardUserDefaults];
Need to #import Foundation

[dhollowa, 2011-07-28 20:05:35]

http://codereview.chromium.org/7522025/diff/1/chrome/browser/prefs/session_st...
File chrome/browser/prefs/session_startup_pref.cc (right):

http://codereview.chromium.org/7522025/diff/1/chrome/browser/prefs/session_st...
chrome/browser/prefs/session_startup_pref.cc:16: #ifdef OS_MACOSX
On 2011/07/28 18:30:22, rsesek wrote:
> Break this out to be in its own #include block

Done.

http://codereview.chromium.org/7522025/diff/1/chrome/browser/resources/option...
File chrome/browser/resources/options/browser_options.js (right):

http://codereview.chromium.org/7522025/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/browser_options.js:67: for (i=0; i <
items.length; i++) {
On 2011/07/28 18:30:22, rsesek wrote:
> nit: spaces around operators

Done.

http://codereview.chromium.org/7522025/diff/1/chrome/browser/resources/option...
chrome/browser/resources/options/browser_options.js:248: items[i].checked = true
On 2011/07/28 18:30:22, rsesek wrote:
> nit: missing semicolon

Done.

http://codereview.chromium.org/7522025/diff/1/chrome/browser/ui/cocoa/window_...
File chrome/browser/ui/cocoa/window_restore_utils.mm (right):

http://codereview.chromium.org/7522025/diff/1/chrome/browser/ui/cocoa/window_...
chrome/browser/ui/cocoa/window_restore_utils.mm:15: NSUserDefaults* defaults =
[NSUserDefaults standardUserDefaults];
On 2011/07/28 18:30:22, rsesek wrote:
> Need to #import Foundation

Done.

[Robert Sesek, 2011-07-28 20:08:38]

Everything but WebUI LGTM.

http://codereview.chromium.org/7522025/diff/4001/chrome/browser/prefs/session...
File chrome/browser/prefs/session_startup_pref.cc (right):

http://codereview.chromium.org/7522025/diff/4001/chrome/browser/prefs/session...
chrome/browser/prefs/session_startup_pref.cc:114: pref.type =
SessionStartupPref::DEFAULT;
WHy do you need to do this? Won't pref.type still be default?

http://codereview.chromium.org/7522025/diff/4001/chrome/browser/resources/opt...
File chrome/browser/resources/options/browser_options.js (right):

http://codereview.chromium.org/7522025/diff/4001/chrome/browser/resources/opt...
chrome/browser/resources/options/browser_options.js:246: for (i=0; i <
items.length; i++) {
nit: spaces

[dhollowa, 2011-07-28 20:13:26]

http://codereview.chromium.org/7522025/diff/4001/chrome/browser/prefs/session...
File chrome/browser/prefs/session_startup_pref.cc (right):

http://codereview.chromium.org/7522025/diff/4001/chrome/browser/prefs/session...
chrome/browser/prefs/session_startup_pref.cc:114: pref.type =
SessionStartupPref::DEFAULT;
This is a bit confusing.  DEFAULT really means "don't restore", so if the real
default value ever changes this will be needed.  I put it in for safety's sake.

On 2011/07/28 20:08:38, rsesek wrote:
> WHy do you need to do this? Won't pref.type still be default?

http://codereview.chromium.org/7522025/diff/4001/chrome/browser/resources/opt...
File chrome/browser/resources/options/browser_options.js (right):

http://codereview.chromium.org/7522025/diff/4001/chrome/browser/resources/opt...
chrome/browser/resources/options/browser_options.js:246: for (i=0; i <
items.length; i++) {
On 2011/07/28 20:08:38, rsesek wrote:
> nit: spaces

Done.

[Robert Sesek, 2011-07-28 20:14:07]

http://codereview.chromium.org/7522025/diff/4001/chrome/browser/prefs/session...
File chrome/browser/prefs/session_startup_pref.cc (right):

http://codereview.chromium.org/7522025/diff/4001/chrome/browser/prefs/session...
chrome/browser/prefs/session_startup_pref.cc:114: pref.type =
SessionStartupPref::DEFAULT;
On 2011/07/28 20:13:26, dhollowa wrote:
> This is a bit confusing.  DEFAULT really means "don't restore", so if the real
> default value ever changes this will be needed.  I put it in for safety's
sake.

Makes sense. Leave a comment for the next guy?

[dhollowa, 2011-07-28 20:18:48]

http://codereview.chromium.org/7522025/diff/4001/chrome/browser/prefs/session...
File chrome/browser/prefs/session_startup_pref.cc (right):

http://codereview.chromium.org/7522025/diff/4001/chrome/browser/prefs/session...
chrome/browser/prefs/session_startup_pref.cc:114: pref.type =
SessionStartupPref::DEFAULT;
On 2011/07/28 20:14:07, rsesek wrote:
> On 2011/07/28 20:13:26, dhollowa wrote:
> > This is a bit confusing.  DEFAULT really means "don't restore", so if the
real
> > default value ever changes this will be needed.  I put it in for safety's
> sake.
> 
> Makes sense. Leave a comment for the next guy?

Done.

[csilv, 2011-07-28 20:29:36]

LGTM w/ nits.

http://codereview.chromium.org/7522025/diff/11001/chrome/browser/resources/op...
File chrome/browser/resources/options/browser_options.js (right):

http://codereview.chromium.org/7522025/diff/11001/chrome/browser/resources/op...
chrome/browser/resources/options/browser_options.js:71:
[event.target.getAttribute("metric")]);
nit: use single quotes for metric

http://codereview.chromium.org/7522025/diff/11001/chrome/browser/resources/op...
chrome/browser/resources/options/browser_options.js:245: var items =
document.querySelectorAll('input[name=startup]')
nit: add semicolon to this line.

http://codereview.chromium.org/7522025/diff/11001/chrome/browser/ui/webui/opt...
File chrome/browser/ui/webui/options/browser_options_handler.h (right):

http://codereview.chromium.org/7522025/diff/11001/chrome/browser/ui/webui/opt...
chrome/browser/ui/webui/options/browser_options_handler.h:68: // Sets the
session restore setting to the integer values (enum) at index 0 of
nit: I think singular "value" is more correct in this comment

[dhollowa, 2011-07-28 20:51:37]

http://codereview.chromium.org/7522025/diff/11001/chrome/browser/resources/op...
File chrome/browser/resources/options/browser_options.js (right):

http://codereview.chromium.org/7522025/diff/11001/chrome/browser/resources/op...
chrome/browser/resources/options/browser_options.js:71:
[event.target.getAttribute("metric")]);
On 2011/07/28 20:29:36, csilv wrote:
> nit: use single quotes for metric

Done.

http://codereview.chromium.org/7522025/diff/11001/chrome/browser/resources/op...
chrome/browser/resources/options/browser_options.js:245: var items =
document.querySelectorAll('input[name=startup]')
On 2011/07/28 20:29:36, csilv wrote:
> nit: add semicolon to this line.

Done.

http://codereview.chromium.org/7522025/diff/11001/chrome/browser/ui/webui/opt...
File chrome/browser/ui/webui/options/browser_options_handler.h (right):

http://codereview.chromium.org/7522025/diff/11001/chrome/browser/ui/webui/opt...
chrome/browser/ui/webui/options/browser_options_handler.h:68: // Sets the
session restore setting to the integer values (enum) at index 0 of
On 2011/07/28 20:29:36, csilv wrote:
> nit: I think singular "value" is more correct in this comment

Done.

[Robert Sesek, 2011-07-29 21:33:16]

http://codereview.chromium.org/7522025/diff/8002/chrome/browser/ui/cocoa/wind...
File chrome/browser/ui/cocoa/window_restore_utils.mm (right):

http://codereview.chromium.org/7522025/diff/8002/chrome/browser/ui/cocoa/wind...
chrome/browser/ui/cocoa/window_restore_utils.mm:21: return !![defaults
boolForKey:@"NSQuitAlwaysKeepsWindows"];
Dave, take a look at http://codereview.chromium.org/7537003/. If the default is
supposed to be YES but is not set, you'll have to add a similar solution.