[Mac] Add a confirm to quit experiment to about:flags.

[Mac] Add a confirm to quit experiment to about:flags.

BUG=27786, 60591
TEST=Enable 'Confirm to Quit' in about:flags. When you press Cmd+Q, it brings up the floaty panel telling you to hold the key combo. When you do, it quits after 1.5 seconds.

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

[Robert Sesek, 2010-10-29 20:05:32]

Screen shot: http://cl.ly/7b3985d7ad4f7b9b8ac4

[pink (ping after 24hrs), 2010-11-01 13:47:33]

No major problems, just small things.

http://codereview.chromium.org/4220005/diff/8001/9004
File chrome/browser/app_controller_mac.mm (right):

http://codereview.chromium.org/4220005/diff/8001/9004#newcode246
chrome/browser/app_controller_mac.mm:246: // Check if the user really wants to
quit by employing the hold-to-quit
everywhere else you call it "confirm-to-quit". Stay consistent?

http://codereview.chromium.org/4220005/diff/8001/9004#newcode284
chrome/browser/app_controller_mac.mm:284: // invocation, use the special sauce
to prevent accidental quitting.
can you describe what that sauce is? or a pointer to where one might find a
writeup of what happens?

http://codereview.chromium.org/4220005/diff/8001/9004#newcode294
chrome/browser/app_controller_mac.mm:294: NSDate* targetDate = [NSDate
dateWithTimeIntervalSinceNow:1.5];
make the 1.5 a constant?

