Adds more debugging code in hopes of figuring out why on dragging the

tab strip model is empty, but not the tab strip. The latest crash data
indicates the tab strip is not closing all.

BUG=24132
TEST=none

Review URL: http://codereview.chromium.org/375017

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@31450 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 143 insertions, 27 deletions

diff --git a/chrome/browser/state_tracker.cc b/chrome/browser/state_tracker.cc
new file mode 100644
index 0000000..03af631
--- /dev/null
+++ b/chrome/browser/state_tracker.cc
@@ -0,0 +1,35 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/state_tracker.h"
+
+// Number of characters the buffer contains.
+static const size_t kSize = 256;
+
+StateTracker::StateTracker()
+  : index_(0),
+    content_(new char[kSize]) {
+  for (size_t i = 0; i < kSize; ++i)
+    content_[i] = '\0';
+}
+
+void StateTracker::Append(const std::string& text) {
+  if (text.size() >= kSize) {
+    NOTREACHED();
+    return;
+  }
+  for (size_t i = 0; i < text.size(); ++i) {
+    content_[index_] = text[i];
+    index_ = (index_ + 1) % kSize;
+  }
+  content_[index_] = '!';
+}
+
+void StateTracker::Crash() {
+  volatile char state[kSize];
+  for (size_t i = 0; i < kSize; ++i)
+    state[i] = content_[i];
+
+  CHECK(false);
+}
diff --git a/chrome/browser/state_tracker.h b/chrome/browser/state_tracker.h
new file mode 100644
index 0000000..3d9ce2e
--- /dev/null
+++ b/chrome/browser/state_tracker.h
@@ -0,0 +1,40 @@
+// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_STATE_TRACKER_H_
+#define CHROME_BROWSER_STATE_TRACKER_H_
+
+#include <string>
+
+#include "base/logging.h"
+#include "base/scoped_ptr.h"
+
+// StateTracker maintains a circular character buffer. It is intended for use in
+// tracking down crashes. As various events occur invoke Append. When you're
+// ready to crash, invoke Crash, which copies the state onto
+// the state and CHECKs.
+// The '!' in the array indicates the position the next character is inserted.
+// The string of characters before the '!' gives the most recent strings that
+// were appended.
+class StateTracker {
+ public:
+  StateTracker();
+
+  // Appends |text|.
+  void Append(const std::string& text);
+
+  // Copies the appended text onto the stack and CHECKs.
+  void Crash();
+
+ private:
+  // Current index into the string content is inserted at.
+  size_t index_;
+
+  // The content.
+  scoped_array<char> content_;
+
+  DISALLOW_COPY_AND_ASSIGN(StateTracker);
+};
+
+#endif  // CHROME_BROWSER_STATE_TRACKER_H_
diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index 1d0e9d9..2fb46aa 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -58,6 +58,7 @@ TabStripModel::TabStripModel(TabStripModelDelegate* delegate, Profile* profile)
 }
 
 TabStripModel::~TabStripModel() {
+  LogEvent(DELETE_MODEL);
   STLDeleteContainerPointers(contents_data_.begin(), contents_data_.end());
   delete order_controller_;
 }
@@ -70,6 +71,14 @@ void TabStripModel::RemoveObserver(TabStripModelObserver* observer) {
   observers_.RemoveObserver(observer);
 }
 
+bool TabStripModel::HasObserver(TabStripModelObserver* observer) {
+  for (size_t i = 0; i < observers_.size(); ++i) {
+    if (observers_.GetElementAt(i) == observer)
+      return true;
+  }
+  return false;
+}
+
 bool TabStripModel::ContainsIndex(int index) const {
   return index >= 0 && index < count();
 }
