Turn on -Wexit-time-destructors for ui and content.

Turn on -Wexit-time-destructors for ui and content.

I had hoped to just turn this on globally after fixing all instances, but people
add these fairly often. So enable the warning for clean targets incrementally.

BUG=10600
TEST=none
TBR=ben

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

[Mark Mentovai, 2011-11-09 17:44:44]

http://codereview.chromium.org/8495043/diff/7004/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/8495043/diff/7004/build/common.gypi#newcode1174
build/common.gypi:1174: ['enable_wexit_time_destructors==1', {
Or you could write “and clang==1” here to avoid having the extra conditions
block contained within this.

http://codereview.chromium.org/8495043/diff/7004/content/browser/renderer_hos...
File content/browser/renderer_host/render_widget_host_view_gtk.cc (right):

http://codereview.chromium.org/8495043/diff/7004/content/browser/renderer_hos...
content/browser/renderer_host/render_widget_host_view_gtk.cc:1026:
CR_DEFINE_STATIC_LOCAL(gfx::Size, drag_monitor_size, ());
Separate this and the .gyp work into distinct changes, please.

http://codereview.chromium.org/8495043/diff/7004/content/content.gyp
File content/content.gyp (right):

http://codereview.chromium.org/8495043/diff/7004/content/content.gyp#newcode30
content/content.gyp:30: 'variables': { 'enable_wexit_time_destructors': 1, },
Not strictly necessary for a none-type target, but see the next comment.

http://codereview.chromium.org/8495043/diff/7004/content/content.gyp#newcode45
content/content.gyp:45: 'variables': { 'enable_wexit_time_destructors': 1, },
Why not put this in the variables block at the top of this file next to
chromium_code? It looks like all of the targets in this file want to
participate, and the new GYP variable functions the same as chromium_code, so
that seems like a logical place for it.

http://codereview.chromium.org/8495043/diff/7004/ui/base/x/x11_util.cc
File ui/base/x/x11_util.cc (right):

http://codereview.chromium.org/8495043/diff/7004/ui/base/x/x11_util.cc#newcod...
ui/base/x/x11_util.cc:219: CR_DEFINE_STATIC_LOCAL(XCursorCache, cache, ());
Break out. I won’t mention this again during this review.

http://codereview.chromium.org/8495043/diff/7004/ui/ui.gyp
File ui/ui.gyp (left):

http://codereview.chromium.org/8495043/diff/7004/ui/ui.gyp#oldcode4
ui/ui.gyp:4: 
Don’t remove this line, most of our files seem to have it.

http://codereview.chromium.org/8495043/diff/7004/ui/ui.gyp
File ui/ui.gyp (right):

http://codereview.chromium.org/8495043/diff/7004/ui/ui.gyp#newcode22
ui/ui.gyp:22: 'variables': { 'enable_wexit_time_destructors': 1, },
Can this go into the top-level 'variables' in this file next to 'chromium_code'?

[Nico, 2011-11-09 18:16:02]

Thanks!

http://codereview.chromium.org/8495043/diff/7004/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/8495043/diff/7004/build/common.gypi#newcode1174
build/common.gypi:1174: ['enable_wexit_time_destructors==1', {
On 2011/11/09 17:44:44, Mark Mentovai wrote:
> Or you could write “and clang==1” here to avoid having the extra conditions
> block contained within this.

That yields: NameError: name 'clang' is not defined while evaluating condition
'enable_wexit_time_destructors==1 and clang==1' in ui/ui.gyp

I could copy the clang variable around to fix this, but I'm likely to forget to
undo that when I delete this bit once it's no longer needed.

http://codereview.chromium.org/8495043/diff/7004/content/browser/renderer_hos...
File content/browser/renderer_host/render_widget_host_view_gtk.cc (right):

http://codereview.chromium.org/8495043/diff/7004/content/browser/renderer_hos...
content/browser/renderer_host/render_widget_host_view_gtk.cc:1026:
CR_DEFINE_STATIC_LOCAL(gfx::Size, drag_monitor_size, ());
On 2011/11/09 17:44:44, Mark Mentovai wrote:
> Separate this and the .gyp work into distinct changes, please.

Done.

http://codereview.chromium.org/8495043/diff/7004/content/content.gyp
File content/content.gyp (right):

http://codereview.chromium.org/8495043/diff/7004/content/content.gyp#newcode30
content/content.gyp:30: 'variables': { 'enable_wexit_time_destructors': 1, },
On 2011/11/09 17:44:44, Mark Mentovai wrote:
> Not strictly necessary for a none-type target, but see the next comment.

Removed.

http://codereview.chromium.org/8495043/diff/7004/content/content.gyp#newcode45
content/content.gyp:45: 'variables': { 'enable_wexit_time_destructors': 1, },
On 2011/11/09 17:44:44, Mark Mentovai wrote:
> Why not put this in the variables block at the top of this file next to
> chromium_code? It looks like all of the targets in this file want to
> participate, and the new GYP variable functions the same as chromium_code, so
> that seems like a logical place for it.

I believe the included gypi files get it too in that case, and the tests / the
shell aren't clean.

http://codereview.chromium.org/8495043/diff/7004/ui/base/x/x11_util.cc
File ui/base/x/x11_util.cc (right):

http://codereview.chromium.org/8495043/diff/7004/ui/base/x/x11_util.cc#newcod...
ui/base/x/x11_util.cc:219: CR_DEFINE_STATIC_LOCAL(XCursorCache, cache, ());
On 2011/11/09 17:44:44, Mark Mentovai wrote:
> Break out. I won’t mention this again during this review.

(Un)done. I won't say "done" for the other instances where I (un)did this.

http://codereview.chromium.org/8495043/diff/7004/ui/ui.gyp
File ui/ui.gyp (left):

http://codereview.chromium.org/8495043/diff/7004/ui/ui.gyp#oldcode4
ui/ui.gyp:4: 
On 2011/11/09 17:44:44, Mark Mentovai wrote:
> Don’t remove this line, most of our files seem to have it.

Fine, let's waste those bytes. Done.

http://codereview.chromium.org/8495043/diff/7004/ui/ui.gyp
File ui/ui.gyp (right):

http://codereview.chromium.org/8495043/diff/7004/ui/ui.gyp#newcode22
ui/ui.gyp:22: 'variables': { 'enable_wexit_time_destructors': 1, },
On 2011/11/09 17:44:44, Mark Mentovai wrote:
> Can this go into the top-level 'variables' in this file next to
'chromium_code'?

No, then it's enabled for tests as well, and they aren't clean yet.

[Mark Mentovai, 2011-11-09 18:18:36]

LGTM

[Nico, 2011-11-09 18:45:46]

ben: OWNERS

[Ben Goodger (Google), 2011-11-09 18:48:35]

lgtm