Fix memory leak in OneClickSigninHelperTest.

Fix memory leak in OneClickSigninHelperTest.
Also fixed CanOfferNoSigninCookies() to not use an incognito profile.

BUG=130246
TEST=Make sure all OneClickSigninHelperTest tests do not leak a RPH.


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

[Roger Tawa OOO till Jul 10th, 2012-05-31 20:20:47]

Hi Drew, Derek,

Please take a look.  Thanks.

[Derek Bruening, 2012-06-01 15:04:24]

LGTM though I'm not familiar w/ this code

http://codereview.chromium.org/10443113/diff/1002/chrome/browser/ui/sync/one_...
File chrome/browser/ui/sync/one_click_signin_helper_unittest.cc (right):

http://codereview.chromium.org/10443113/diff/1002/chrome/browser/ui/sync/one_...
chrome/browser/ui/sync/one_click_signin_helper_unittest.cc:70: // Flush messages
related web contents destruction before destroying the
nit: grammar

[Roger Tawa OOO till Jul 10th, 2012-06-01 15:29:45]

Thank Derek.  I added you because you are an owner in tools\valgrind.  Maybe
that was not needed?  I'll wait for Drew to review the sign in test code.

Note that James also tried to fix this memory leak in CL
https://chromiumcodereview.appspot.com/10444125/, so I merged my changes with
those.  However I am not sure James' change is correct or complete, so I may
commit this anyways.

[Andrew T Wilson (Slow), 2012-06-01 16:22:10]

LGTM

[Andrew T Wilson (Slow), 2012-06-01 16:23:44]

On 2012/06/01 16:22:10, Andrew T Wilson wrote:
> LGTM

Note that the build failures are because the patch you're sending to the trybots
is turning on client oauth (I'm guessing the parent branch for this change is
not trunk, but rather is some other branch so "git try" is picking up the other
branch's changes too).

[Roger Tawa OOO till Jul 10th, 2012-06-01 18:04:30]

Correct Drew.  I will send a try job with only these changes and see.  Thanks
for pointing it out!

[Roger Tawa OOO till Jul 10th, 2012-06-05 20:13:23]

Hi James,

I fixed up the test to make better use of the new base class you added, although
it only needs the base class of that one.  Many less member variables and
methods are needed.  I also cleaned up the include headers which are not needed.
 Please take a look.  Thanks.

[James Hawkins, 2012-06-05 21:14:23]

http://codereview.chromium.org/10443113/diff/31005/chrome/browser/ui/sync/one...
File chrome/browser/ui/sync/one_click_signin_helper_unittest.cc (right):

http://codereview.chromium.org/10443113/diff/31005/chrome/browser/ui/sync/one...
chrome/browser/ui/sync/one_click_signin_helper_unittest.cc:97:
MarkCurrentThreadAsUIThread();
This method is now called in every test.  The method should be removed, and the
functionality of the method should be moved to SetUp().

[Roger Tawa OOO till Jul 10th, 2012-06-06 15:40:26]

Hi James, please take another look.

http://codereview.chromium.org/10443113/diff/31005/chrome/browser/ui/sync/one...
File chrome/browser/ui/sync/one_click_signin_helper_unittest.cc (right):

http://codereview.chromium.org/10443113/diff/31005/chrome/browser/ui/sync/one...
chrome/browser/ui/sync/one_click_signin_helper_unittest.cc:97:
MarkCurrentThreadAsUIThread();
On 2012/06/05 21:14:23, James Hawkins wrote:
> This method is now called in every test.  The method should be removed, and
the
> functionality of the method should be moved to SetUp().

Done.

[James Hawkins, 2012-06-06 16:36:37]

LGTM with nit.

http://codereview.chromium.org/10443113/diff/35006/chrome/browser/ui/sync/one...
File chrome/browser/ui/sync/one_click_signin_helper_unittest.cc (right):

http://codereview.chromium.org/10443113/diff/35006/chrome/browser/ui/sync/one...
chrome/browser/ui/sync/one_click_signin_helper_unittest.cc:54:
ui_thread_.reset(new content::TestBrowserThread(
nit: I'm sorry I didn't suggest this before, but now that we don't explicitly
tear this down anymore, we can get rid of the use of scoped_ptr and just
initialize |ui_thread_| in the constructor.

[Roger Tawa OOO till Jul 10th, 2012-06-06 17:05:02]

Thanks James.  Comments addressed, changes uploaded. Will CQ shortly.

http://codereview.chromium.org/10443113/diff/35006/chrome/browser/ui/sync/one...
File chrome/browser/ui/sync/one_click_signin_helper_unittest.cc (right):

http://codereview.chromium.org/10443113/diff/35006/chrome/browser/ui/sync/one...
chrome/browser/ui/sync/one_click_signin_helper_unittest.cc:54:
ui_thread_.reset(new content::TestBrowserThread(
On 2012/06/06 16:36:37, James Hawkins wrote:
> nit: I'm sorry I didn't suggest this before, but now that we don't explicitly
> tear this down anymore, we can get rid of the use of scoped_ptr and just
> initialize |ui_thread_| in the constructor.

Done.

[commit-bot: I haz the power, 2012-06-06 17:05:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rogerta@chromium.org/10443113/38003

[commit-bot: I haz the power, 2012-06-06 18:28:04]

Change committed as 140798