chromeos: Try to avoid LoginUtilsImpl delegate segfault.

chromeos: Try to avoid LoginUtilsImpl delegate segfault.

This makes LoginUtilsImpl::OnProfileCreated() check that its 
delegate is non-NULL before using it.  I'm hoping that this
will avoid a segfault that's apparently showing up during
testing when Chrome is killed during initialization.

Note that this is just a temporary workaround; the 
underlying code is possibly still incorrect (or relying on
incorrect assumptions about the order in which things will
happen).

BUG=chromium-os:18269
TEST=none
TBR=altimofeev@chromium.org

[Evan Martin, 2011-08-10 17:44:29]

I believe this may work around the bug, but is it Correct?

[Daniel Erat, 2011-08-10 18:14:07]

On 2011/08/10 17:44:29, Evan Martin wrote:
> I believe this may work around the bug, but is it Correct?

(as discussed in person)

I suspect that the existing code makes assumptions that are incorrect; this is
just an attempt to avoid a segfault.  I've added a TODO.

[Evan Martin, 2011-08-10 18:28:13]

LGTM

On Wed, Aug 10, 2011 at 11:14 AM,  <derat@chromium.org> wrote:
> On 2011/08/10 17:44:29, Evan Martin wrote:
>>
>> I believe this may work around the bug, but is it Correct?
>
> (as discussed in person)
>
> I suspect that the existing code makes assumptions that are incorrect; this
> is
> just an attempt to avoid a segfault.  I've added a TODO.
>
> http://codereview.chromium.org/7590026/
>

[altimofeev, 2011-08-10 20:12:42]

http://codereview.chromium.org/7590026/diff/3001/chrome/browser/chromeos/logi...
File chrome/browser/chromeos/login/login_utils.cc (right):

http://codereview.chromium.org/7590026/diff/3001/chrome/browser/chromeos/logi...
chrome/browser/chromeos/login/login_utils.cc:567: //
LoginUtilsImpl::PrepareProfile() has set |delegate_| when Chrome is killed
Actually, only LoginUtilsImpl:: PrepareProflie does invoke
CreateDefaultProfileAsync(this), which starts profile loading and makes
ProfileManager to callback in the end (note, it passes 'this' as
ProfileManager::Observer).  So |delegate_| should be initialized before
ProfileManager even gets to know about 'this' observer.

Why do you think, that Oshima's suggestion is wrong - singleton is deleted in
the main thread, but UI thread is alive and tries to use it?

[Daniel Erat, 2011-08-10 20:59:26]

On Wed, Aug 10, 2011 at 13:12,  <altimofeev@chromium.org> wrote:
>
>
http://codereview.chromium.org/7590026/diff/3001/chrome/browser/chromeos/logi...
> File chrome/browser/chromeos/login/login_utils.cc (right):
>
>
http://codereview.chromium.org/7590026/diff/3001/chrome/browser/chromeos/logi...
> chrome/browser/chromeos/login/login_utils.cc:567: //
> LoginUtilsImpl::PrepareProfile() has set |delegate_| when Chrome is
> killed
> Actually, only LoginUtilsImpl:: PrepareProflie does invoke
> CreateDefaultProfileAsync(this), which starts profile loading and makes
> ProfileManager to callback in the end (note, it passes 'this' as
> ProfileManager::Observer).  So |delegate_| should be initialized before
> ProfileManager even gets to know about 'this' observer.
>
> Why do you think, that Oshima's suggestion is wrong - singleton is
> deleted in the main thread, but UI thread is alive and tries to use it?

(replied at http://crosbug.com/18269)

> http://codereview.chromium.org/7590026/
>