Valgrind: Fix a leak in OneClickSigninHelperTest.

Valgrind: Fix a leak in OneClickSigninHelperTest.

Fix is to run the current MessageLoop in test TearDown to cleanup DeleteSoon()s.

BUG=130246
TEST=OneClickSigninHelperTest.*
R=groby
TBR=atwilson

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=139968

[groby-ooo-7-16, 2012-05-31 22:33:07]

lgtm

[Roger Tawa OOO till Jul 10th, 2012-06-01 13:56:42]

Hi James,

I already had a change out for review to fix this leak, see
http://codereview.chromium.org/10443113/.  Note that you did not remove the
suppression from tools/heapcheck/suppressions.txt, see the original suppression
CL crrev.com/139500 for details.  Please remove it from there too.

Also see comments below.

https://chromiumcodereview.appspot.com/10444125/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/api/identity/web_auth_flow_unittest.cc (left):

https://chromiumcodereview.appspot.com/10444125/diff/1/chrome/browser/extensi...
chrome/browser/extensions/api/identity/web_auth_flow_unittest.cc:73: return
web_contents_;
Why is this change needed to fix the memory leak?

https://chromiumcodereview.appspot.com/10444125/diff/1/chrome/browser/ui/sync...
File chrome/browser/ui/sync/one_click_signin_helper_unittest.cc (right):

https://chromiumcodereview.appspot.com/10444125/diff/1/chrome/browser/ui/sync...
chrome/browser/ui/sync/one_click_signin_helper_unittest.cc:70:
MessageLoop::current()->RunAllPending();
Are you sure this should/can be done after destroying the browser context (i.e.
test profile) and ui thread?

[James Hawkins, 2012-06-01 23:53:13]

https://chromiumcodereview.appspot.com/10444125/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/api/identity/web_auth_flow_unittest.cc (left):

https://chromiumcodereview.appspot.com/10444125/diff/1/chrome/browser/extensi...
chrome/browser/extensions/api/identity/web_auth_flow_unittest.cc:73: return
web_contents_;
On 2012/06/01 13:56:42, Roger Tawa wrote:
> Why is this change needed to fix the memory leak?

Didn't say it was.  It's drive-by cleanup I saw while fixing this issue.

https://chromiumcodereview.appspot.com/10444125/diff/1/chrome/browser/ui/sync...
File chrome/browser/ui/sync/one_click_signin_helper_unittest.cc (right):

https://chromiumcodereview.appspot.com/10444125/diff/1/chrome/browser/ui/sync...
chrome/browser/ui/sync/one_click_signin_helper_unittest.cc:70:
MessageLoop::current()->RunAllPending();
On 2012/06/01 13:56:42, Roger Tawa wrote:
> Are you sure this should/can be done after destroying the browser context
(i.e.
> test profile) and ui thread?

In what context are you asking this question?  What are your concerns?

[Roger Tawa OOO till Jul 10th, 2012-06-04 14:50:40]

https://chromiumcodereview.appspot.com/10444125/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/api/identity/web_auth_flow_unittest.cc (left):

https://chromiumcodereview.appspot.com/10444125/diff/1/chrome/browser/extensi...
chrome/browser/extensions/api/identity/web_auth_flow_unittest.cc:73: return
web_contents_;
On 2012/06/01 23:53:13, James Hawkins wrote:
> On 2012/06/01 13:56:42, Roger Tawa wrote:
> > Why is this change needed to fix the memory leak?
> 
> Didn't say it was.  It's drive-by cleanup I saw while fixing this issue.

OK, the CL description did not mention it included other drive by cleanup, so it
was not clear it was that.  Maybe update the CL description?

https://chromiumcodereview.appspot.com/10444125/diff/1/chrome/browser/ui/sync...
File chrome/browser/ui/sync/one_click_signin_helper_unittest.cc (right):

https://chromiumcodereview.appspot.com/10444125/diff/1/chrome/browser/ui/sync...
chrome/browser/ui/sync/one_click_signin_helper_unittest.cc:70:
MessageLoop::current()->RunAllPending();
On 2012/06/01 23:53:13, James Hawkins wrote:
> On 2012/06/01 13:56:42, Roger Tawa wrote:
> > Are you sure this should/can be done after destroying the browser context
> (i.e.
> > test profile) and ui thread?
> 
> In what context are you asking this question?  What are your concerns?

My concern is calling RunAllPending() after the destruction of the
content::TestBrowserThread object. Also, there may be tasks to run for the
browsing context.  Other tests make sure not to do this, see TearDown() in
src/content/test/test_renderer_host.cc for example.