Make PrerenderHandle an observer of PrerenderContents.

chrome/browser/prerender/prerender_unittest.cc

Index: chrome/browser/prerender/prerender_unittest.cc
diff --git a/chrome/browser/prerender/prerender_unittest.cc b/chrome/browser/prerender/prerender_unittest.cc
index 27fa12a775ce55fbba447f53e8fd171c6718922c..47c05a561a351bf09fc4c45e3004914c73fd3555 100644
--- a/chrome/browser/prerender/prerender_unittest.cc
+++ b/chrome/browser/prerender/prerender_unittest.cc
@@ -135,7 +135,7 @@ class UnitTestPrerenderManager : public PrerenderManager {
     active_prerenders_.erase(to_erase);
 
     prerender_contents->SetFinalStatus(FINAL_STATUS_USED);
-    prerender_contents->StartPendingPrerenders();
+    prerender_contents->PrepareForUse();
     return prerender_contents;
   }
 
@@ -362,12 +362,10 @@ TEST_F(PrerenderTest, DuplicateTest) {
           kDefaultChildId, kDefaultRenderViewRouteId, url,
           Referrer(url, WebKit::WebReferrerPolicyDefault), kSize));
 
-  EXPECT_TRUE(duplicate_prerender_handle->IsValid());
   EXPECT_TRUE(duplicate_prerender_handle->IsPrerendering());
 
   ASSERT_EQ(prerender_contents, prerender_manager()->FindAndUseEntry(url));
 
-  EXPECT_FALSE(duplicate_prerender_handle->IsValid());
   EXPECT_FALSE(duplicate_prerender_handle->IsPrerendering());
 }
 
@@ -649,13 +647,12 @@ TEST_F(PrerenderTest, PendingPrerenderTest) {
           child_id, route_id, pending_url,
           Referrer(url, WebKit::WebReferrerPolicyDefault), kSize));
   CHECK(pending_prerender_handle.get());
-  EXPECT_TRUE(pending_prerender_handle->IsValid());
-  EXPECT_TRUE(pending_prerender_handle->IsPending());
+  EXPECT_FALSE(pending_prerender_handle->IsPrerendering());
 
   EXPECT_TRUE(prerender_contents->prerendering_has_started());
   ASSERT_EQ(prerender_contents, prerender_manager()->FindAndUseEntry(url));
 
-  EXPECT_FALSE(pending_prerender_handle->IsPending());
+  EXPECT_TRUE(pending_prerender_handle->IsPrerendering());
   ASSERT_EQ(pending_prerender_contents,
             prerender_manager()->FindAndUseEntry(pending_url));
 }
@@ -686,14 +683,12 @@ TEST_F(PrerenderTest, InvalidPendingPrerenderTest) {
           child_id, route_id, pending_url,
           Referrer(url, WebKit::WebReferrerPolicyDefault), kSize));
   DCHECK(pending_prerender_handle.get());
-  EXPECT_TRUE(pending_prerender_handle->IsValid());
-  EXPECT_TRUE(pending_prerender_handle->IsPending());
+  EXPECT_FALSE(pending_prerender_handle->IsPrerendering());
 
   EXPECT_TRUE(prerender_contents->prerendering_has_started());
   ASSERT_EQ(prerender_contents, prerender_manager()->FindAndUseEntry(url));
 
-  EXPECT_FALSE(pending_prerender_handle->IsValid());
-  EXPECT_FALSE(pending_prerender_handle->IsPending());
+  EXPECT_FALSE(pending_prerender_handle->IsPrerendering());
 }
 
 TEST_F(PrerenderTest, CancelPendingPrerenderTest) {
@@ -716,15 +711,17 @@ TEST_F(PrerenderTest, CancelPendingPrerenderTest) {
           child_id, route_id, pending_url,
           Referrer(url, WebKit::WebReferrerPolicyDefault), kSize));
   CHECK(pending_prerender_handle.get());
-  EXPECT_TRUE(pending_prerender_handle->IsValid());
-  EXPECT_TRUE(pending_prerender_handle->IsPending());
+  EXPECT_FALSE(pending_prerender_handle->IsPrerendering());
 
   EXPECT_TRUE(prerender_contents->prerendering_has_started());
 
-  pending_prerender_handle->OnCancel();
-  EXPECT_FALSE(pending_prerender_handle->IsValid());
+  EXPECT_FALSE(pending_prerender_handle->IsPrerendering());

[mmenke, 2012/12/13 18:27:16]

nit:  No need to check this twice (Though suppose it is the season for that kind
of thing...)

[gavinp, 2012/12/13 20:45:34]

Done.

+  pending_prerender_handle.reset();
 
   ASSERT_EQ(prerender_contents, prerender_manager()->FindAndUseEntry(url));
