Leak ResourceBundle during browser shutdown

Leak ResourceBundle during browser shutdown since we are exiting anyway. OS will reclaim the memory once the process gone.  


BUG=130768, 127266
TEST=


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

[bshe, 2012-06-11 22:11:21]

Hi Tony.

As discussed in email thread, it should be safe to leak ResourceBundle when
chrome shutdown. I found that there is a Boolean to control the destruction of
ResourceBundle. So I set the flag to false on ChromeOS when chrome shut down. I
am not familiar with chrome shut down behavior and I am not sure if I am on the
right track. Could you please take a quick look?

Thanks!

[tony, 2012-06-11 22:16:57]

Scott added this flag, but it looks like it's currently unused because all tests
leak resources (see chrome/browser/chrome_browser_main.cc:394).

Why would we want different behavior from ChromeOS vs Chrome?  We should just
leak everywhere.

[bshe, 2012-06-11 23:01:00]

On 2012/06/11 22:16:57, tony wrote:
> Scott added this flag, but it looks like it's currently unused because all
tests
> leak resources (see chrome/browser/chrome_browser_main.cc:394).
> 
> Why would we want different behavior from ChromeOS vs Chrome?  We should just
> leak everywhere.

My understanding was that I should not leak the memory after shut down on other
platforms so that some other programs can use the memory. But for chromeos,
since we always have chrome running, so it is fine to leak the ResourceBundle
memory. But I might be wrong.

[tony, 2012-06-11 23:06:09]

On 2012/06/11 23:01:00, bshe wrote:
> On 2012/06/11 22:16:57, tony wrote:
> > Scott added this flag, but it looks like it's currently unused because all
> tests
> > leak resources (see chrome/browser/chrome_browser_main.cc:394).
> > 
> > Why would we want different behavior from ChromeOS vs Chrome?  We should
just
> > leak everywhere.
> 
> My understanding was that I should not leak the memory after shut down on
other
> platforms so that some other programs can use the memory. But for chromeos,
> since we always have chrome running, so it is fine to leak the ResourceBundle
> memory. But I might be wrong.

Once the process is gone, the OS will reclaim the memory.

[bshe, 2012-06-12 00:10:00]

On 2012/06/11 23:06:09, tony wrote:
> On 2012/06/11 23:01:00, bshe wrote:
> > On 2012/06/11 22:16:57, tony wrote:
> > > Scott added this flag, but it looks like it's currently unused because all
> > tests
> > > leak resources (see chrome/browser/chrome_browser_main.cc:394).
> > > 
> > > Why would we want different behavior from ChromeOS vs Chrome?  We should
> just
> > > leak everywhere.
> > 
> > My understanding was that I should not leak the memory after shut down on
> other
> > platforms so that some other programs can use the memory. But for chromeos,
> > since we always have chrome running, so it is fine to leak the
ResourceBundle
> > memory. But I might be wrong.
> 
> Once the process is gone, the OS will reclaim the memory.

Got it. I thought we can somehow keep the leaked resourcebundle memory. My
misunderstanding. Will change it to all platform. Thanks!

[tony, 2012-06-12 17:45:08]

http://codereview.chromium.org/10539091/diff/9001/chrome/browser/browser_shut...
File chrome/browser/browser_shutdown.cc (right):

http://codereview.chromium.org/10539091/diff/9001/chrome/browser/browser_shut...
chrome/browser/browser_shutdown.cc:73: bool delete_resources_on_shutdown =
false;
Since this is no longer used, you can probably just remove this variable.

[bshe, 2012-06-13 17:04:37]

http://codereview.chromium.org/10539091/diff/9001/chrome/browser/browser_shut...
File chrome/browser/browser_shutdown.cc (right):

http://codereview.chromium.org/10539091/diff/9001/chrome/browser/browser_shut...
chrome/browser/browser_shutdown.cc:73: bool delete_resources_on_shutdown =
false;
On 2012/06/12 17:45:08, tony wrote:
> Since this is no longer used, you can probably just remove this variable.

Done.

[tony, 2012-06-13 17:31:51]

LGTM.  Please fix the change description and include some 'why' text.  E.g.,
Leak ResourceBundle during shutdown since we're just going to exit anyway.

[bshe, 2012-06-13 20:30:55]

On 2012/06/13 17:31:51, tony wrote:
> LGTM.  Please fix the change description and include some 'why' text.  E.g.,
> Leak ResourceBundle during shutdown since we're just going to exit anyway.

friendly ping? sky@ Could you please take a look since you add the flag?

[sky, 2012-06-13 22:04:24]

LGTM

[commit-bot: I haz the power, 2012-06-14 14:46:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bshe@chromium.org/10539091/9002

[commit-bot: I haz the power, 2012-06-14 17:40:54]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bshe@chromium.org/10539091/9002

[commit-bot: I haz the power, 2012-06-14 19:33:38]

Change committed as 142194