Fix memory error in unit test

chrome/browser/web_contents_unittest.cc

Index: chrome/browser/web_contents_unittest.cc
===================================================================
--- chrome/browser/web_contents_unittest.cc	(revision 7309)
+++ chrome/browser/web_contents_unittest.cc	(working copy)
@@ -258,6 +258,15 @@
     CANCELED       // DontProceed was called.
   };
 
+  // IMPORTANT NOTE: if you pass stack allocated values for |state| and
+  // |deleted| (like all interstitial related tests do at this point), make sure
+  // to create an instance of the TestInterstitialPageStateGuard class on the
+  // stack in your test.  This will ensure that the TestInterstitialPage states
+  // are cleared when the test finishes.
+  // Not doing so will cause stack trashing if your test does not hide the
+  // interstitial, as in such a case it will be destroyed in the test TearDown
+  // method and will dereference the |deleted| local variable which by then is
+  // out of scope.
   TestInterstitialPage(WebContents* tab,
                        bool new_navigation,
                        const GURL& url,
@@ -272,15 +281,18 @@
   }
 
   virtual ~TestInterstitialPage() {
-    *deleted_ = true;
+    if (deleted_)
+      *deleted_ = true;
   }
 
   virtual void DontProceed() {
-    *state_ = CANCELED;
+    if (state_)
+      *state_ = CANCELED;
     InterstitialPage::DontProceed();
   }
   virtual void Proceed() {
-    *state_ = OKED;
+    if (state_)
+      *state_ = OKED;
     InterstitialPage::Proceed();
   }
 
@@ -307,6 +319,11 @@
         is_showing();
   }
 