+  // We succeed if we don't crash here; if the pending prerender wasn't canceled
+  // then CreateNextPrerenderContents will fail an assertion.
+  SUCCEED();

[mmenke, 2012/12/13 18:27:16]

I'm not following this.

[gavinp, 2012/12/13 20:45:34]

Done.

 }
 
 // Tests that a PrerenderManager created for a browser session in the control
@@ -793,6 +790,7 @@ TEST_F(PrerenderTest, PPLTDummy) {
           url, FINAL_STATUS_UNSUPPORTED_SCHEME);
   EXPECT_TRUE(AddSimplePrerender(url));
   EXPECT_TRUE(prerender_contents->prerendering_has_started());
+  EXPECT_FALSE(IsEmptyPrerenderLinkManager());
 
   DummyPrerenderContents* pplt_dummy_contents =
       prerender_manager()->CreateNextPrerenderContents(url,
@@ -800,8 +798,10 @@ TEST_F(PrerenderTest, PPLTDummy) {
   GURL ftp_url("ftp://ftp.google.com/");
   // Adding this ftp URL will force the expected unsupported scheme error.
   prerender_contents->AddAliasURL(ftp_url);
+  EXPECT_FALSE(IsEmptyPrerenderLinkManager());
 
   ASSERT_EQ(pplt_dummy_contents, prerender_manager()->FindAndUseEntry(url));
+  EXPECT_TRUE(IsEmptyPrerenderLinkManager());
 }
 
 // Tests that our PPLT dummy prerender gets created properly, even
@@ -839,13 +839,14 @@ TEST_F(PrerenderTest, PPLTLateCancel) {
   prerender_contents->Destroy(FINAL_STATUS_JAVASCRIPT_ALERT);
   ASSERT_EQ(duplicate_prerender_contents, prerender_manager()->FindEntry(url));
 
+  // Make sure that events on prerender handles propogate to the match
+  // complete replacement.
+  DummyPrerenderContents* null = NULL;
   prerender_link_manager()->OnCancelPrerender(kDefaultChildId,
                                               last_prerender_id());
-  DummyPrerenderContents* null = NULL;
   ASSERT_EQ(null, prerender_manager()->FindEntry(url));
 }
 
-
 // Tests that the prerender manager matches include the fragment.
 TEST_F(PrerenderTest, FragmentMatchesTest) {
   GURL fragment_url("http://www.google.com/#test");
@@ -986,7 +987,6 @@ TEST_F(PrerenderTest, LinkManagerAbandon) {
 
   EXPECT_FALSE(prerender_contents->prerendering_has_been_cancelled());
   ASSERT_EQ(prerender_contents, prerender_manager()->FindAndUseEntry(url));

[mmenke, 2012/12/13 18:27:16]

I don't think we have a test with a cancel after an abandon, or a use after an
abandon.  Suggest duplicating this test and checking both cases.

[gavinp, 2012/12/13 20:45:34]

The abandon tests do try and use the prerender.

I added a test with cancel after abandon.

-  EXPECT_TRUE(IsEmptyPrerenderLinkManager());
 }
 
 TEST_F(PrerenderTest, LinkManagerCancelTwice) {
@@ -1116,7 +1116,6 @@ TEST_F(PrerenderTest, LinkManagerAddTwiceAbandonTwice) {
   prerender_link_manager()->OnAbandonPrerender(kDefaultChildId,
                                                second_prerender_id);
 
-  EXPECT_TRUE(IsEmptyPrerenderLinkManager());
   EXPECT_FALSE(prerender_contents->prerendering_has_been_cancelled());
   ASSERT_EQ(prerender_contents, prerender_manager()->FindAndUseEntry(url));
 }
@@ -1172,9 +1171,6 @@ TEST_F(PrerenderTest, LinkManagerExpireThenAddAgain) {
   EXPECT_TRUE(second_prerender_contents->prerendering_has_started());
   ASSERT_EQ(second_prerender_contents,
             prerender_manager()->FindAndUseEntry(url));
-  // The PrerenderLinkManager is not empty since we never removed the first
-  // prerender.
-  EXPECT_FALSE(IsEmptyPrerenderLinkManager());
 }
 
 TEST_F(PrerenderTest, LinkManagerCancelThenAddAgain) {
@@ -1200,7 +1196,6 @@ TEST_F(PrerenderTest, LinkManagerCancelThenAddAgain) {
   EXPECT_TRUE(second_prerender_contents->prerendering_has_started());
   ASSERT_EQ(second_prerender_contents,
             prerender_manager()->FindAndUseEntry(url));
-  EXPECT_FALSE(IsEmptyPrerenderLinkManager());
 }
 
 }  // namespace prerender