Revert 99295 - Add some more temporary instrumentation for diagnosing bug 93747.

The goal is to conclusively tell whether the object is freed or not when running TabStripModel::GetWrapperIndex. Also surrounded the vector that we are seeing as corrupt with some special padding bytes (64 bytes total), to see if that area of memory is getting clobbered.

BUG=93747

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

TBR=eroman@chromium.org
Review URL: http://codereview.chromium.org/7831031

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

2 files changed, 9 insertions, 78 deletions

diff --git a/chrome/browser/tabs/tab_strip_model.cc b/chrome/browser/tabs/tab_strip_model.cc
index df424af..d5f6799 100644
--- a/chrome/browser/tabs/tab_strip_model.cc
+++ b/chrome/browser/tabs/tab_strip_model.cc
@@ -8,7 +8,6 @@
 #include <map>
 
 #include "base/command_line.h"
-#include "base/debug/alias.h"
 #include "base/stl_util.h"
 #include "base/string_util.h"
 #include "build/build_config.h"
@@ -36,15 +35,6 @@
 
 namespace {
 
-// The instance is alive.
-const uint32 kMagicIdAlive = 0xCa11ab1e;
-
-// The destructor is running (and hasn't finished yet).
-const uint32 kMagicIdDestroying = 0xB100D1ED;
-
-  // The instance has been deleted.
-const uint32 kMagicIdDead = 0xDECEA5ED;
-
 // Returns true if the specified transition is one of the types that cause the
 // opener relationships for the tab in which the transition occured to be
 // forgotten. This is generally any navigation that isn't a link click (i.e.
@@ -75,7 +65,7 @@ TabStripModel::TabStripModel(TabStripModelDelegate* delegate, Profile* profile)
       profile_(profile),
       closing_all_(false),
       order_controller_(NULL),
-      magic_id_(kMagicIdAlive) {
+      magic_id_(0) {
   DCHECK(delegate_);
   registrar_.Add(this,
                  content::NOTIFICATION_TAB_CONTENTS_DESTROYED,
@@ -86,55 +76,16 @@ TabStripModel::TabStripModel(TabStripModelDelegate* delegate, Profile* profile)
   order_controller_ = new TabStripModelOrderController(this);
 }
 
-// TODO(eroman): Remove this when done investigating 93747.
-#if defined(COMPILER_MSVC)
-#pragma optimize("", off)
-#endif
-
 TabStripModel::~TabStripModel() {
-  CheckIsAliveAndWell();
-  magic_id_ = kMagicIdDestroying;
+  magic_id_ = 0x1234567;
   FOR_EACH_OBSERVER(TabStripModelObserver, observers_,
                     TabStripModelDeleted());
   STLDeleteElements(&contents_data_);
   delete order_controller_;
   order_controller_ = NULL;
-  magic_id_ = kMagicIdDead;
-}
-
-TabStripModel::Padding::Padding() {
-  for (size_t i = 0; i < arraysize(bytes_); ++i)
-    bytes_[i] = static_cast<char>(89 + i);
-}
-
-void TabStripModel::Padding::CheckValid() const {
-  for (size_t i = 0; i < arraysize(bytes_); ++i)
-    CHECK_EQ(bytes_[i], static_cast<char>(89 + i));
-}
-
-void TabStripModel::CheckIsAliveAndWell() const {
-  // Copy everything that might be of interest onto the stack, to guarantee it
-  // will be available in the minidumps should we crash here.
-  uint32 magic_id = magic_id_;
-  Padding padding1 = padding1_;
-  Padding padding2 = padding2_;
-
-  // Make sure the object is alive.
-  CHECK_EQ(magic_id_, kMagicIdAlive);
-
-  // Make sure our memory hasn't been tampered with.
-  padding1_.CheckValid();
-  padding2_.CheckValid();
-
-  base::debug::Alias(&magic_id);
-  base::debug::Alias(&padding1);
-  base::debug::Alias(&padding2);
+  magic_id_ = 0x76543210;
 }
 
-#if defined(COMPILER_MSVC)
-#pragma optimize("", on)
-#endif
-
 void TabStripModel::AddObserver(TabStripModelObserver* observer) {
   observers_.AddObserver(observer);
 }
@@ -260,7 +211,7 @@ TabContentsWrapper* TabStripModel::DetachTabContentsAt(int index) {
   if (contents_data_.empty())
     return NULL;
 
-  CHECK(ContainsIndex(index));
+  DCHECK(ContainsIndex(index));
 
   TabContentsWrapper* removed_contents = GetContentsAt(index);
   bool was_selected = IsTabSelected(index);
@@ -309,7 +260,7 @@ TabContentsWrapper* TabStripModel::DetachTabContentsAt(int index) {
 }
 
 void TabStripModel::ActivateTabAt(int index, bool user_gesture) {
-  CHECK(ContainsIndex(index));
+  DCHECK(ContainsIndex(index));
   TabContentsWrapper* old_contents = GetActiveTabContents();
   TabStripSelectionModel old_model;
   old_model.Copy(selection_model_);
@@ -320,7 +271,7 @@ void TabStripModel::ActivateTabAt(int index, bool user_gesture) {
 void TabStripModel::MoveTabContentsAt(int index,
                                       int to_position,
                                       bool select_after_move) {
-  CHECK(ContainsIndex(index));
+  DCHECK(ContainsIndex(index));
   if (index == to_position)
     return;
 
@@ -390,10 +341,11 @@ int TabStripModel::GetIndexOfTabContents(
 }
 
 int TabStripModel::GetWrapperIndex(const TabContents* contents) const {
-  CheckIsAliveAndWell();
   int index = 0;
   TabContentsDataVector::const_iterator iter = contents_data_.begin();
   for (; iter != contents_data_.end(); ++iter, ++index) {
+    CHECK(*iter) << magic_id_;
+    CHECK((*iter)->contents) << magic_id_;
     if ((*iter)->contents->tab_contents() == contents)
       return index;
   }
@@ -1314,9 +1266,6 @@ void TabStripModel::SelectRelativeTab(bool next) {
 void TabStripModel::MoveTabContentsAtImpl(int index,
                                           int to_position,
                                           bool select_after_move) {
-  CHECK(ContainsIndex(index));
-  CHECK(to_position >=0 || to_position <= count());
-
   TabContentsData* moved_data = contents_data_.at(index);
   contents_data_.erase(contents_data_.begin() + index);
   contents_data_.insert(contents_data_.begin() + to_position, moved_data);
diff --git a/chrome/browser/tabs/tab_strip_model.h b/chrome/browser/tabs/tab_strip_model.h
index 54252f5..49ce5f2 100644
--- a/chrome/browser/tabs/tab_strip_model.h
+++ b/chrome/browser/tabs/tab_strip_model.h
@@ -622,25 +622,7 @@ class TabStripModel : public NotificationObserver {
 
   // The TabContents data currently hosted within this TabStripModel.
   typedef std::vector<TabContentsData*> TabContentsDataVector;
-
-  // TODO(eroman): Temporary for investigating 93747.
-  class Padding {
-   public:
-    Padding();
-    void CheckValid() const;
-
-   private:
-    char bytes_[32];
-  };
-
-  // Surround contents_data_ with some extra bytes to try and catch external
-  // mutation of this memory. This is temporary for debugging 93747.
-  Padding padding1_;
   TabContentsDataVector contents_data_;
-  Padding padding2_;
-
-  // This is temporary for debugging 93747.
-  void CheckIsAliveAndWell() const;
 
   // A profile associated with this TabStripModel, used when creating new Tabs.
   Profile* profile_;
@@ -661,7 +643,7 @@ class TabStripModel : public NotificationObserver {
 
   TabStripSelectionModel selection_model_;
 
-  uint32 magic_id_;
+  int32 magic_id_;
 
   DISALLOW_IMPLICIT_CONSTRUCTORS(TabStripModel);
 };