+  void ClearStates() {
+    state_ = NULL;
+    deleted_ = NULL;
+  }
+
  protected:
   virtual RenderViewHost* CreateRenderViewHost() {
     return new TestRenderViewHost(
@@ -324,6 +341,20 @@
   int command_received_count_;
 };
 
+class TestInterstitialPageStateGuard {
+ public:
+  TestInterstitialPageStateGuard(TestInterstitialPage* interstitial_page)
+      : interstitial_page_(interstitial_page) {
+    DCHECK(interstitial_page_);
+  }
+  ~TestInterstitialPageStateGuard() {
+    interstitial_page_->ClearStates();
+  }
+
+ private:
+  TestInterstitialPage* interstitial_page_;
+};
+
 class WebContentsTest : public testing::Test {
  public:
   WebContentsTest() : contents(NULL) {}
@@ -756,6 +787,7 @@
   GURL url2("http://interstitial");
   TestInterstitialPage* interstitial =
       new TestInterstitialPage(contents, true, url2, &state, &deleted);
+  TestInterstitialPageStateGuard state_guard(interstitial);
   interstitial->Show();
   // The interstitial should not show until its navigation has committed.
   EXPECT_FALSE(interstitial->is_showing());
@@ -800,6 +832,7 @@
   GURL url2("http://interstitial");
   TestInterstitialPage* interstitial =
       new TestInterstitialPage(contents, true, url2, &state, &deleted);
+  TestInterstitialPageStateGuard state_guard(interstitial);
   interstitial->Show();
   // The interstitial should not show until its navigation has committed.
   EXPECT_FALSE(interstitial->is_showing());
@@ -842,6 +875,7 @@
   GURL url2("http://interstitial");
   TestInterstitialPage* interstitial =
       new TestInterstitialPage(contents, false, url2, &state, &deleted);
+  TestInterstitialPageStateGuard state_guard(interstitial);
   interstitial->Show();
   // The interstitial should not show until its navigation has committed.
   EXPECT_FALSE(interstitial->is_showing());
@@ -889,6 +923,7 @@
   GURL url2("http://interstitial");
   TestInterstitialPage* interstitial =
       new TestInterstitialPage(contents, true, url2, &state, &deleted);
+  TestInterstitialPageStateGuard state_guard(interstitial);
   interstitial->Show();
   // The interstitial should not show until its navigation has committed.
   EXPECT_FALSE(interstitial->is_showing());
@@ -942,6 +977,7 @@
   GURL url2("http://interstitial");
   TestInterstitialPage* interstitial =
       new TestInterstitialPage(contents, true, url2, &state, &deleted);
+  TestInterstitialPageStateGuard state_guard(interstitial);
   interstitial->Show();
   // The interstitial should not show until its navigation has committed.
   EXPECT_FALSE(interstitial->is_showing());
@@ -995,6 +1031,7 @@
   GURL url2("http://interstitial");
   TestInterstitialPage* interstitial =
       new TestInterstitialPage(contents, false, url2, &state, &deleted);
+  TestInterstitialPageStateGuard state_guard(interstitial);
   interstitial->Show();
   // The interstitial should not show until its navigation has committed.
   EXPECT_FALSE(interstitial->is_showing());
@@ -1034,6 +1071,7 @@
   GURL url("http://interstitial");
   TestInterstitialPage* interstitial =
       new TestInterstitialPage(contents, true, url, &state, &deleted);
+  TestInterstitialPageStateGuard state_guard(interstitial);
   interstitial->Show();
   interstitial->TestDidNavigate(1, url);
 
@@ -1054,6 +1092,7 @@
   GURL url("http://interstitial");
   TestInterstitialPage* interstitial =
       new TestInterstitialPage(contents, true, url, &state, &deleted);
+  TestInterstitialPageStateGuard state_guard(interstitial);
   interstitial->Show();
   interstitial->TestDidNavigate(1, url);
 
@@ -1066,8 +1105,7 @@
 
 // Test that after Proceed is called and an interstitial is still shown, no more
 // commands get executed.
-// TODO(jcampan): bug #5700 Disabled because crashes on "XP Test" build bot.
-TEST_F(WebContentsTest, DISABLED_ShowInterstitialProceedMultipleCommands) {
+TEST_F(WebContentsTest, ShowInterstitialProceedMultipleCommands) {
   // Navigate to a page so we have a navigation entry in the controller.
   GURL url1("http://www.google.com");
   Navigate(1, url1);
@@ -1080,6 +1118,7 @@
   GURL url2("http://interstitial");
   TestInterstitialPage* interstitial =
       new TestInterstitialPage(contents, true, url2, &state, &deleted);
+  TestInterstitialPageStateGuard state_guard(interstitial);
   interstitial->Show();
   interstitial->TestDidNavigate(1, url2);
 
@@ -1113,6 +1152,7 @@
   GURL url1("http://interstitial1");
   TestInterstitialPage* interstitial1 =
       new TestInterstitialPage(contents, true, url1, &state1, &deleted1);
+  TestInterstitialPageStateGuard state_guard1(interstitial1);
   interstitial1->Show();
   interstitial1->TestDidNavigate(1, url1);
 
@@ -1123,6 +1163,7 @@
   GURL url2("http://interstitial2");
   TestInterstitialPage* interstitial2 =
       new TestInterstitialPage(contents, true, url2, &state2, &deleted2);
+  TestInterstitialPageStateGuard state_guard2(interstitial2);
   interstitial2->Show();
   interstitial2->TestDidNavigate(1, url2);
 
@@ -1157,6 +1198,7 @@
   TestInterstitialPage* interstitial =
       new TestInterstitialPage(contents, true, interstitial_url,
                                &state, &deleted);
+  TestInterstitialPageStateGuard state_guard(interstitial);
   interstitial->Show();
 
   // Let's simulate a navigation initiated from the browser before the
@@ -1183,6 +1225,7 @@
   GURL url("http://interstitial");
   TestInterstitialPage* interstitial =
       new TestInterstitialPage(contents, true, url, &state, &deleted);
+  TestInterstitialPageStateGuard state_guard(interstitial);
   interstitial->Show();
   // Simulate a renderer crash before the interstitial is shown.
   interstitial->TestRendererGone();