@@ -84,6 +93,7 @@ void TabStripModel::InsertTabContentsAt(int index,
                                         TabContents* contents,
                                         bool foreground,
                                         bool inherit_group) {
+  LogEvent(INSERT);
   // In tab dragging situations, if the last tab in the window was detached
   // then the user aborted the drag, we will have the |closing_all_| member
   // set (see DetachTabContentsAt) which will mess with our mojo here. We need
@@ -121,6 +131,7 @@ void TabStripModel::InsertTabContentsAt(int index,
 
 void TabStripModel::ReplaceNavigationControllerAt(
     int index, NavigationController* controller) {
+  LogEvent(REPLACE);
   // This appears to be OK with no flicker since no redraw event
   // occurs between the call to add an aditional tab and one to close
   // the previous tab.
@@ -135,12 +146,15 @@ TabContents* TabStripModel::DetachTabContentsAt(int index) {
     return NULL;
 
   DCHECK(ContainsIndex(index));
+  LogEvent(DETACH);
   TabContents* removed_contents = GetContentsAt(index);
   next_selected_index_ = order_controller_->DetermineNewSelectedIndex(index);
   delete contents_data_.at(index);
   contents_data_.erase(contents_data_.begin() + index);
-  if (contents_data_.empty())
+  if (contents_data_.empty()) {
+    LogEvent(DETACH_EMPTY);
     closing_all_ = true;
+  }
   TabStripModelObservers::Iterator iter(observers_);
   while (TabStripModelObserver* obs = iter.GetNext()) {
     obs->TabDetachedAt(removed_contents, index);
@@ -168,6 +182,7 @@ void TabStripModel::SelectTabContentsAt(int index, bool user_gesture) {
 
 void TabStripModel::MoveTabContentsAt(int index, int to_position,
                                       bool select_after_move) {
+  LogEvent(MOVE);
   MoveTabContentsAtImpl(index, to_position, select_after_move, true);
 }
 
@@ -211,6 +226,7 @@ void TabStripModel::UpdateTabContentsStateAt(int index,
 }
 
 void TabStripModel::CloseAllTabs() {
+  LogEvent(CLOSE_ALL);
   // Set state so that observers can adjust their behavior to suit this
   // specific condition when CloseTabContentsAt causes a flurry of
   // Close/Detach/Select notifications to be sent.
@@ -336,6 +352,8 @@ void TabStripModel::SetTabPinned(int index, bool pinned) {
   if (contents_data_[index]->pinned == pinned)
     return;
 
+  LogEvent(PIN);
+
   int first_non_pinned_tab = IndexOfFirstNonPinnedTab();
 
   contents_data_[index]->pinned = pinned;
@@ -460,6 +478,7 @@ void TabStripModel::MoveTabPrevious() {
 Browser* TabStripModel::TearOffTabContents(TabContents* detached_contents,
                                            const gfx::Rect& window_bounds,
                                            const DockInfo& dock_info) {
+  LogEvent(TEAR);
   DCHECK(detached_contents);
   return delegate_->CreateNewStripWithContents(detached_contents, window_bounds,
                                                dock_info);
@@ -590,6 +609,7 @@ void TabStripModel::Observe(NotificationType type,
   // here so we don't crash later.
   int index = GetIndexOfTabContents(Source<TabContents>(source).ptr());
   if (index != TabStripModel::kNoTab) {
+    LogEvent(TAB_DESTROYED);
     // Note that we only detach the contents here, not close it - it's already
     // been closed. We just want to undo our bookkeeping.
     DetachTabContentsAt(index);
@@ -608,6 +628,8 @@ bool TabStripModel::IsNewTabAtEndOfTabStrip(TabContents* contents) const {
 
 bool TabStripModel::InternalCloseTabs(std::vector<int> indices,
                                       bool create_historical_tabs) {
+  LogEvent(CLOSE);
+
   bool retval = true;
 
   // We only try the fast shutdown path if the whole browser process is *not*
@@ -743,3 +765,9 @@ bool TabStripModel::OpenerMatches(const TabContentsData* data,
                                   bool use_group) {
   return data->opener == opener || (use_group && data->group == opener);
 }
+
+void TabStripModel::LogEvent(Event type) {
+  char c = 'a' + (int)type;
+  tracker_.Append(std::string(&c, 1));
+  tracker_.Append(IntToString(count()));
+}
diff --git a/chrome/browser/tabs/tab_strip_model.h b/chrome/browser/tabs/tab_strip_model.h
index d7643b1..19ea875 100644
--- a/chrome/browser/tabs/tab_strip_model.h
+++ b/chrome/browser/tabs/tab_strip_model.h
@@ -8,6 +8,7 @@
 #include <vector>
 
 #include "base/observer_list.h"
+#include "chrome/browser/state_tracker.h"
 #include "chrome/common/notification_registrar.h"
 #include "chrome/common/page_transition_types.h"
 
@@ -282,6 +283,13 @@ class TabStripModel : public NotificationObserver {
     return order_controller_;
   }
 
+  // Returns the StateTracker. This never returns null.
+  StateTracker* tracker() { return &tracker_; }
+
+  // Returns true if |observer| is in the list of observers. This is intended
+  // for debugging.
+  bool HasObserver(TabStripModelObserver* observer);
+
   // Basic API /////////////////////////////////////////////////////////////////
 
   static const int kNoTab = -1;
@@ -545,6 +553,24 @@ class TabStripModel : public NotificationObserver {
   // be |opener|'s NavigationController.
   void SetOpenerForContents(TabContents* contents, TabContents* opener);
 
+  // In hopes of tracking a crash we're logging various events. These events
+  // are logged to tracker_ with the following characters.
+  enum Event {
+    INSERT = 0,      // a
+    REPLACE,         // b
+    DETACH,          // c
+    DETACH_EMPTY,    // d
+    MOVE,            // e
+    CLOSE_ALL,       // f
+    TEAR,            // g
+    DELETE_MODEL,    // h
+    CLOSE,           // i
+    PIN,             // j
+    TAB_DESTROYED    // k
+  };
+
+  void LogEvent(Event type);
+
   // Returns true if the tab represented by the specified data has an opener
   // that matches the specified one. If |use_group| is true, then this will
   // fall back to check the group relationship as well.
@@ -642,6 +668,8 @@ class TabStripModel : public NotificationObserver {
   // A scoped container for notification registries.
   NotificationRegistrar registrar_;
 
+  StateTracker tracker_;
+
   DISALLOW_COPY_AND_ASSIGN(TabStripModel);
 };
 
diff --git a/chrome/browser/views/tabs/tab_strip.cc b/chrome/browser/views/tabs/tab_strip.cc
index e70c88c..f643cbb 100644
--- a/chrome/browser/views/tabs/tab_strip.cc
+++ b/chrome/browser/views/tabs/tab_strip.cc
@@ -1206,34 +1206,17 @@ void TabStrip::MaybeStartDrag(Tab* tab, const views::MouseEvent& event) {
   if (!model_->ContainsIndex(index)) {
     // It appears to be possible for a drag to start with an invalid tab.
     // This records some extra information in hopes of tracking down why.
-    // The string contains the following, in order:
-    // . If a drag is already in progress, a 'D'.
-    // . If the model is closing all, an 'A'.
-    // . Index of tab the user is dragging.
-    // . Number of tabs.
-    // . Size of the model.
-    // . Indices of any tabs that are being closed.
-    // . ! end marker.
     std::string tmp;
-    if (drag_controller_.get())
-      tmp += "D";
+    StateTracker* tracker = model_->tracker();
+    tracker->Append("|");
     if (model_->closing_all())
-      tmp += " A";
-    tmp += " " + IntToString(index);
-    tmp += " " + IntToString(GetTabCount());
-    tmp += " " + IntToString(model_->count());
-    for (int i = 0; i < GetTabCount(); ++i) {
-      if (GetTabAt(i)->closing())
-        tmp += " " + IntToString(i);
-    }
-    tmp += "!";  // End marker so we know we got all closing tabs.
-
-    volatile char tab_state[128];
-    for (size_t i = 0; i < std::min(ARRAYSIZE_UNSAFE(tab_state),
-                                    tmp.size()); ++i) {
-      tab_state[i] = tmp[i];
-    }
-    CHECK(false);
+      tracker->Append("A");
+    tracker->Append(" " + IntToString(index));
+    tracker->Append(" " + IntToString(GetTabCount()));
+    tracker->Append(" " + IntToString(model_->count()));
+    if (model_->HasObserver(this))
+      tracker->Append("Y");
+    tracker->Crash();
     return;
   }
   drag_controller_.reset(new DraggedTabController(tab, this));
diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp
index 7711bfd..07dc335 100755
--- a/chrome/chrome.gyp
+++ b/chrome/chrome.gyp
@@ -2153,6 +2153,8 @@
         'browser/sync/sync_setup_wizard.h',
         'browser/sync/sync_status_ui_helper.cc',
         'browser/sync/sync_status_ui_helper.h',
+        'browser/state_tracker.cc',
+        'browser/state_tracker.h',
         'browser/tab_contents/constrained_window.h',
         'browser/tab_contents/infobar_delegate.cc',
         'browser/tab_contents/infobar_delegate.h',