Add "Relaunch Chrome" dialog to Mac Views.

Add "Relaunch Chrome" dialog to Mac Views.

This implements the Relaunch Chrome dialog as a Views based dialog on the mac
platform.

BUG=684167
TEST=./out/Default/browser_tests --gtest_filter=BrowserDialogTest.Invoke
--interactive --dialog=UpdateRecommendedDialogTest.InvokeDialog_default

Review-Url: https://codereview.chromium.org/2652823003
Cr-Commit-Position: refs/heads/master@{#446601}
Committed: https://chromium.googlesource.com/chromium/src/+/28a279f9a121b7948b388257a43c79823bf311d6

[Greg K, 2017-01-24 00:21:16]

The CQ bit was checked by kerrnel@chromium.org to run a CQ dry run

[Greg K, 2017-01-24 00:21:34]

kerrnel@chromium.org changed reviewers:
+ tapted@chromium.org

[Greg K, 2017-01-24 00:21:34]

PTAL.

Thanks,

Greg

[commit-bot: I haz the power, 2017-01-24 00:21:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tapted, 2017-01-24 00:28:11]

basically lgtm - i'll get back to you on the windows 10 thing..

https://codereview.chromium.org/2652823003/diff/1/chrome/browser/ui/update_ch...
File chrome/browser/ui/update_chrome_dialog_browsertest.mm (right):

https://codereview.chromium.org/2652823003/diff/1/chrome/browser/ui/update_ch...
chrome/browser/ui/update_chrome_dialog_browsertest.mm:19: };
nit: private:
  DISALLOW_..

https://codereview.chromium.org/2652823003/diff/1/chrome/browser/ui/update_ch...
chrome/browser/ui/update_chrome_dialog_browsertest.mm:27: void
SetUpCommandLine(base::CommandLine* command_line) override {
I think we can just collapse this into the test above (the other CL needed to
split the test harness to support some existing tests)

https://codereview.chromium.org/2652823003/diff/1/chrome/test/BUILD.gn
File chrome/test/BUILD.gn (right):

https://codereview.chromium.org/2652823003/diff/1/chrome/test/BUILD.gn#newcod...
chrome/test/BUILD.gn:1295: # desktop platforms.
it's possible this fails on Windows 10 due to http://crbug.com/683808 - I'll try
patching it in. (if it does we can probably just #ifdef guard the test in that
case) - hopefully I have a fix for that soon

[Greg K, 2017-01-24 00:46:39]

The CQ bit was checked by kerrnel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-24 00:46:49]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Greg K, 2017-01-24 00:46:49]

https://codereview.chromium.org/2652823003/diff/1/chrome/browser/ui/update_ch...
File chrome/browser/ui/update_chrome_dialog_browsertest.mm (right):

https://codereview.chromium.org/2652823003/diff/1/chrome/browser/ui/update_ch...
chrome/browser/ui/update_chrome_dialog_browsertest.mm:19: };
On 2017/01/24 00:28:11, tapted wrote:
> nit: private:
>   DISALLOW_..

Done.