http://codereview.chromium.org/4220005/diff/8001/9004#newcode296
chrome/browser/app_controller_mac.mm:296: while (YES) {
why not make this a do { } while() loop? seems a bit cleaner

http://codereview.chromium.org/4220005/diff/8001/9004#newcode308
chrome/browser/app_controller_mac.mm:308: if (difference < 1.0) {
constant?

http://codereview.chromium.org/4220005/diff/8001/9005
File chrome/browser/cocoa/confirm_quit_panel_controller.h (right):

http://codereview.chromium.org/4220005/diff/8001/9005#newcode7
chrome/browser/cocoa/confirm_quit_panel_controller.h:7: #include
"base/cocoa_protocols_mac.h"
why is this needed?

http://codereview.chromium.org/4220005/diff/8001/9007
File chrome/browser/cocoa/confirm_quit_panel_controller_unittest.mm (right):

http://codereview.chromium.org/4220005/diff/8001/9007#newcode14
chrome/browser/cocoa/confirm_quit_panel_controller_unittest.mm:14: #import
"chrome/browser/cocoa/page_info_bubble_controller.h"
do you need all these includes?

[Robert Sesek, 2010-11-01 21:33:08]

Addressed all comments.

http://codereview.chromium.org/4220005/diff/8001/9004
File chrome/browser/app_controller_mac.mm (right):

http://codereview.chromium.org/4220005/diff/8001/9004#newcode246
chrome/browser/app_controller_mac.mm:246: // Check if the user really wants to
quit by employing the hold-to-quit
On 2010/11/01 13:47:33, pink wrote:
> everywhere else you call it "confirm-to-quit". Stay consistent?

Done.

http://codereview.chromium.org/4220005/diff/8001/9004#newcode284
chrome/browser/app_controller_mac.mm:284: // invocation, use the special sauce
to prevent accidental quitting.
On 2010/11/01 13:47:33, pink wrote:
> can you describe what that sauce is? or a pointer to where one might find a
> writeup of what happens?

Added a design doc and put the link here.

http://codereview.chromium.org/4220005/diff/8001/9004#newcode294
chrome/browser/app_controller_mac.mm:294: NSDate* targetDate = [NSDate
dateWithTimeIntervalSinceNow:1.5];
On 2010/11/01 13:47:33, pink wrote:
> make the 1.5 a constant?

Done.

http://codereview.chromium.org/4220005/diff/8001/9004#newcode296
chrome/browser/app_controller_mac.mm:296: while (YES) {
On 2010/11/01 13:47:33, pink wrote:
> why not make this a do { } while() loop? seems a bit cleaner

Done.

http://codereview.chromium.org/4220005/diff/8001/9004#newcode308
chrome/browser/app_controller_mac.mm:308: if (difference < 1.0) {
On 2010/11/01 13:47:33, pink wrote:
> constant?

Done.

http://codereview.chromium.org/4220005/diff/8001/9005
File chrome/browser/cocoa/confirm_quit_panel_controller.h (right):

http://codereview.chromium.org/4220005/diff/8001/9005#newcode7
chrome/browser/cocoa/confirm_quit_panel_controller.h:7: #include
"base/cocoa_protocols_mac.h"
On 2010/11/01 13:47:33, pink wrote:
> why is this needed?

NSWindowDelegate.

http://codereview.chromium.org/4220005/diff/8001/9007
File chrome/browser/cocoa/confirm_quit_panel_controller_unittest.mm (right):

http://codereview.chromium.org/4220005/diff/8001/9007#newcode14
chrome/browser/cocoa/confirm_quit_panel_controller_unittest.mm:14: #import
"chrome/browser/cocoa/page_info_bubble_controller.h"
On 2010/11/01 13:47:33, pink wrote:
> do you need all these includes?

No. Copy & paste artifact.

[viettrungluu, 2010-11-02 17:28:13]

LGTM, other than my probably-ill-founded fears.

http://codereview.chromium.org/4220005/diff/14001/15003
File chrome/browser/about_flags.cc (right):

http://codereview.chromium.org/4220005/diff/14001/15003#newcode147
chrome/browser/about_flags.cc:147: "confirm-to-quit",   // Do not change; see
above.
One might complain that the "see above" part is a total tease. I blame thakis.

http://codereview.chromium.org/4220005/diff/14001/15004
File chrome/browser/app_controller_mac.mm (right):

http://codereview.chromium.org/4220005/diff/14001/15004#newcode278
chrome/browser/app_controller_mac.mm:278: // Check if the experiment is enabled.
Probably you should say which experiment.

http://codereview.chromium.org/4220005/diff/14001/15004#newcode307
chrome/browser/app_controller_mac.mm:307: do {
Generically, I'm kind of scared of this, though I think it should be okay
(nested loops in event tracking mode are all over the place). To reassure me,
could you check that nothing bad happens if, e.g., Javascript opens a new
window, or pops up a modal alert, or an extension closes a window? (Can any of
those things happen?)

[pink (ping after 24hrs), 2010-11-02 19:39:20]

lgtm, thanks for writing that design doc.

[Robert Sesek, 2010-11-02 20:05:58]

http://codereview.chromium.org/4220005/diff/14001/15004
File chrome/browser/app_controller_mac.mm (right):

http://codereview.chromium.org/4220005/diff/14001/15004#newcode307
chrome/browser/app_controller_mac.mm:307: do {
On 2010/11/02 17:28:13, viettrungluu wrote:
> Generically, I'm kind of scared of this, though I think it should be okay
> (nested loops in event tracking mode are all over the place). To reassure me,
> could you check that nothing bad happens if, e.g., Javascript opens a new
> window, or pops up a modal alert, or an extension closes a window? (Can any of
> those things happen?)

Open new window: OK
Extension close window: OK
JS modal alert: bad

I'll need to dig more how to make JS modal alerts not bork it, but considering
this is behind a flag, I'm OK with shipping this and figuring out that edge case
later. We may never even ship this, too. But if we do, all platforms will face
this issue and we should figure out a good way to handle it across all of them.

[Nico, 2010-11-03 17:55:25]

http://codereview.chromium.org/4220005/diff/14001/15003
File chrome/browser/about_flags.cc (right):

http://codereview.chromium.org/4220005/diff/14001/15003#newcode147
chrome/browser/about_flags.cc:147: "confirm-to-quit",   // Do not change; see
above.
On 2010/11/02 17:28:13, viettrungluu wrote:
> One might complain that the "see above" part is a total tease. I blame thakis.

Can you elaborate on this untenable accusation?