Only return fully initialized profiles in ProfileManager::GetProfileByPath.

Only return fully initialized profiles in ProfileManager::GetProfileByPath.

When a profile was loaded asynchronously, ProfileManager::GetProfileByPath could return it before it had been fully initialized, which could lead to... interesting problems. We now return null if that is the case, because callers of GetProfileByPath need to deal with a null return value anyway.

ProfileManager::GetProfile (which is supposed to create the profile synchronously if it doesn't exist) will now DCHECK when trying to get a profile that is not fully initialized.

This CL also stops AppController from trying to get a profile that has just been deleted, which would now crash otherwise.

BUG=425785
Committed: https://crrev.com/773470007a411cff16492cae579b91649d5ee8b5
Cr-Commit-Position: refs/heads/master@{#317298}

[Bernhard Bauer, 2015-02-16 16:58:06]

The CQ bit was unchecked by bauerb@chromium.org

[Bernhard Bauer, 2015-02-16 16:58:06]

The CQ bit was checked by bauerb@chromium.org

[Bernhard Bauer, 2015-02-17 12:00:00]

bauerb@chromium.org changed reviewers:
+ noms@chromium.org, rsesek@chromium.org

[Bernhard Bauer, 2015-02-17 12:00:16]

Please review. Thanks!

[Bernhard Bauer, 2015-02-17 18:01:07]

On 2015/02/17 12:00:16, Bernhard Bauer wrote:
> Please review. Thanks!

(And by that, I meant Robert: Mac parts, Monica: profiles)

[Robert Sesek, 2015-02-17 19:19:05]

Mac bits LGTM

[Bernhard Bauer, 2015-02-18 21:00:04]

Monica, ping? I'm willing to bribe you with cute pictures (e.g.
http://i.imgur.com/MdNdNEq.jpg).

[noms (inactive), 2015-02-18 21:26:21]

https://codereview.chromium.org/929073002/diff/120001/chrome/browser/profiles...
File chrome/browser/profiles/profile_manager.cc (right):

https://codereview.chromium.org/929073002/diff/120001/chrome/browser/profiles...
chrome/browser/profiles/profile_manager.cc:387:
DCHECK(!GetProfileByPathInternal(profile_dir));
Hmm, so in non-debug world, if the profile has not been founded but we *have*
started loading it (and it's not fully initialized), don't we then, on line 389,
create it again?

[Bernhard Bauer, 2015-02-18 22:51:31]

On Wed, Feb 18, 2015, 21:26 null <noms@chromium.org> wrote:


https://codereview.chromium.org/929073002/diff/120001/chrome/browser/profiles...
File chrome/browser/profiles/profile_manager.cc (right):

https://codereview.chromium.org/929073002/diff/120001/chrome/browser/profiles...
<https://codereview.chromium.org/929073002/diff/120001/chrome/browser/profiles...>
chrome/browser/profiles/profile_manager.cc:387
<https://codereview.chromium.org/929073002/diff/120001/chrome/browser/profiles...>
:


DCHECK(!GetProfileByPathInternal(profile_dir));
Hmm, so in non-debug world, if the profile has not been founded but we
*have* started loading it (and it's not fully initialized), don't we
then, on line 389, create it again?

 https://codereview.chromium.org/929073002/


 Yeah... Should I

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

[Bernhard Bauer, 2015-02-18 22:54:31]

Gah, Inbox hates me. What I wanted to say was:

Should I make it a CHECK? We could probably justify that, to prevent
profile corruption from opening the same profile twice.

On Wed, Feb 18, 2015, 22:51 Bernhard Bauer <bauerb@chromium.org> wrote:

> On Wed, Feb 18, 2015, 21:26 null <noms@chromium.org> wrote:
>
>
>
>
https://codereview.chromium.org/929073002/diff/120001/chrome/browser/profiles...
> File chrome/browser/profiles/profile_manager.cc (right):
>
>
>
https://codereview.chromium.org/929073002/diff/120001/chrome/browser/profiles...
>
<https://codereview.chromium.org/929073002/diff/120001/chrome/browser/profiles...>
> chrome/browser/profiles/profile_manager.cc:387
>
<https://codereview.chromium.org/929073002/diff/120001/chrome/browser/profiles...>
> :
>
>
> DCHECK(!GetProfileByPathInternal(profile_dir));
> Hmm, so in non-debug world, if the profile has not been founded but we
> *have* started loading it (and it's not fully initialized), don't we
> then, on line 389, create it again?
>
>  https://codereview.chromium.org/929073002/
>
>
>  Yeah... Should I
>

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

[Bernhard Bauer, 2015-02-19 10:16:44]

New patchsets have been uploaded after l-g-t-m from rsesek@chromium.org

[Bernhard Bauer, 2015-02-19 10:22:37]

https://codereview.chromium.org/929073002/diff/120001/chrome/browser/profiles...
File chrome/browser/profiles/profile_manager.cc (right):

https://codereview.chromium.org/929073002/diff/120001/chrome/browser/profiles...
chrome/browser/profiles/profile_manager.cc:387:
DCHECK(!GetProfileByPathInternal(profile_dir));
On 2015/02/18 21:26:21, Monica Dinculescu wrote:
> Hmm, so in non-debug world, if the profile has not been founded but we *have*
> started loading it (and it's not fully initialized), don't we then, on line
389,
> create it again?

Made it a CHECK.

[noms (inactive), 2015-02-19 23:42:00]

ok. i think this makes sense. profiles lgtm!

[Bernhard Bauer, 2015-02-20 08:49:12]

The CQ bit was checked by bauerb@chromium.org

[commit-bot: I haz the power, 2015-02-20 08:49:32]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/929073002/140001

[commit-bot: I haz the power, 2015-02-20 09:07:09]

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

[commit-bot: I haz the power, 2015-02-20 09:07:10]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Bernhard Bauer, 2015-02-20 09:34:28]

New patchsets have been uploaded after l-g-t-m from noms@chromium.org

[Bernhard Bauer, 2015-02-20 11:18:22]

The CQ bit was checked by bauerb@chromium.org

[commit-bot: I haz the power, 2015-02-20 11:19:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/929073002/180001

[commit-bot: I haz the power, 2015-02-20 12:20:29]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-02-20 12:22:01]

Patchset 10 (id:??) landed as
https://crrev.com/773470007a411cff16492cae579b91649d5ee8b5
Cr-Commit-Position: refs/heads/master@{#317298}

[Pam (message me for reviews), 2015-02-23 12:02:01]

pam@chromium.org changed reviewers:
+ pam@chromium.org

[Pam (message me for reviews), 2015-02-23 12:02:02]

LGTM, one optional nit

https://codereview.chromium.org/929073002/diff/180001/chrome/browser/profiles...
File chrome/browser/profiles/profile_manager.cc (right):

https://codereview.chromium.org/929073002/diff/180001/chrome/browser/profiles...
chrome/browser/profiles/profile_manager.cc:565: return profile_info &&
profile_info->created ? profile_info->profile.get()
nit: This would be slightly clearer with the condition parenthesized.

[Bernhard Bauer, 2015-02-23 18:42:14]

https://codereview.chromium.org/929073002/diff/180001/chrome/browser/profiles...
File chrome/browser/profiles/profile_manager.cc (right):

https://codereview.chromium.org/929073002/diff/180001/chrome/browser/profiles...
chrome/browser/profiles/profile_manager.cc:565: return profile_info &&
profile_info->created ? profile_info->profile.get()
On 2015/02/23 12:02:02, Pam (also PM for reviews) wrote:
> nit: This would be slightly clearer with the condition parenthesized.

Done (in https://codereview.chromium.org/953453002).