https://codereview.chromium.org/2652823003/diff/1/chrome/browser/ui/update_ch...
chrome/browser/ui/update_chrome_dialog_browsertest.mm:27: void
SetUpCommandLine(base::CommandLine* command_line) override {
On 2017/01/24 00:28:11, tapted wrote:
> I think we can just collapse this into the test above (the other CL needed to
> split the test harness to support some existing tests)

Done.

[commit-bot: I haz the power, 2017-01-24 02:52:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-24 02:52:37]

Dry run: This issue passed the CQ dry run.

[tapted, 2017-01-24 03:36:26]

https://codereview.chromium.org/2652823003/diff/20001/chrome/browser/ui/BUILD.gn
File chrome/browser/ui/BUILD.gn (right):

https://codereview.chromium.org/2652823003/diff/20001/chrome/browser/ui/BUILD...
chrome/browser/ui/BUILD.gn:1: # Copyright 2014 The Chromium Authors. All rights
reserved.
CL description nit: UpdateRecommendedDialogTestMd -> UpdateRecommendedDialogTest

https://codereview.chromium.org/2652823003/diff/20001/chrome/browser/ui/updat...
File chrome/browser/ui/update_chrome_dialog_browsertest.mm (right):

https://codereview.chromium.org/2652823003/diff/20001/chrome/browser/ui/updat...
chrome/browser/ui/update_chrome_dialog_browsertest.mm:1: // Copyright 2017 The
Chromium Authors. All rights reserved.
oh whoops - this file should be .cc, not .mm

https://codereview.chromium.org/2652823003/diff/20001/chrome/browser/ui/updat...
chrome/browser/ui/update_chrome_dialog_browsertest.mm:6: #include
"chrome/browser/ui/browser_dialogs.h"
we can drop this one - I don't think it's used

https://codereview.chromium.org/2652823003/diff/20001/chrome/browser/ui/updat...
chrome/browser/ui/update_chrome_dialog_browsertest.mm:7: #include
"chrome/browser/ui/cocoa/browser_window_cocoa.h"
chrome/browser/ui/browser_window.h

https://codereview.chromium.org/2652823003/diff/20001/chrome/browser/ui/updat...
chrome/browser/ui/update_chrome_dialog_browsertest.mm:15: //
TestDialogInterface:
whoops - this class doesn't exist (I landed a CL to fix the other browsertest.cc
file but it got reverted :p). This should be // DialogBrowserTest:

https://codereview.chromium.org/2652823003/diff/20001/chrome/browser/ui/updat...
chrome/browser/ui/update_chrome_dialog_browsertest.mm:32:
IN_PROC_BROWSER_TEST_F(UpdateRecommendedDialogTest, InvokeDialog_default) {
Yeah this fails on Windows 10 due to http://crbug.com/683808, but it works after
I apply https://codereview.chromium.org/2645253002/

can you add

#if defined(OS_WIN)
// Initially disabled on Windows due to http://crbug.com/683808.
#define MAYBE_InvokeDialog_default DISABLED_InvokeDialog_default
#else
#define MAYBE_InvokeDialog_default InvokeDialog_default
#endif

but.. to pass on Linux we might also need the diff in
https://codereview.chromium.org/2625813003/diff/140001/chrome/browser/ui/test...

in which case... perhaps

#if !defined(OS_MACOSX)
// Initially disabled except on Mac due to http://crbug.com/683808.
#define MAYBE_InvokeDialog_default DISABLED_InvokeDialog_default
#else
#define MAYBE_InvokeDialog_default InvokeDialog_default
#endif

[Greg K, 2017-01-24 03:38:20]

Description was changed from

==========
Add "Relaunch Chrome" dialog to Mac Views.

This implements the Relaunch Chrome dialog as a Views based dialog on the mac
platform.

BUG=684167
TEST=./out/Default/browser_tests --gtest_filter=BrowserDialogTest.Invoke
--interactive --dialog=UpdateRecommendedDialogTestMd.InvokeDialog_default
==========

to

==========
Add "Relaunch Chrome" dialog to Mac Views.

This implements the Relaunch Chrome dialog as a Views based dialog on the mac
platform.

BUG=684167
TEST=./out/Default/browser_tests --gtest_filter=BrowserDialogTest.Invoke
--interactive --dialog=UpdateRecommendedDialogTest.InvokeDialog_default
==========

[Greg K, 2017-01-24 04:11:16]

The CQ bit was checked by kerrnel@chromium.org to run a CQ dry run

[Greg K, 2017-01-24 04:11:20]

https://codereview.chromium.org/2652823003/diff/20001/chrome/browser/ui/BUILD.gn
File chrome/browser/ui/BUILD.gn (right):

https://codereview.chromium.org/2652823003/diff/20001/chrome/browser/ui/BUILD...
chrome/browser/ui/BUILD.gn:1: # Copyright 2014 The Chromium Authors. All rights
reserved.
On 2017/01/24 03:36:25, tapted wrote:
> CL description nit: UpdateRecommendedDialogTestMd ->
UpdateRecommendedDialogTest

Done.

https://codereview.chromium.org/2652823003/diff/20001/chrome/browser/ui/updat...
File chrome/browser/ui/update_chrome_dialog_browsertest.mm (right):

https://codereview.chromium.org/2652823003/diff/20001/chrome/browser/ui/updat...
chrome/browser/ui/update_chrome_dialog_browsertest.mm:1: // Copyright 2017 The
Chromium Authors. All rights reserved.
On 2017/01/24 03:36:26, tapted wrote:
> oh whoops - this file should be .cc, not .mm

Done.

https://codereview.chromium.org/2652823003/diff/20001/chrome/browser/ui/updat...
chrome/browser/ui/update_chrome_dialog_browsertest.mm:6: #include
"chrome/browser/ui/browser_dialogs.h"
On 2017/01/24 03:36:25, tapted wrote:
> we can drop this one - I don't think it's used

Done.

https://codereview.chromium.org/2652823003/diff/20001/chrome/browser/ui/updat...
chrome/browser/ui/update_chrome_dialog_browsertest.mm:7: #include
"chrome/browser/ui/cocoa/browser_window_cocoa.h"
On 2017/01/24 03:36:26, tapted wrote:
> chrome/browser/ui/browser_window.h

Done.

https://codereview.chromium.org/2652823003/diff/20001/chrome/browser/ui/updat...
chrome/browser/ui/update_chrome_dialog_browsertest.mm:15: //
TestDialogInterface:
On 2017/01/24 03:36:26, tapted wrote:
> whoops - this class doesn't exist (I landed a CL to fix the other
browsertest.cc
> file but it got reverted :p). This should be // DialogBrowserTest:

Done.

https://codereview.chromium.org/2652823003/diff/20001/chrome/browser/ui/updat...
chrome/browser/ui/update_chrome_dialog_browsertest.mm:32:
IN_PROC_BROWSER_TEST_F(UpdateRecommendedDialogTest, InvokeDialog_default) {
On 2017/01/24 03:36:26, tapted wrote:
> Yeah this fails on Windows 10 due to http://crbug.com/683808, but it works
after
> I apply https://codereview.chromium.org/2645253002/
> 
> can you add
> 
> #if defined(OS_WIN)
> // Initially disabled on Windows due to http://crbug.com/683808.
> #define MAYBE_InvokeDialog_default DISABLED_InvokeDialog_default
> #else
> #define MAYBE_InvokeDialog_default InvokeDialog_default
> #endif
> 
> but.. to pass on Linux we might also need the diff in
>
https://codereview.chromium.org/2625813003/diff/140001/chrome/browser/ui/test...
> 
> in which case... perhaps
> 
> #if !defined(OS_MACOSX)
> // Initially disabled except on Mac due to http://crbug.com/683808.
> #define MAYBE_InvokeDialog_default DISABLED_InvokeDialog_default
> #else
> #define MAYBE_InvokeDialog_default InvokeDialog_default
> #endif

Done.

[commit-bot: I haz the power, 2017-01-24 04:11:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[tapted, 2017-01-24 04:36:42]

lgtm - thanks!

https://codereview.chromium.org/2652823003/diff/40001/chrome/browser/ui/updat...
File chrome/browser/ui/update_chrome_dialog_browsertest.cc (right):

https://codereview.chromium.org/2652823003/diff/40001/chrome/browser/ui/updat...
chrome/browser/ui/update_chrome_dialog_browsertest.cc:10: #if
!defined(OS_MACOSX)
nit: move down to line 34 ? (I usually see these lines just above the TEST_F
lines)

https://codereview.chromium.org/2652823003/diff/40001/chrome/browser/ui/updat...
chrome/browser/ui/update_chrome_dialog_browsertest.cc:21: // DialogBrowserTesT:
nit: DialogBrowserTesT: -> DialogBrowserTest:

[commit-bot: I haz the power, 2017-01-24 05:07:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-24 05:07:58]

Dry run: This issue passed the CQ dry run.

[Greg K, 2017-01-24 05:57:08]

The CQ bit was checked by kerrnel@chromium.org to run a CQ dry run

[Greg K, 2017-01-24 05:57:17]

https://codereview.chromium.org/2652823003/diff/40001/chrome/browser/ui/updat...
File chrome/browser/ui/update_chrome_dialog_browsertest.cc (right):

https://codereview.chromium.org/2652823003/diff/40001/chrome/browser/ui/updat...
chrome/browser/ui/update_chrome_dialog_browsertest.cc:10: #if
!defined(OS_MACOSX)
On 2017/01/24 04:36:42, tapted wrote:
> nit: move down to line 34 ? (I usually see these lines just above the TEST_F
> lines)

Done.

https://codereview.chromium.org/2652823003/diff/40001/chrome/browser/ui/updat...
chrome/browser/ui/update_chrome_dialog_browsertest.cc:21: // DialogBrowserTesT:
On 2017/01/24 04:36:42, tapted wrote:
> nit: DialogBrowserTesT: -> DialogBrowserTest:

Done.

[commit-bot: I haz the power, 2017-01-24 05:57:22]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Greg K, 2017-01-24 05:57:31]

kerrnel@chromium.org changed reviewers:
+ thestig@chromium.org

[Greg K, 2017-01-24 05:57:32]

thestig@chromium.org: Please review changes in 

Please provide an OWNERS review.

Thanks,

Greg

[commit-bot: I haz the power, 2017-01-24 07:23:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-24 07:23:57]

Dry run: This issue passed the CQ dry run.

[Greg K, 2017-01-25 03:32:02]

kerrnel@chromium.org changed reviewers:
- thestig@chromium.org

[Greg K, 2017-01-25 03:32:34]

kerrnel@chromium.org changed reviewers:
+ jochen@chromium.org

[Greg K, 2017-01-25 03:32:35]

jochen@chromium.org: Please provide an OWNERS review.

Thanks,

Greg

[jochen (gone - plz use gerrit), 2017-01-25 12:19:37]

lgtm

[Greg K, 2017-01-27 02:22:51]

The CQ bit was checked by kerrnel@chromium.org

[Greg K, 2017-01-27 02:22:52]

The patchset sent to the CQ was uploaded after l-g-t-m from tapted@chromium.org
Link to the patchset: https://codereview.chromium.org/2652823003/#ps60001
(title: "Fix nits")

[commit-bot: I haz the power, 2017-01-27 02:24:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-27 04:38:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-27 04:38:36]

Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Greg K, 2017-01-27 04:43:13]

