Adds some debugging code for tracking down a crash

chrome/browser/sessions/session_restore.cc

Index: chrome/browser/sessions/session_restore.cc
diff --git a/chrome/browser/sessions/session_restore.cc b/chrome/browser/sessions/session_restore.cc
index 7cca291c9b3881de619289ab8c46ae5f6096b0a1..4baafa7ce0cad363c6df83edbcfddd722bd24042 100644
--- a/chrome/browser/sessions/session_restore.cc
+++ b/chrome/browser/sessions/session_restore.cc
@@ -13,6 +13,7 @@
 #include "base/bind_helpers.h"
 #include "base/callback.h"
 #include "base/command_line.h"
+#include "base/debug/alias.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/scoped_vector.h"
 #include "base/metrics/histogram.h"
@@ -152,6 +153,9 @@ class TabLoader : public content::NotificationObserver,
   // Called when a tab goes away or a load completes.
   void HandleTabClosedOrLoaded(NavigationController* controller);
 
+  // TODO(sky): remove. For debugging 368236.
+  void CheckNotObserving(NavigationController* controller);
+
   content::NotificationRegistrar registrar_;
 
   // Current delay before a new tab is loaded. See class description for
@@ -203,6 +207,7 @@ TabLoader* TabLoader::GetTabLoader(base::TimeTicks restore_started) {
 }
 
 void TabLoader::ScheduleLoad(NavigationController* controller) {
+  CheckNotObserving(controller);
   DCHECK(controller);
   DCHECK(find(tabs_to_load_.begin(), tabs_to_load_.end(), controller) ==
          tabs_to_load_.end());
@@ -211,6 +216,7 @@ void TabLoader::ScheduleLoad(NavigationController* controller) {
 }
 
 void TabLoader::TabIsLoading(NavigationController* controller) {
+  CheckNotObserving(controller);
   DCHECK(controller);
   DCHECK(find(tabs_loading_.begin(), tabs_loading_.end(), controller) ==
          tabs_loading_.end());
@@ -480,6 +486,29 @@ void TabLoader::HandleTabClosedOrLoaded(NavigationController* tab) {
   }
 }
 
+void TabLoader::CheckNotObserving(NavigationController* controller) {
+  const bool in_tabs_to_load =
+      find(tabs_to_load_.begin(), tabs_to_load_.end(), controller) !=
+          tabs_to_load_.end();

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

Style nit, I find the shorter style more readable:

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

-> tabs_to_load_.find(controller)

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

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

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

Ahh, I see, I missed that it's a list.

+  const bool in_tabs_loading =
+      find(tabs_loading_.begin(), tabs_loading_.end(), controller) !=
+          tabs_loading_.end();
+  const bool observing =
+      registrar_.IsRegistered(
+          this, content::NOTIFICATION_WEB_CONTENTS_DESTROYED,
+          content::Source<WebContents>(controller->GetWebContents())) ||
+      registrar_.IsRegistered(
+          this, content::NOTIFICATION_LOAD_STOP,
+          content::Source<NavigationController>(controller)) ||
+      registrar_.IsRegistered(
+          this, content::NOTIFICATION_LOAD_START,
+          content::Source<NavigationController>(controller));
+  base::debug::Alias(&in_tabs_to_load);

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

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]

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.

+  base::debug::Alias(&in_tabs_loading);
+  base::debug::Alias(&observing);
+  CHECK(!in_tabs_to_load && !in_tabs_loading && !observing);
+}
+
 // SessionRestoreImpl ---------------------------------------------------------
 
 // SessionRestoreImpl is responsible for fetching the set of tabs to create
@@ -960,6 +989,9 @@ class SessionRestoreImpl : public content::NotificationObserver {
         ShowBrowser(
             browser,
             browser->tab_strip_model()->GetIndexOfWebContents(restored_tab));
+        // TODO(sky): remove. For debugging 368236.
+        CHECK_EQ(browser->tab_strip_model()->GetActiveWebContents(),
+                 restored_tab);
         tab_loader_->TabIsLoading(&browser->tab_strip_model()
                                        ->GetActiveWebContents()
                                        ->GetController());