In Chrome ASH on Windows 8 make sure that the browser object is closed before destroying it.

In Chrome ASH on Windows 8 make sure that the browser object is closed before destroying it.

In Chrome ASH on Windows 8 the ASH process could be closed with open browser windows. Make sure that we close them before
uninitializing ASH as that causes DCHECKs/crashes.

Added a local helper function CloseOpenAshBrowsers which runs through the list of open ASH browsers and closes them.

BUG=151718
R=sky
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=170493

[sky, 2012-11-28 22:44:57]

https://codereview.chromium.org/11434013/diff/6001/chrome/browser/ui/views/fr...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/11434013/diff/6001/chrome/browser/ui/views/fr...
chrome/browser/ui/views/frame/browser_view.cc:558: if
(!browser_->tab_strip_model()->empty())
This seems wrong. What code path is letting us get here?

[ananta, 2012-11-29 00:21:15]

On 2012/11/28 22:44:57, sky wrote:
>
https://codereview.chromium.org/11434013/diff/6001/chrome/browser/ui/views/fr...
> File chrome/browser/ui/views/frame/browser_view.cc (right):
> 
>
https://codereview.chromium.org/11434013/diff/6001/chrome/browser/ui/views/fr...
> chrome/browser/ui/views/frame/browser_view.cc:558: if
> (!browser_->tab_strip_model()->empty())
> This seems wrong. What code path is letting us get here?

Fixed this with a new function CloseAllBrowsersOfType  which is invoked in the
OnChannelError handling in the MetroViewerProcessHost class.

PTAL

[sky, 2012-11-29 01:53:26]

https://codereview.chromium.org/11434013/diff/8002/chrome/browser/lifetime/ap...
File chrome/browser/lifetime/application_lifetime.cc (right):

https://codereview.chromium.org/11434013/diff/8002/chrome/browser/lifetime/ap...
chrome/browser/lifetime/application_lifetime.cc:217: browser->window()->Close();
What if the close is vetoed for some reason?

[ananta, 2012-11-29 02:39:41]

https://codereview.chromium.org/11434013/diff/8002/chrome/browser/lifetime/ap...
File chrome/browser/lifetime/application_lifetime.cc (right):

https://codereview.chromium.org/11434013/diff/8002/chrome/browser/lifetime/ap...
chrome/browser/lifetime/application_lifetime.cc:217: browser->window()->Close();
On 2012/11/29 01:53:27, sky wrote:
> What if the close is vetoed for some reason?
I added a comment explaining this. The CloseAllBrowsers function above also
suffers from the same limitation, as were the other examples i found in the code
base. A possible option could be to expose the native CloseNow functionality in
the Widget which forcibly deletes the window. 

Did not deem it necessary for this use case as the actual Browser will be
destroyed anyway subsequently.

[ananta, 2012-11-29 18:33:16]

On 2012/11/29 01:53:26, sky wrote:
>
https://codereview.chromium.org/11434013/diff/8002/chrome/browser/lifetime/ap...
> File chrome/browser/lifetime/application_lifetime.cc (right):
> 
>
https://codereview.chromium.org/11434013/diff/8002/chrome/browser/lifetime/ap...
> chrome/browser/lifetime/application_lifetime.cc:217:
browser->window()->Close();
> What if the close is vetoed for some reason?

Fair enough. Added code to delete all tabs on the page after the Close call with
a comment explaining this.

Thanks
Ananta

[sky, 2012-11-29 20:28:03]

https://codereview.chromium.org/11434013/diff/11003/chrome/browser/lifetime/a...
File chrome/browser/lifetime/application_lifetime.cc (right):

https://codereview.chromium.org/11434013/diff/11003/chrome/browser/lifetime/a...
chrome/browser/lifetime/application_lifetime.cc:210: void
CloseAllBrowsersOfType(chrome::HostDesktopType type) {
I'm worried that keeping this here means other folks might call it when this
isn't meant for public consumption. Can you move it to
metro_viewer_process_host_win and name it something like
DeleteTabsInBrowserOfType to make it clear what's going on.

[ananta, 2012-11-29 21:55:42]

https://codereview.chromium.org/11434013/diff/11003/chrome/browser/lifetime/a...
File chrome/browser/lifetime/application_lifetime.cc (right):

https://codereview.chromium.org/11434013/diff/11003/chrome/browser/lifetime/a...
chrome/browser/lifetime/application_lifetime.cc:210: void
CloseAllBrowsersOfType(chrome::HostDesktopType type) {
On 2012/11/29 20:28:03, sky wrote:
> I'm worried that keeping this here means other folks might call it when this
> isn't meant for public consumption. Can you move it to
> metro_viewer_process_host_win and name it something like
> DeleteTabsInBrowserOfType to make it clear what's going on.

Done. Added a local function CloseOpenAshBrowsers to
metro_viewer_process_host_win to do this.

[sky, 2012-11-29 23:29:14]

LGTM