The CQ bit was checked by kerrnel@chromium.org

[commit-bot: I haz the power, 2017-01-27 04:43:40]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-27 05:22:32]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1485492193899000,
"parent_rev": "be58f69be3a0d8f714293bd03e6a57f1f88ca720", "commit_rev":
"28a279f9a121b7948b388257a43c79823bf311d6"}

[commit-bot: I haz the power, 2017-01-27 05:23:09]

Description was changed from

==========
Add "Relaunch Chrome" dialog to Mac Views.

This implements the Relaunch Chrome dialog as a Views based dialog on the mac
platform.

BUG=684167
TEST=./out/Default/browser_tests --gtest_filter=BrowserDialogTest.Invoke
--interactive --dialog=UpdateRecommendedDialogTest.InvokeDialog_default
==========

to

==========
Add "Relaunch Chrome" dialog to Mac Views.

This implements the Relaunch Chrome dialog as a Views based dialog on the mac
platform.

BUG=684167
TEST=./out/Default/browser_tests --gtest_filter=BrowserDialogTest.Invoke
--interactive --dialog=UpdateRecommendedDialogTest.InvokeDialog_default

Review-Url: https://codereview.chromium.org/2652823003
Cr-Commit-Position: refs/heads/master@{#446601}
Committed:
https://chromium.googlesource.com/chromium/src/+/28a279f9a121b7948b388257a43c...
==========

[commit-bot: I haz the power, 2017-01-27 05:23:10]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/28a279f9a121b7948b388257a43c...