Ensure that on Windows 7, relaunching the browser via chrome://restart or changing about:flags rela…

Ensure that on Windows 7, relaunching the browser via chrome://restart or changing about:flags relaunches into Chrome OS mode.

There were two problems here.
1. The viewer process on Windows 7 would not die : Fixed by destroying the window when we get a
   signal from the browser process that the viewer needs to die.

2. Relaunching on Windows 7 would relaunch the existing browser process with the same command line.
   On Windows 8 relaunch launches back into Windows 8 mode as delegate execute has code to launch back
   into the previous mode. On Windows 7 we don't have that luxury. The current approach is to detect
   that we are in Windows 7 in a browser launched to service the viewer process and return that we
   need to launch into Chrome OS mode. Additionally we set the corresponding pref to ensure that
   this is honored.

BUG=413101
Committed: https://crrev.com/069fc881828aa504dc01891d2db80fc377d02643
Cr-Commit-Position: refs/heads/master@{#294721}

[ananta, 2014-09-11 23:02:18]

ananta@chromium.org changed reviewers:
+ cpu@chromium.org

[ananta, 2014-09-11 23:02:19]

sky : Please look at the upgrade_util_win.cc changes.
cpu : Please look at both files.

[sky, 2014-09-12 02:25:07]

LGTM

[cpu_(ooo_6.6-7.5), 2014-09-12 21:08:17]

the ash change lgtm. Please have Shrikant bless the other file change.

[Shrikant Kelkar, 2014-09-12 21:53:28]

shrikant@chromium.org changed reviewers:
+ shrikant@chromium.org

[Shrikant Kelkar, 2014-09-12 21:53:29]

https://codereview.chromium.org/568623002/diff/20001/chrome/browser/first_run...
File chrome/browser/first_run/upgrade_util_win.cc (right):

https://codereview.chromium.org/568623002/diff/20001/chrome/browser/first_run...
chrome/browser/first_run/upgrade_util_win.cc:131: 
Why temporary patch here? Why not make it uniform across windows 8/7?
Also have you considered side by side dual-mode?

https://codereview.chromium.org/568623002/diff/20001/win8/metro_driver/chrome...
File win8/metro_driver/chrome_app_view_ash.cc (right):

https://codereview.chromium.org/568623002/diff/20001/win8/metro_driver/chrome...
win8/metro_driver/chrome_app_view_ash.cc:976: 
Instead of using multiple close methods may be merge it into above
TERMINATE_USING_KEY_SEQUENCE? Or do it during app_exit somewhere?

[ananta, 2014-09-12 23:49:59]

https://codereview.chromium.org/568623002/diff/20001/chrome/browser/first_run...
File chrome/browser/first_run/upgrade_util_win.cc (right):

https://codereview.chromium.org/568623002/diff/20001/chrome/browser/first_run...
chrome/browser/first_run/upgrade_util_win.cc:131: 
On 2014/09/12 21:53:28, Shrikant Kelkar wrote:
> Why temporary patch here? Why not make it uniform across windows 8/7?
> Also have you considered side by side dual-mode?

We don't know yet what we want to do with the launch in chrome os mode.
which is why there is a TODO here.

Unifying is a great idea. however this will open up more testing for QA even if
we use the other path for Windows 8.

We will be revisiting this.

Dual mode is a legitimate thing to worry about. We will cross that bridge when
it comes.

https://codereview.chromium.org/568623002/diff/20001/win8/metro_driver/chrome...
File win8/metro_driver/chrome_app_view_ash.cc (right):

https://codereview.chromium.org/568623002/diff/20001/win8/metro_driver/chrome...
win8/metro_driver/chrome_app_view_ash.cc:976: 
On 2014/09/12 21:53:29, Shrikant Kelkar wrote:
> Instead of using multiple close methods may be merge it into above
> TERMINATE_USING_KEY_SEQUENCE? Or do it during app_exit somewhere?

This function is fragile and the ordering of statements causes timing and
related failures. Doing it in app exit will not work on Windows 8 as that is 
a windows object providing this functionality.

[Shrikant Kelkar, 2014-09-12 23:56:57]

On 2014/09/12 23:49:59, ananta wrote:
>
https://codereview.chromium.org/568623002/diff/20001/chrome/browser/first_run...
> File chrome/browser/first_run/upgrade_util_win.cc (right):
> 
>
https://codereview.chromium.org/568623002/diff/20001/chrome/browser/first_run...
> chrome/browser/first_run/upgrade_util_win.cc:131: 
> On 2014/09/12 21:53:28, Shrikant Kelkar wrote:
> > Why temporary patch here? Why not make it uniform across windows 8/7?
> > Also have you considered side by side dual-mode?
> 
> We don't know yet what we want to do with the launch in chrome os mode.
> which is why there is a TODO here.
> 
> Unifying is a great idea. however this will open up more testing for QA even
if
> we use the other path for Windows 8.
> 
> We will be revisiting this.
> 
> Dual mode is a legitimate thing to worry about. We will cross that bridge when
> it comes.
> 
>
https://codereview.chromium.org/568623002/diff/20001/win8/metro_driver/chrome...
> File win8/metro_driver/chrome_app_view_ash.cc (right):
> 
>
https://codereview.chromium.org/568623002/diff/20001/win8/metro_driver/chrome...
> win8/metro_driver/chrome_app_view_ash.cc:976: 
> On 2014/09/12 21:53:29, Shrikant Kelkar wrote:
> > Instead of using multiple close methods may be merge it into above
> > TERMINATE_USING_KEY_SEQUENCE? Or do it during app_exit somewhere?
> 
> This function is fragile and the ordering of statements causes timing and
> related failures. Doing it in app exit will not work on Windows 8 as that is 
> a windows object providing this functionality.

If you are saying this is all temporary (though I doubt) lgtm!

[ananta, 2014-09-13 00:24:43]

The CQ bit was checked by ananta@chromium.org

[commit-bot: I haz the power, 2014-09-13 00:32:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/568623002/20001

[commit-bot: I haz the power, 2014-09-13 01:24:25]

Committed patchset #2 (id:20001) as a3645b8ce814cf6a4c37f66b6a6f28ba4f28f873

[commit-bot: I haz the power, 2014-09-13 01:29:27]

Patchset 2 (id:??) landed as
https://crrev.com/069fc881828aa504dc01891d2db80fc377d02643
Cr-Commit-Position: refs/heads/master@{#294721}