Adds some debugging code for tracking down a crash

Adds some debugging code for tracking down a crash

Crash seems to indicate we're registering twice for a
notification. Perhaps we're calling ScheduleLoad and TabIsLoading for
the same tab. Only way I could see that happening is if ShowBrowser()
activated some other tab. This should help isolate if that is
happening.

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

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

[marja, 2014-05-23 08:30:43]

LGTM


I was wondering if this is related to pinned tabs, but.. meh, dunno, looks
weird. It looks like RestoreTabsToBrowser calls TabIsLoading or ScheduleLoad (by
RestoreTab) exactly once for each tab. o.O

https://codereview.chromium.org/299053002/diff/1/chrome/browser/sessions/sess...
File chrome/browser/sessions/session_restore.cc (right):

https://codereview.chromium.org/299053002/diff/1/chrome/browser/sessions/sess...
chrome/browser/sessions/session_restore.cc:492: tabs_to_load_.end();
Style nit, I find the shorter style more readable:

find(tabs_to_load_.begin(), tabs_to_load_.end(), controller)

-> tabs_to_load_.find(controller)

https://codereview.chromium.org/299053002/diff/1/chrome/browser/sessions/sess...
chrome/browser/sessions/session_restore.cc:506:
base::debug::Alias(&in_tabs_to_load);
Wouldn't it be simpler to CHECK(!foo) for all the conditions separately (from
least suspected to most suspected), so you'd see directly from the stack trace
which one fails?

(This comment is not important; feel free to ignore.)

[sky, 2014-05-23 15:38:41]

https://codereview.chromium.org/299053002/diff/1/chrome/browser/sessions/sess...
File chrome/browser/sessions/session_restore.cc (right):

https://codereview.chromium.org/299053002/diff/1/chrome/browser/sessions/sess...
chrome/browser/sessions/session_restore.cc:492: tabs_to_load_.end();
On 2014/05/23 08:30:43, marja wrote:
> Style nit, I find the shorter style more readable:
> 
> find(tabs_to_load_.begin(), tabs_to_load_.end(), controller)
> 
> -> tabs_to_load_.find(controller)

I agree with you, but find isn't on list, right?

https://codereview.chromium.org/299053002/diff/1/chrome/browser/sessions/sess...
chrome/browser/sessions/session_restore.cc:506:
base::debug::Alias(&in_tabs_to_load);
On 2014/05/23 08:30:43, marja wrote:
> Wouldn't it be simpler to CHECK(!foo) for all the conditions separately (from
> least suspected to most suspected), so you'd see directly from the stack trace
> which one fails?
> 
> (This comment is not important; feel free to ignore.)

I was tempted to do as you suggest, but that would mean I wouldn't know if more
than one thing is true. This is why I went the alias route.

[sky, 2014-05-23 15:38:44]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2014-05-23 15:39:13]

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

[commit-bot: I haz the power, 2014-05-23 18:52:01]

FYI, CQ is re-trying this CL (attempt #1).
Please consider checking whether the failures are real,
and report flakes to chrome-troopers@google.com.
The failing builders are:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[commit-bot: I haz the power, 2014-05-23 21:09:23]

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

[commit-bot: I haz the power, 2014-05-23 21:09:23]

Try jobs failed on following builders:
  mac_chromium_rel on tryserver.chromium
(http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...)

[sky, 2014-05-23 21:14:21]

The CQ bit was checked by sky@chromium.org

[commit-bot: I haz the power, 2014-05-23 21:16:02]

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

[commit-bot: I haz the power, 2014-05-24 12:29:11]

Change committed as 272716

[marja, 2014-05-27 07:54:55]

https://codereview.chromium.org/299053002/diff/1/chrome/browser/sessions/sess...
File chrome/browser/sessions/session_restore.cc (right):

https://codereview.chromium.org/299053002/diff/1/chrome/browser/sessions/sess...
chrome/browser/sessions/session_restore.cc:492: tabs_to_load_.end();
Ahh, I see, I missed that it's a list.