Make TestingProfile auto-create incognito profiles.

Make TestingProfile auto-create incognito profiles.

Previously, tests that involved incognito profiles needed to manually
create them and add them to the parent profile before
GetOffTheRecordProfile was ever called.  However, the documentation for
Profile explicitly states that the incognito profile will be created if
it doesn't already exist when GetOffTheRecordProfile is called. Further,
it was prohibitvely invasive to test code that creates a profile and
expects to be able to do something with its associated incognito
profile.

This commit also updates existing tests to remove unnecessary manual
incognito profile creation.

BUG=318803
R=atwilson@chromium.org, dpolukhin@chromium.org, sky@chromium.org

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

[Andrew T Wilson (Slow), 2014-03-04 23:37:28]

LGTM if the tests all pass. I'm somewhat nervous about the
ProfileManagerUnittest change, so if you can get a review from someone familiar
with that specific test, that'd be good.

https://codereview.chromium.org/181403006/diff/1/chrome/browser/profiles/prof...
File chrome/browser/profiles/profile_manager_unittest.cc (left):

https://codereview.chromium.org/181403006/diff/1/chrome/browser/profiles/prof...
chrome/browser/profiles/profile_manager_unittest.cc:549:
ASSERT_TRUE(profile->GetOffTheRecordProfile());
I think we want to still include this line:

ASSERT_TRUE(profile->GetOffTheRecordProfile());

https://codereview.chromium.org/181403006/diff/1/chrome/test/base/testing_pro...
File chrome/test/base/testing_profile.cc (right):

https://codereview.chromium.org/181403006/diff/1/chrome/test/base/testing_pro...
chrome/test/base/testing_profile.cc:606:
incognito_profile_.reset(incognito_test_profile.release());
Call SetOffTheRecordProfile() here instead of setting incognito_profile_
directly.

[dconnelly1, 2014-03-05 19:09:43]

All the unit tests pass on both linux_chromeos and linux. I'll add a
knowledgeable reviewer, since it looks like setting the guest session state
should matter, but doesn't.

On Tue, Mar 4, 2014 at 3:37 PM, <atwilson@chromium.org> wrote:

> LGTM if the tests all pass. I'm somewhat nervous about the
> ProfileManagerUnittest change, so if you can get a review from someone
> familiar
> with that specific test, that'd be good.
>
>
> https://codereview.chromium.org/181403006/diff/1/chrome/
> browser/profiles/profile_manager_unittest.cc
> File chrome/browser/profiles/profile_manager_unittest.cc (left):
>
> https://codereview.chromium.org/181403006/diff/1/chrome/
> browser/profiles/profile_manager_unittest.cc#oldcode549
> chrome/browser/profiles/profile_manager_unittest.cc:549:
> ASSERT_TRUE(profile->GetOffTheRecordProfile());
> I think we want to still include this line:
>
> ASSERT_TRUE(profile->GetOffTheRecordProfile());
>

Done.


>
> https://codereview.chromium.org/181403006/diff/1/chrome/
> test/base/testing_profile.cc
> File chrome/test/base/testing_profile.cc (right):
>
> https://codereview.chromium.org/181403006/diff/1/chrome/
> test/base/testing_profile.cc#newcode606
> chrome/test/base/testing_profile.cc:606:
> incognito_profile_.reset(incognito_test_profile.release());
> Call SetOffTheRecordProfile() here instead of setting incognito_profile_
> directly.
>

Done.


>
> https://codereview.chromium.org/181403006/
>



-- 
Daniel Connelly | Software Engineer – Chrome | dconnelly@google.com |

Google Germany GmbH
Dienerstr. 12
80331 München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dconnelly, 2014-03-05 19:20:49]

dpolukhin: please look at profile_manager_unittest.cc. The existing
GuestProfileManager tests weren't making use of the fact that the child
incognito profiles had GuestSession set, which made this cleanup easier. Let me
know if you have a problem with that change.

sky: please look at testing_profile.

[Dmitry Polukhin, 2014-03-05 19:29:07]

LGTM

For this particular test I think it is fine to not check that the profile has
GuestSession set because it is useless to set this attribute in setup check it
in the test.

[sky, 2014-03-05 21:27:16]

LGTM

[dconnelly, 2014-03-05 21:44:22]

The CQ bit was checked by dconnelly@chromium.org

[commit-bot: I haz the power, 2014-03-05 21:47:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dconnelly@chromium.org/181403006/200001

[commit-bot: I haz the power, 2014-03-06 00:02:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dconnelly@chromium.org/181403006/200001

[commit-bot: I haz the power, 2014-03-06 00:49:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-06 00:49:25]

Try jobs failed on following builders: mac_chromium_rel

[dconnelly, 2014-03-06 00:56:48]

The CQ bit was checked by dconnelly@chromium.org

[commit-bot: I haz the power, 2014-03-06 00:58:37]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dconnelly@chromium.org/181403006/200001

[commit-bot: I haz the power, 2014-03-06 02:53:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dconnelly@chromium.org/181403006/200001

[commit-bot: I haz the power, 2014-03-06 03:18:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dconnelly@chromium.org/181403006/200001

[commit-bot: I haz the power, 2014-03-06 04:01:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-06 04:01:32]

Try jobs failed on following builders: mac_chromium_rel

[dconnelly, 2014-03-06 14:28:49]

The CQ bit was checked by dconnelly@chromium.org

[commit-bot: I haz the power, 2014-03-06 14:29:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dconnelly@chromium.org/181403006/200001

[commit-bot: I haz the power, 2014-03-06 17:14:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-06 17:14:28]

Retried try job too often on win_rel for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[dconnelly, 2014-03-06 17:15:41]

The CQ bit was checked by dconnelly@chromium.org

[commit-bot: I haz the power, 2014-03-06 17:16:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dconnelly@chromium.org/181403006/200001

[commit-bot: I haz the power, 2014-03-06 19:56:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-06 19:56:40]

Retried try job too often on win_rel for step(s) unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[dconnelly, 2014-03-06 21:33:05]

FYI: after talking to ananta@, I'm removing a pre-Aura Windows unit test that
was broken by this change.

[dconnelly, 2014-03-06 21:33:29]

The CQ bit was checked by dconnelly@chromium.org

[commit-bot: I haz the power, 2014-03-06 21:35:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dconnelly@chromium.org/181403006/220001

[commit-bot: I haz the power, 2014-03-06 22:10:48]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dconnelly@chromium.org/181403006/220001

[commit-bot: I haz the power, 2014-03-06 22:46:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-06 22:46:13]

Try jobs failed on following builders: mac_chromium_rel

[dconnelly1, 2014-03-06 23:13:02]

oh my god, how many times can gclient revert fail?


On Thu, Mar 6, 2014 at 2:46 PM, <commit-bot@chromium.org> wrote:

> Try jobs failed on following builders: mac_chromium_rel
>
> https://codereview.chromium.org/181403006/
>



-- 
Daniel Connelly | Software Engineer – Chrome | dconnelly@google.com |

Google Germany GmbH
Dienerstr. 12
80331 München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[dconnelly, 2014-03-06 23:13:19]

The CQ bit was checked by dconnelly@chromium.org

[commit-bot: I haz the power, 2014-03-06 23:21:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dconnelly@chromium.org/181403006/220001

[commit-bot: I haz the power, 2014-03-07 06:04:28]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-07 06:04:29]

Retried try job too often on mac_rel for step(s) telemetry_unittests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[dconnelly, 2014-03-07 17:16:33]

Committed patchset #4 manually as r255647 (presubmit successful).