Create the testing profile using the builder.

Create the testing profile using the TestingProfile::Builder.

This could fix the problem that some services do no shutdown properly in the test.

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

[Adrian Kuegel, 2013-03-07 12:53:44]

This changelist tries to fix the issue with the
IncognitoModeOnSigninAllowedPrefChange unit test on DrMemory bot. I was not able
to test it, since I have no windows machine, but I learned that I should use the
profile builder if I want to have an incognito profile, otherwise some services
might not shutdown properly (which seems to be the case looking at the kind of
error). So at least this change shouldn't make it worse.

[dhollowa, 2013-03-07 17:52:17]

To test it properly you'll need to remove the exclusion in
https://codereview.chromium.org/12377050 and then watch the bots to make sure it
got fixed.  Best to alert the memory sheriff what you're doing too.

Please add the revert of the exclusion to this CL, that way they'll land
together.

[Adrian Kuegel, 2013-03-08 09:07:34]

Ok, I added the revert to this changelist. Is it ok if I alert the memory
sheriff just before I will commit this changelist? I will need OWNERS approval
first, anyway.

[Adrian Kuegel, 2013-03-08 09:08:10]

Scott, can you please review this changelist and give me OWNERS approval?

[dhollowa, 2013-03-08 15:57:56]

LGTM.

[sky, 2013-03-08 16:08:05]

https://codereview.chromium.org/12610003/diff/4001/chrome/browser/ui/browser_...
File chrome/browser/ui/browser_command_controller_unittest.cc (right):

https://codereview.chromium.org/12610003/diff/4001/chrome/browser/ui/browser_...
chrome/browser/ui/browser_command_controller_unittest.cc:332: TestingProfile*
profile2 = builder.Build().release();
I'm not familiar with TestingPRofile::Builder, who owns profile2?

[Adrian Kuegel, 2013-03-08 16:28:54]

https://codereview.chromium.org/12610003/diff/4001/chrome/browser/ui/browser_...
File chrome/browser/ui/browser_command_controller_unittest.cc (right):

https://codereview.chromium.org/12610003/diff/4001/chrome/browser/ui/browser_...
chrome/browser/ui/browser_command_controller_unittest.cc:332: TestingProfile*
profile2 = builder.Build().release();
Build() returns a scoped_ptr. So I assume with this line I will have ownership
of profile2.
I used
chrome/browser/content_settings/content_settings_pref_provider_unittest.cc as
example (see line 156 there).

[sky, 2013-03-08 16:52:18]

Then where do you delete profile2?


On Fri, Mar 8, 2013 at 8:28 AM, <akuegel@chromium.org> wrote:

>
> https://codereview.chromium.**org/12610003/diff/4001/chrome/**
>
browser/ui/browser_command_**controller_unittest.cc<https://codereview.chromium.org/12610003/diff/4001/chrome/browser/ui/browser_command_controller_unittest.cc>
> File chrome/browser/ui/browser_**command_controller_unittest.cc (right):
>
> https://codereview.chromium.**org/12610003/diff/4001/chrome/**
>
browser/ui/browser_command_**controller_unittest.cc#**newcode332<https://codereview.chromium.org/12610003/diff/4001/chrome/browser/ui/browser_command_controller_unittest.cc#newcode332>
> chrome/browser/ui/browser_**command_controller_unittest.**cc:332:
> TestingProfile* profile2 = builder.Build().release();
> Build() returns a scoped_ptr. So I assume with this line I will have
> ownership of profile2.
> I used
> chrome/browser/content_**settings/content_settings_**
> pref_provider_unittest.cc
> as example (see line 156 there).
>
>
https://codereview.chromium.**org/12610003/<https://codereview.chromium.org/1...
>

[Adrian Kuegel, 2013-03-08 16:56:37]

Since profile2 is a off-the-record profile of profile1, this should be taken
care of by profile1. I am not an expert there, so I may be wrong, but at least
in the other test I used as an example of how to create a off-the-record
profile, the off-the-record profile is also not deleted directly (and when I try
to delete it directly, it crashes).

On 2013/03/08 16:52:18, sky wrote:
> Then where do you delete profile2?
> 
> 
> On Fri, Mar 8, 2013 at 8:28 AM, <mailto:akuegel@chromium.org> wrote:
> 
> >
> > https://codereview.chromium.**org/12610003/diff/4001/chrome/**
> >
>
browser/ui/browser_command_**controller_unittest.cc<https://codereview.chromium.org/12610003/diff/4001/chrome/browser/ui/browser_command_controller_unittest.cc>
> > File chrome/browser/ui/browser_**command_controller_unittest.cc (right):
> >
> > https://codereview.chromium.**org/12610003/diff/4001/chrome/**
> >
>
browser/ui/browser_command_**controller_unittest.cc#**newcode332<https://codereview.chromium.org/12610003/diff/4001/chrome/browser/ui/browser_command_controller_unittest.cc#newcode332>
> > chrome/browser/ui/browser_**command_controller_unittest.**cc:332:
> > TestingProfile* profile2 = builder.Build().release();
> > Build() returns a scoped_ptr. So I assume with this line I will have
> > ownership of profile2.
> > I used
> > chrome/browser/content_**settings/content_settings_**
> > pref_provider_unittest.cc
> > as example (see line 156 there).
> >
> >
>
https://codereview.chromium.**org/12610003/%3Chttps://codereview.chromium.org...>
> >

[sky, 2013-03-08 18:28:43]

Indeed it does, LGTM

[Adrian Kuegel, 2013-03-11 08:52:17]

@thestig: Just wanted to notify you that I am about to commit this changelist
which hopefully fixes the problem with the unit test
IncognitoModeOnSigninAllowedPrefChange and removes the test suppression. If the
memory bot still reports an error, please add the suppression again.

[commit-bot: I haz the power, 2013-03-11 08:53:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/akuegel@chromium.org/12610003/4001

[commit-bot: I haz the power, 2013-03-11 11:17:04]

Change committed as 187272