Adds NULL check in NotifySessionServiceOfRestoredTabs for

Adds NULL check in NotifySessionServiceOfRestoredTabs for
SessionService. This comes from crash data. Only way I could see this
happening is if between the time we request the data from the backend
and the time we get it the service is destroyed.

BUG=95125
TEST=none
R=marja@chromium.org

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

[marja, 2012-04-30 18:20:27]

lgtm

http://codereview.chromium.org/10265010/diff/1/chrome/browser/sessions/sessio...
File chrome/browser/sessions/session_restore.cc (right):

http://codereview.chromium.org/10265010/diff/1/chrome/browser/sessions/sessio...
chrome/browser/sessions/session_restore.cc:956: if (!session_service)
Hmm, SessionServiceFactory calls
ProfileKeyedServiceFactory::GetServiceForProfile with create = true. It can
return NULL if GetProfileForUse returns NULL or
SessionServiceFactory::BuildServiceInstanceFor returns NULL (that cannot
happen).

GetProfileToUse can return NULL if the profile is incognito &&
ServiceRedirectedInIncognito() == false && ServiceHasOwnInstanceInIncognito() ==
false, and those are the defaults (code search doesn't work so much that I could
check if SessionServiceFactory overrides them and I'm sitting in a park :) So,
this can happen if this is called for incognito profiles..

How could it happen in the "service already destroyed" case since create = true?

[sky, 2012-04-30 20:21:07]

On Mon, Apr 30, 2012 at 11:20 AM,  <marja@chromium.org> wrote:
> lgtm
>
>
>
>
http://codereview.chromium.org/10265010/diff/1/chrome/browser/sessions/sessio...
> File chrome/browser/sessions/session_restore.cc (right):
>
>
http://codereview.chromium.org/10265010/diff/1/chrome/browser/sessions/sessio...
> chrome/browser/sessions/session_restore.cc:956: if (!session_service)
> Hmm, SessionServiceFactory calls
> ProfileKeyedServiceFactory::GetServiceForProfile with create = true. It
> can return NULL if GetProfileForUse returns NULL or
> SessionServiceFactory::BuildServiceInstanceFor returns NULL (that cannot
> happen).
>
> GetProfileToUse can return NULL if the profile is incognito &&
> ServiceRedirectedInIncognito() == false &&
> ServiceHasOwnInstanceInIncognito() == false, and those are the defaults
> (code search doesn't work so much that I could check if
> SessionServiceFactory overrides them and I'm sitting in a park :) So,
> this can happen if this is called for incognito profiles..

SessionRestore::RestoreSession returns if there is no SessionService,
but it looks like the Foreign session code doesn't. I want to say this
crash predates the foreign session code, but I'm not positive.

> How could it happen in the "service already destroyed" case since create
> = true?

I was thinking of the case where SessionRestore requests the sessions
and before it gets a response the sessionservice is destroyed. I'm
actually not sure what would happen in that case. It may be
SessionRestore never sees the competed task.

Either way, I'll land this since it looks like the foreign session
code could trigger this getting invoked.

  -Scott

[commit-bot: I haz the power, 2012-04-30 20:21:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sky@chromium.org/10265010/1

[commit-bot: I haz the power, 2012-04-30 20:32:19]

Try job failure for 10265010-1 on win_rel for step "update".
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

Step "update" is always a major failure.
Look at the try server FAQ for more details.