Debug the generic session restore failure.

Debug the generic session restore failure.

Working theory: somebody (a separate chrome process) cycles the session restore
files when they shouldn't get cycled.

We cannot debug by adding logging, since the logs are destroyed when processes
exit and new ones are started.

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

[marja, 2012-08-20 10:55:57]

Hi sky,

Here's another attempt at debugging crbug.com/110324.

I'm suspecting it's some not-the-real-browser process which cycles the session
restore files and makes the session restore fail.

[sky, 2012-08-20 16:07:43]

LGTM

[marja, 2012-08-24 23:19:18]

Hi again, sky,

I had to change this CL quite a bit to make it work. Could you have another look
and check if this still LGTY?

The preferences are not yet necessarily loaded when the session service is
created, so, I can't write to the preferences and I changed this to use the
local state instead.

Also, I added spinning the message loop into SessionService unit tests, to make
sure the backend really gets destroyed (tests were failing on windows).

[sky, 2012-08-24 23:42:25]

LGTM

http://codereview.chromium.org/10854217/diff/18002/chrome/browser/sessions/se...
File chrome/browser/sessions/session_backend.cc (right):

http://codereview.chromium.org/10854217/diff/18002/chrome/browser/sessions/se...
chrome/browser/sessions/session_backend.cc:429: if (list->GetSize() > 10)
nit: while
Alkso, does this delete the value too if you end up removing?

http://codereview.chromium.org/10854217/diff/18002/chrome/browser/sessions/se...
chrome/browser/sessions/session_backend.cc:433: strftime(time_string, 32,
"%d.%m.%Y %H:%M:%S", localtime(&now));
I thought we had conversion routines some where, but maybe I'm wrong?

[marja, 2012-08-27 07:37:17]

Thanks for review (again)!

http://codereview.chromium.org/10854217/diff/18002/chrome/browser/sessions/se...
File chrome/browser/sessions/session_backend.cc (right):

http://codereview.chromium.org/10854217/diff/18002/chrome/browser/sessions/se...
chrome/browser/sessions/session_backend.cc:429: if (list->GetSize() > 10)
On 2012/08/24 23:42:25, sky wrote:
> nit: while
> Alkso, does this delete the value too if you end up removing?

Done. It deletes the value, the docs say: "If |out_value| is NULL, the removed
value will be deleted."

http://codereview.chromium.org/10854217/diff/18002/chrome/browser/sessions/se...
chrome/browser/sessions/session_backend.cc:433: strftime(time_string, 32,
"%d.%m.%Y %H:%M:%S", localtime(&now));
On 2012/08/24 23:42:25, sky wrote:
> I thought we had conversion routines some where, but maybe I'm wrong?

I was also a bit surprised that there is nothing in base/time.h. If we have
something, it's not very discoverable... :/

[commit-bot: I haz the power, 2012-08-27 07:37:32]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/marja@chromium.org/10854217/24001

[commit-bot: I haz the power, 2012-08-27 07:38:58]

Try job failure for 10854217-24001 on linux_chromeos for step "update".
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

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