Cleanup: Pull some browser keep alive functions into its own file.

Cleanup: Pull some browser keep alive functions into its own file.

And make it desktop-only.

BUG=159847

[Lei Zhang, 2015-10-01 06:06:49]

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

[Lei Zhang, 2015-10-01 06:06:50]

How's this for a first step at pulling application_lifetime.h apart?

[sky, 2015-10-02 15:44:27]

https://codereview.chromium.org/1376063005/diff/40001/chrome/browser/lifetime...
File chrome/browser/lifetime/browser_keep_alive.h (right):

https://codereview.chromium.org/1376063005/diff/40001/chrome/browser/lifetime...
chrome/browser/lifetime/browser_keep_alive.h:15: void IncrementKeepAliveCount();
I like factoring this code out into it's file. But I think code like this is
error prone. Is it possible to make IncrementKeepAliveCount/Decrement private
and instead force use through a class whose constructor IncrementKeepAliveCount
and destructor Decrements?

[Lei Zhang, 2015-10-07 00:54:53]

On 2015/10/02 15:44:27, sky wrote:
>
https://codereview.chromium.org/1376063005/diff/40001/chrome/browser/lifetime...
> File chrome/browser/lifetime/browser_keep_alive.h (right):
> 
>
https://codereview.chromium.org/1376063005/diff/40001/chrome/browser/lifetime...
> chrome/browser/lifetime/browser_keep_alive.h:15: void
IncrementKeepAliveCount();
> I like factoring this code out into it's file. But I think code like this is
> error prone. Is it possible to make IncrementKeepAliveCount/Decrement private
> and instead force use through a class whose constructor
IncrementKeepAliveCount
> and destructor Decrements?

We have a ScopedKeepAlive class. I'll see if I can move that in here and move
IncrementKeepAliveCount / DecrementKeepAliveCount to the .cc file.

[Lei Zhang, 2015-10-07 05:49:32]

Patchset #7 (id:120001) has been deleted

[Lei Zhang, 2015-10-07 05:52:23]

There are a few standalone IncrementKeepAliveCount / DecrementKeepAliveCount
uses that are hard to get rid of. All other places have been converted to
ScopedKeepAlive usage.

[sky, 2015-10-07 17:27:25]

https://codereview.chromium.org/1376063005/diff/140001/chrome/browser/backgro...
File chrome/browser/background/background_mode_manager.cc (right):

https://codereview.chromium.org/1376063005/diff/140001/chrome/browser/backgro...
chrome/browser/background/background_mode_manager.cc:130: keep_alive.reset(); 
// Explicitly reset().
You don't actually need this.

https://codereview.chromium.org/1376063005/diff/140001/chrome/browser/chromeo...
File chrome/browser/chromeos/login/ui/login_display_host_impl.cc (right):

https://codereview.chromium.org/1376063005/diff/140001/chrome/browser/chromeo...
chrome/browser/chromeos/login/ui/login_display_host_impl.cc:401:
keep_alive_.reset();
As this is the destructor you shouldn't need the explicit reset here.

https://codereview.chromium.org/1376063005/diff/140001/chrome/browser/devtool...
File chrome/browser/devtools/devtools_sanity_browsertest.cc (right):

https://codereview.chromium.org/1376063005/diff/140001/chrome/browser/devtool...
chrome/browser/devtools/devtools_sanity_browsertest.cc:714:
browser_lifetime::IncrementKeepAliveCount();
Could you use a scopedkeepalive here that is reset on 724 ish?

https://codereview.chromium.org/1376063005/diff/140001/chrome/browser/lifetim...
File chrome/browser/lifetime/browser_close_manager_browsertest.cc (right):

https://codereview.chromium.org/1376063005/diff/140001/chrome/browser/lifetim...
chrome/browser/lifetime/browser_close_manager_browsertest.cc:980:
browser_lifetime::IncrementKeepAliveCount();
And a scopedkeepalive here that is reset on 992ish?

https://codereview.chromium.org/1376063005/diff/140001/chrome/browser/lifetim...
chrome/browser/lifetime/browser_close_manager_browsertest.cc:1026:
browser_lifetime::IncrementKeepAliveCount();
And a scopedkeepalive that is reset on 1037 ish?

https://codereview.chromium.org/1376063005/diff/140001/chrome/browser/lifetim...
File chrome/browser/lifetime/browser_keep_alive.cc (right):

https://codereview.chromium.org/1376063005/diff/140001/chrome/browser/lifetim...
chrome/browser/lifetime/browser_keep_alive.cc:17: int g_keep_alive_count = 0;
These aren't global, they are file local. My understanding is they don't get the
g_ here. Just keep_alive_count and disable_shutdown_for_testing.

https://codereview.chromium.org/1376063005/diff/140001/chrome/browser/lifetim...
File chrome/browser/lifetime/browser_keep_alive.h (right):

https://codereview.chromium.org/1376063005/diff/140001/chrome/browser/lifetim...
chrome/browser/lifetime/browser_keep_alive.h:10: namespace browser_lifetime {
I don't think I would go with any namespace here. But I'm fine with this if you
want it.

https://codereview.chromium.org/1376063005/diff/140001/chrome/browser/lifetim...
chrome/browser/lifetime/browser_keep_alive.h:29: void IncrementKeepAliveCount();
WDYT of moving these to ScopedKeepAlive, making them private state and friending
the places that are hard? That way we shouldn't get new uses.

https://codereview.chromium.org/1376063005/diff/140001/chrome/browser/ui/view...
File chrome/browser/ui/views/auto_keep_alive.cc (right):

https://codereview.chromium.org/1376063005/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/auto_keep_alive.cc:17: if (desktop_type !=
chrome::HOST_DESKTOP_TYPE_ASH) {
nit: no {}

https://codereview.chromium.org/1376063005/diff/140001/chrome/browser/ui/view...
chrome/browser/ui/views/auto_keep_alive.cc:23: keep_alive_.reset();
Shouldn't be necessary as this is the destructor.

[Lei Zhang, 2017-04-14 04:45:01]

Description was changed from

==========
Cleanup: Pull some browser keep alive functions into its own file.

And make it desktop-only.

BUG=159847
==========

to

==========
Cleanup: Pull some browser keep alive functions into its own file.

And make it desktop-only.

BUG=159847
==========