Views views_unittests native widget tests leak

Views views_unittests native widget tests leak

Eliminates leaks from NativeWidgetTest.*.  Adds additional test target to valgrind wrapper script.  Refactors native_widget_test_utils_{gtk|win}.cc to avoid duplication.

BUG=87805
TEST=tools/valgrind/chrome_tests.sh views --gtest_filter=NativeWidgetTest.*

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

[dhollowa, 2011-06-28 23:50:10]

ben -> views stuff
thestig -> valgrind addition

[Lei Zhang, 2011-06-29 01:00:30]

LGTM for tools/valgrind/chrome_tests.py changes.

[Timur Iskhodzhanov, 2011-06-29 09:24:01]

http://codereview.chromium.org/7272044/diff/1/tools/valgrind/chrome_tests.py
File tools/valgrind/chrome_tests.py (right):

http://codereview.chromium.org/7272044/diff/1/tools/valgrind/chrome_tests.py#...
tools/valgrind/chrome_tests.py:276: return self.SimpleTest("views",
"views_unittests")
a few questions:
a) is it single-process?
b) how long does it run under Valgrind?
c) should we add it to the memory waterfall?

[dhollowa, 2011-06-29 16:01:30]

On 2011/06/29 09:24:01, Timur Iskhodzhanov wrote:
> http://codereview.chromium.org/7272044/diff/1/tools/valgrind/chrome_tests.py
> File tools/valgrind/chrome_tests.py (right):
> 
>
http://codereview.chromium.org/7272044/diff/1/tools/valgrind/chrome_tests.py#...
> tools/valgrind/chrome_tests.py:276: return self.SimpleTest("views",
> "views_unittests")
> a few questions:
> a) is it single-process?

Yes.  The tests spin up multiple threads, but only one process AFAIKT.

> b) how long does it run under Valgrind?

On my Linux box:
  real    0m41.263s
  user    0m40.340s
  sys     0m0.540s

> c) should we add it to the memory waterfall?

Eventually I would like to, yes.  I'm weeding out some other leaks and
invalid-reads currently.  But once those are addressed it'd be great to increase
coverage here.  These tests are for GYP_DEFINES="toolkit_views=1" builds, i.e.
Windows and CrOS.  I don't know if we have Linux bots with this configuration.

[Timur Iskhodzhanov, 2011-06-29 16:07:13]

> > a) is it single-process?
> Yes.  The tests spin up multiple threads, but only one process AFAIKT.
[Patch sanity check] PASSED
 
> > b) how long does it run under Valgrind?
> On my Linux box:
>   real    0m41.263s
>   user    0m40.340s
>   sys     0m0.540s
Cool, shouldn't be a problem to add it to the bots.

> > c) should we add it to the memory waterfall?
> 
> Eventually I would like to, yes.  I'm weeding out some other leaks and
> invalid-reads currently.  But once those are addressed it'd be great to
increase
> coverage here.
Feel free to ping me then.

> These tests are for GYP_DEFINES="toolkit_views=1" builds, i.e.
> Windows and CrOS.  I don't know if we have Linux bots with this configuration.
Nope we don't have. Should we? It will change unit/ui tests as well, right?

[Timur Iskhodzhanov, 2011-06-29 16:07:56]

> > These tests are for GYP_DEFINES="toolkit_views=1" builds, i.e.
> > Windows and CrOS.  I don't know if we have Linux bots with this
configuration.
> Nope we don't have. Should we? It will change unit/ui tests as well, right?
(note: we already have CrOS/Valgrind bots but without "toolkit_views=1"

[Ben Goodger (Google), 2011-06-29 16:12:48]

LGTM for the code changes.

http://codereview.chromium.org/7272044/diff/1/views/widget/native_widget_unit...
File views/widget/native_widget_unittest.cc (right):

http://codereview.chromium.org/7272044/diff/1/views/widget/native_widget_unit...
views/widget/native_widget_unittest.cc:22: // delegate.
remove " delegate"

[dhollowa, 2011-06-29 16:17:43]

http://codereview.chromium.org/7272044/diff/1/views/widget/native_widget_unit...
File views/widget/native_widget_unittest.cc (right):

http://codereview.chromium.org/7272044/diff/1/views/widget/native_widget_unit...
views/widget/native_widget_unittest.cc:22: // delegate.
On 2011/06/29 16:12:48, Ben Goodger (Google) wrote:
> remove " delegate"

Done.

[dhollowa, 2011-06-29 16:30:36]

On 2011/06/29 16:07:56, Timur Iskhodzhanov wrote:
> > > These tests are for GYP_DEFINES="toolkit_views=1" builds, i.e.
> > > Windows and CrOS.  I don't know if we have Linux bots with this
> configuration.
> > Nope we don't have. Should we? It will change unit/ui tests as well, right?
> (note: we already have CrOS/Valgrind bots but without "toolkit_views=1"

According to build/common.gypi:50 toolkit_views=1 for Windows and CrOS builds. 
I don't think a Linux/toolkit_views=1 build would give us extra coverage.

I will ping you on enabling the CrOS Valgrind "views" once that is ready. 
Thanks!