Switch content_shell from using PruneAllButActive to a more explicit approach

During layout tests, we want to reuse one WebContents for running multiple
layout tests. Since layout tests assume that they're the first entry in the
session history, we need to reset the session history.

DumpRenderTree achieved this by just killing the NavigationController. Since
that's not possible in the content module, I used the instant api
PruneAllButActive. This API, however, depends on the currently active RenderView
being the last in the session history, which is almost impossible to guarantee
in practice.

Instead, I added a flag to LoadURLParams that instructs the NavigationController
to reset itself as soon as the navigation to the next test commits.

BUG=111316
TEST=the fast/history tests don't fail flakily
TBR=creis@chromium.org, jam@chromium.org

Review URL: https://codereview.chromium.org/14134003

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

14 files changed, 178 insertions, 20 deletions

diff --git a/content/browser/renderer_host/test_render_view_host.cc b/content/browser/renderer_host/test_render_view_host.cc
index cff1d21..0e1792a 100644
--- a/content/browser/renderer_host/test_render_view_host.cc
+++ b/content/browser/renderer_host/test_render_view_host.cc
@@ -241,6 +241,7 @@ TestRenderViewHost::TestRenderViewHost(
       render_view_created_(false),
       delete_counter_(NULL),
       simulate_fetch_via_proxy_(false),
+      simulate_history_list_was_cleared_(false),
       contents_mime_type_("text/html") {
   // For normal RenderViewHosts, this is freed when |Shutdown()| is
   // called.  For TestRenderViewHost, the view is explicitly
@@ -322,6 +323,7 @@ void TestRenderViewHost::SendNavigateWithParameters(
   params.socket_address.set_host("2001:db8::1");
   params.socket_address.set_port(80);
   params.was_fetched_via_proxy = simulate_fetch_via_proxy_;
+  params.history_list_was_cleared = simulate_history_list_was_cleared_;
   params.content_state = webkit_glue::CreateHistoryStateForURL(GURL(url));
   params.original_request_url = original_request_url;
 
@@ -362,6 +364,10 @@ void TestRenderViewHost::set_simulate_fetch_via_proxy(bool proxy) {
   simulate_fetch_via_proxy_ = proxy;
 }
 
+void TestRenderViewHost::set_simulate_history_list_was_cleared(bool cleared) {
+  simulate_history_list_was_cleared_ = cleared;
+}
+
 RenderViewHostImplTestHarness::RenderViewHostImplTestHarness() {
 }
 
diff --git a/content/browser/renderer_host/test_render_view_host.h b/content/browser/renderer_host/test_render_view_host.h
index 4251acf..a6ec555 100644
--- a/content/browser/renderer_host/test_render_view_host.h
+++ b/content/browser/renderer_host/test_render_view_host.h
@@ -284,6 +284,11 @@ class TestRenderViewHost
   // False by default.
   void set_simulate_fetch_via_proxy(bool proxy);
 
+  // If set, navigations will appear to have cleared the history list in the
+  // RenderView (ViewHostMsg_FrameNavigate_Params::history_list_was_cleared).
+  // False by default.
+  void set_simulate_history_list_was_cleared(bool cleared);
+
   // RenderViewHost overrides --------------------------------------------------
 
   virtual bool CreateRenderView(const string16& frame_name,
@@ -319,6 +324,9 @@ class TestRenderViewHost
   // See set_simulate_fetch_via_proxy() above.
   bool simulate_fetch_via_proxy_;
 
+  // See set_simulate_history_list_was_cleared() above.
+  bool simulate_history_list_was_cleared_;
+
   // See SetContentsMimeType() above.
   std::string contents_mime_type_;
 
diff --git a/content/browser/web_contents/navigation_controller_impl.cc b/content/browser/web_contents/navigation_controller_impl.cc
index 6051efe..f7c8215 100644
--- a/content/browser/web_contents/navigation_controller_impl.cc
+++ b/content/browser/web_contents/navigation_controller_impl.cc
@@ -659,6 +659,7 @@ void NavigationControllerImpl::LoadURLWithParams(const LoadURLParams& params) {
           browser_context_));
   if (params.is_cross_site_redirect)
     entry->set_should_replace_entry(true);
+  entry->set_should_clear_history_list(params.should_clear_history_list);
   entry->SetIsOverridingUserAgent(override);
   entry->set_transferred_global_request_id(
       params.transferred_global_request_id);
@@ -781,6 +782,10 @@ bool NavigationControllerImpl::RendererDidNavigate(
   // the renderer.
   active_entry->set_is_renderer_initiated(false);
 
+  // Once committed, we no longer need to track whether the session history was
+  // cleared. Navigating to this entry again shouldn't clear it again.
+  active_entry->set_should_clear_history_list(false);
+
   // The active entry's SiteInstance should match our SiteInstance.
   CHECK(active_entry->site_instance() == web_contents_->GetSiteInstance());
 
@@ -843,6 +848,9 @@ NavigationType NavigationControllerImpl::ClassifyNavigation(
     return NAVIGATION_TYPE_NEW_SUBFRAME;
   }
 
+  // We only clear the session history when navigating to a new page.
+  DCHECK(!params.history_list_was_cleared);
+
   // Now we know that the notification is for an existing page. Find that entry.
   int existing_entry_index = GetEntryIndexWithPageID(
       web_contents_->GetSiteInstance(),
@@ -988,6 +996,16 @@ void NavigationControllerImpl::RendererDidNavigateToNewPage(
   new_entry->SetOriginalRequestURL(params.original_request_url);
   new_entry->SetIsOverridingUserAgent(params.is_overriding_user_agent);
 
+  DCHECK(!params.history_list_was_cleared || !replace_entry);
+  // The browser requested to clear the session history when it initiated the
+  // navigation. Now we know that the renderer has updated its state accordingly
+  // and it is safe to also clear the browser side history.
+  if (params.history_list_was_cleared) {
+    DiscardNonCommittedEntriesInternal();
+    entries_.clear();
+    last_committed_entry_index_ = -1;
+  }
+
   InsertOrReplaceEntry(new_entry, replace_entry);
 }
 
diff --git a/content/browser/web_contents/navigation_controller_impl_unittest.cc b/content/browser/web_contents/navigation_controller_impl_unittest.cc
index a8433e7..1442be1 100644
--- a/content/browser/web_contents/navigation_controller_impl_unittest.cc
+++ b/content/browser/web_contents/navigation_controller_impl_unittest.cc
@@ -3325,6 +3325,55 @@ TEST_F(NavigationControllerTest, MAYBE_PurgeScreenshot) {
   }
 }
 
+// Test that the navigation controller clears its session history when a
+// navigation commits with the clear history list flag set.
+TEST_F(NavigationControllerTest, ClearHistoryList) {
+  const GURL url1("http://foo1");
+  const GURL url2("http://foo2");
+  const GURL url3("http://foo3");
+  const GURL url4("http://foo4");
+
+  NavigationControllerImpl& controller = controller_impl();
+
+  // Create a session history with three entries, second entry is active.
+  NavigateAndCommit(url1);
+  NavigateAndCommit(url2);
+  NavigateAndCommit(url3);
+  controller.GoBack();
+  contents()->CommitPendingNavigation();
+  EXPECT_EQ(3, controller.GetEntryCount());
+  EXPECT_EQ(1, controller.GetCurrentEntryIndex());
+
+  // Create a new pending navigation, and indicate that the session history
+  // should be cleared.
+  NavigationController::LoadURLParams params(url4);
+  params.should_clear_history_list = true;
+  controller.LoadURLWithParams(params);
+
+  // Verify that the pending entry correctly indicates that the session history
+  // should be cleared.
+  NavigationEntryImpl* entry =
+      NavigationEntryImpl::FromNavigationEntry(
+          controller.GetPendingEntry());
+  ASSERT_TRUE(entry);
+  EXPECT_TRUE(entry->should_clear_history_list());
+
+  // Assume that the RV correctly cleared its history and commit the navigation.
+  static_cast<TestRenderViewHost*>(contents()->GetPendingRenderViewHost())->
+      set_simulate_history_list_was_cleared(true);
+  contents()->CommitPendingNavigation();
+
+  // Verify that the NavigationController's session history was correctly
+  // cleared.
+  EXPECT_EQ(1, controller.GetEntryCount());
+  EXPECT_EQ(0, controller.GetCurrentEntryIndex());
+  EXPECT_EQ(0, controller.GetLastCommittedEntryIndex());
+  EXPECT_EQ(-1, controller.GetPendingEntryIndex());
+  EXPECT_FALSE(controller.CanGoBack());
+  EXPECT_FALSE(controller.CanGoForward());
+  EXPECT_EQ(url4, controller.GetActiveEntry()->GetURL());
+}
+
 /* TODO(brettw) These test pass on my local machine but fail on the XP buildbot
    (but not Vista) cleaning up the directory after they run.
    This should be fixed.
diff --git a/content/browser/web_contents/navigation_entry_impl.cc b/content/browser/web_contents/navigation_entry_impl.cc
index b3b6f78..b904acf 100644
--- a/content/browser/web_contents/navigation_entry_impl.cc
+++ b/content/browser/web_contents/navigation_entry_impl.cc
@@ -50,6 +50,7 @@ NavigationEntryImpl::NavigationEntryImpl()
       is_overriding_user_agent_(false),
       is_renderer_initiated_(false),
       should_replace_entry_(false),
+      should_clear_history_list_(false),
       can_load_local_resources_(false) {
 }
 
@@ -76,6 +77,7 @@ NavigationEntryImpl::NavigationEntryImpl(SiteInstanceImpl* instance,
       is_overriding_user_agent_(false),
       is_renderer_initiated_(is_renderer_initiated),
       should_replace_entry_(false),
+      should_clear_history_list_(false),
       can_load_local_resources_(false) {
 }
 
diff --git a/content/browser/web_contents/navigation_entry_impl.h b/content/browser/web_contents/navigation_entry_impl.h
index c5d8c49..c956867 100644
--- a/content/browser/web_contents/navigation_entry_impl.h
+++ b/content/browser/web_contents/navigation_entry_impl.h
@@ -190,6 +190,15 @@ class CONTENT_EXPORT NavigationEntryImpl
     return screenshot_;
   }
 
+  // Whether this (pending) navigation should clear the session history. Resets
+  // to false after commit.
+  bool should_clear_history_list() const {
+    return should_clear_history_list_;
+  }
+  void set_should_clear_history_list(bool should_clear_history_list) {
+    should_clear_history_list_ = should_clear_history_list;
+  }
+
  private:
   // WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING
   // Session/Tab restore save portions of this class so that it can be recreated
@@ -275,6 +284,13 @@ class CONTENT_EXPORT NavigationEntryImpl
   // doing the redirect).
   bool should_replace_entry_;
 
+  // This is set to true when this entry's navigation should clear the session
+  // history both on the renderer and browser side. The browser side history
+  // won't be cleared until the renderer has committed this navigation. This
+  // entry is not persisted by the session restore system, as it is always
+  // reset to false after commit.
+  bool should_clear_history_list_;
+
   // Set when this entry should be able to access local file:// resources. This
   // value is not needed after the entry commits and is not persisted.
   bool can_load_local_resources_;
diff --git a/content/browser/web_contents/web_contents_impl.cc b/content/browser/web_contents/web_contents_impl.cc
index 7ba9ba4..78e728f 100644
--- a/content/browser/web_contents/web_contents_impl.cc
+++ b/content/browser/web_contents/web_contents_impl.cc
@@ -199,9 +199,20 @@ void MakeNavigateParams(const NavigationEntryImpl& entry,
                         NavigationController::ReloadType reload_type,
                         ViewMsg_Navigate_Params* params) {
   params->page_id = entry.GetPageID();
-  params->pending_history_list_offset = controller.GetIndexOfEntry(&entry);
-  params->current_history_list_offset = controller.GetLastCommittedEntryIndex();
-  params->current_history_list_length = controller.GetEntryCount();
+  params->should_clear_history_list = entry.should_clear_history_list();
+  if (entry.should_clear_history_list()) {
+    // Set the history list related parameters to the same values a
+    // NavigationController would return before its first navigation. This will
+    // fully clear the RenderView's view of the session history.
+    params->pending_history_list_offset = -1;
+    params->current_history_list_offset = -1;
+    params->current_history_list_length = 0;
+  } else {
+    params->pending_history_list_offset = controller.GetIndexOfEntry(&entry);
+    params->current_history_list_offset =
+        controller.GetLastCommittedEntryIndex();
+    params->current_history_list_length = controller.GetEntryCount();
+  }
   if (!entry.GetBaseURLForDataURL().is_empty()) {
     params->base_url_for_data_url = entry.GetBaseURLForDataURL();
     params->history_url_for_data_url = entry.GetVirtualURL();
diff --git a/content/common/view_messages.h b/content/common/view_messages.h
index 575d481..adc35cf 100644
--- a/content/common/view_messages.h
+++ b/content/common/view_messages.h
@@ -421,6 +421,10 @@ IPC_STRUCT_BEGIN_WITH_PARENT(ViewHostMsg_FrameNavigate_Params,
 
   // User agent override used to navigate.
   IPC_STRUCT_MEMBER(bool, is_overriding_user_agent)
+
+  // Notifies the browser that for this navigation, the session history was
+  // successfully cleared.
+  IPC_STRUCT_MEMBER(bool, history_list_was_cleared)
 IPC_STRUCT_END()
 
 IPC_STRUCT_BEGIN(ViewHostMsg_OpenURL_Params)
@@ -572,6 +576,11 @@ IPC_STRUCT_BEGIN(ViewMsg_Navigate_Params)
   IPC_STRUCT_MEMBER(int, current_history_list_offset)
   IPC_STRUCT_MEMBER(int, current_history_list_length)
 
+  // Informs the RenderView the session history should be cleared. In that
+  // case, the RenderView needs to notify the browser that the clearing was
+  // succesful when the navigation commits.
+  IPC_STRUCT_MEMBER(bool, should_clear_history_list)
+
   // The URL to load.
   IPC_STRUCT_MEMBER(GURL, url)
 
diff --git a/content/public/browser/navigation_controller.cc b/content/public/browser/navigation_controller.cc
index 5fc56dc..fa1dc1b 100644
--- a/content/public/browser/navigation_controller.cc
+++ b/content/public/browser/navigation_controller.cc
@@ -16,7 +16,8 @@ NavigationController::LoadURLParams::LoadURLParams(const GURL& url)
       override_user_agent(UA_OVERRIDE_INHERIT),
       browser_initiated_post_data(NULL),
       can_load_local_resources(false),
-      is_cross_site_redirect(false) {
+      is_cross_site_redirect(false),
+      should_clear_history_list(false)  {
 }
 
 NavigationController::LoadURLParams::~LoadURLParams() {
@@ -35,7 +36,8 @@ NavigationController::LoadURLParams::LoadURLParams(
       base_url_for_data_url(other.base_url_for_data_url),
       virtual_url_for_data_url(other.virtual_url_for_data_url),
       browser_initiated_post_data(other.browser_initiated_post_data),
-      is_cross_site_redirect(false) {
+      is_cross_site_redirect(false),
+      should_clear_history_list(false) {
 }
 
 NavigationController::LoadURLParams&
@@ -53,6 +55,7 @@ NavigationController::LoadURLParams::operator=(
   virtual_url_for_data_url = other.virtual_url_for_data_url;
   browser_initiated_post_data = other.browser_initiated_post_data;
   is_cross_site_redirect = other.is_cross_site_redirect;
+  should_clear_history_list = other.should_clear_history_list;
 
   return *this;
 }
diff --git a/content/public/browser/navigation_controller.h b/content/public/browser/navigation_controller.h
index 1cb18ec..5109301 100644
--- a/content/public/browser/navigation_controller.h
+++ b/content/public/browser/navigation_controller.h
@@ -159,6 +159,14 @@ class NavigationController {
     // navigated. This is currently only used in tests.
     std::string frame_name;
 
+    // Indicates that during this navigation, the session history should be
+    // cleared such that the resulting page is the first and only entry of the
+    // session history.
+    //
+    // The clearing is done asynchronously, and completes when this navigation
+    // commits.
+    bool should_clear_history_list;
+
     explicit LoadURLParams(const GURL& url);
     ~LoadURLParams();
 
diff --git a/content/public/renderer/navigation_state.cc b/content/public/renderer/navigation_state.cc
index 9208efa..5745a2b 100644
--- a/content/public/renderer/navigation_state.cc
+++ b/content/public/renderer/navigation_state.cc
@@ -9,12 +9,14 @@ namespace content {
 NavigationState::NavigationState(content::PageTransition transition_type,
                                  bool is_content_initiated,
                                  int32 pending_page_id,
-                                 int pending_history_list_offset)
+                                 int pending_history_list_offset,
+                                 bool history_list_was_cleared)
     : transition_type_(transition_type),
       request_committed_(false),
       is_content_initiated_(is_content_initiated),
       pending_page_id_(pending_page_id),
       pending_history_list_offset_(pending_history_list_offset),
+      history_list_was_cleared_(history_list_was_cleared),
       was_within_same_page_(false),
       transferred_request_child_id_(-1),
       transferred_request_request_id_(-1),
diff --git a/content/public/renderer/navigation_state.h b/content/public/renderer/navigation_state.h
index ba9f438..01f92a0 100644
--- a/content/public/renderer/navigation_state.h
+++ b/content/public/renderer/navigation_state.h
@@ -22,13 +22,18 @@ class CONTENT_EXPORT NavigationState {
   static NavigationState* CreateBrowserInitiated(
       int32 pending_page_id,
       int pending_history_list_offset,
+      bool history_list_was_cleared,
       content::PageTransition transition_type) {
-    return new NavigationState(transition_type, false, pending_page_id,
-                               pending_history_list_offset);
+    return new NavigationState(transition_type,
+                               false,
+                               pending_page_id,
+                               pending_history_list_offset,
+                               history_list_was_cleared);
   }
 
   static NavigationState* CreateContentInitiated() {
-    return new NavigationState(content::PAGE_TRANSITION_LINK, true, -1, -1);
+    return new NavigationState(
+        content::PAGE_TRANSITION_LINK, true, -1, -1, false);
   }
 
   // Contains the page_id for this navigation or -1 if there is none yet.
@@ -40,6 +45,12 @@ class CONTENT_EXPORT NavigationState {
     return pending_history_list_offset_;
   }
 
+  // If pending_page_id() is not -1, then this returns true if the session
+  // history was cleared during this navigation.
+  bool history_list_was_cleared() const {
+    return history_list_was_cleared_;
+  }
+
   // Contains the transition type that the browser specified when it
   // initiated the load.
   content::PageTransition transition_type() const { return transition_type_; }
@@ -91,13 +102,15 @@ class CONTENT_EXPORT NavigationState {
   NavigationState(content::PageTransition transition_type,
                   bool is_content_initiated,
                   int32 pending_page_id,
-                  int pending_history_list_offset);
+                  int pending_history_list_offset,
+                  bool history_list_was_cleared);
 
   content::PageTransition transition_type_;
   bool request_committed_;
   bool is_content_initiated_;
   int32 pending_page_id_;
   int pending_history_list_offset_;
+  bool history_list_was_cleared_;
 
   bool was_within_same_page_;
   int transferred_request_child_id_;
diff --git a/content/renderer/render_view_impl.cc b/content/renderer/render_view_impl.cc
index ab7bde9..6f0a219 100644
--- a/content/renderer/render_view_impl.cc
+++ b/content/renderer/render_view_impl.cc
@@ -1164,6 +1164,11 @@ void RenderViewImpl::OnNavigate(const ViewMsg_Navigate_Params& params) {
     SetSwappedOut(false);
   }
 
+  if (params.should_clear_history_list) {
+    CHECK_EQ(params.pending_history_list_offset, -1);
+    CHECK_EQ(params.current_history_list_offset, -1);
+    CHECK_EQ(params.current_history_list_length, 0);
+  }
   history_list_offset_ = params.current_history_list_offset;
   history_list_length_ = params.current_history_list_length;
   if (history_list_length_ >= 0)
@@ -1746,6 +1751,9 @@ void RenderViewImpl::UpdateURL(WebFrame* frame) {
     // Track the URL of the original request.
     params.original_request_url = original_request.url();
 
+    params.history_list_was_cleared =
+        navigation_state->history_list_was_cleared();
+
     // Save some histogram data so we can compute the average memory used per
     // page load of the glyphs.
     UMA_HISTOGRAM_COUNTS_10000("Memory.GlyphPagesPerLoad",
@@ -1767,6 +1775,9 @@ void RenderViewImpl::UpdateURL(WebFrame* frame) {
     else
       params.transition = PAGE_TRANSITION_AUTO_SUBFRAME;
 
+    DCHECK(!navigation_state->history_list_was_cleared());
+    params.history_list_was_cleared = false;
+
     Send(new ViewHostMsg_FrameNavigate(routing_id_, params));
   }
 
@@ -3295,6 +3306,7 @@ NavigationState* RenderViewImpl::CreateNavigationStateFromPending() {
     navigation_state = NavigationState::CreateBrowserInitiated(
         params.page_id,
         params.pending_history_list_offset,
+        params.should_clear_history_list,
         params.transition);
     navigation_state->set_transferred_request_child_id(
         params.transferred_request_child_id);
@@ -3492,6 +3504,8 @@ void RenderViewImpl::didFailProvisionalLoad(WebFrame* frame,
         navigation_state->pending_page_id();
     pending_navigation_params_->pending_history_list_offset =
         navigation_state->pending_history_list_offset();
+    pending_navigation_params_->should_clear_history_list =
+        navigation_state->history_list_was_cleared();
     pending_navigation_params_->transition =
         navigation_state->transition_type();
     pending_navigation_params_->request_time =
diff --git a/content/shell/webkit_test_controller.cc b/content/shell/webkit_test_controller.cc
index d30444d..7af5ef8 100644
--- a/content/shell/webkit_test_controller.cc
+++ b/content/shell/webkit_test_controller.cc
@@ -22,6 +22,7 @@
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/web_contents.h"
+#include "content/public/browser/web_contents_view.h"
 #include "content/shell/shell.h"
 #include "content/shell/shell_browser_context.h"
 #include "content/shell/shell_content_browser_client.h"
@@ -223,6 +224,7 @@ bool WebKitTestController::PrepareForLayoutTest(
     WebContentsObserver::Observe(main_window_->web_contents());
     send_configuration_to_next_host_ = true;
     current_pid_ = base::kNullProcessId;
+    main_window_->LoadURL(test_url);
   } else {
 #if (defined(OS_WIN) && !defined(USE_AURA)) || defined(TOOLKIT_GTK)
     // Shell::SizeTo is not implemented on all platforms.
@@ -238,12 +240,14 @@ bool WebKitTestController::PrepareForLayoutTest(
     OverrideWebkitPrefs(&prefs);
     render_view_host->UpdateWebkitPreferences(prefs);
     SendTestConfiguration();
-    registrar_.Add(this,
-                   NOTIFICATION_NAV_ENTRY_PENDING,
-                   Source<NavigationController>(
-                       &main_window_->web_contents()->GetController()));
+
+    NavigationController::LoadURLParams params(test_url);
+    params.transition_type = PageTransitionFromInt(
+        PAGE_TRANSITION_TYPED | PAGE_TRANSITION_FROM_ADDRESS_BAR);
+    params.should_clear_history_list = true;
+    main_window_->web_contents()->GetController().LoadURLWithParams(params);
+    main_window_->web_contents()->GetView()->Focus();
   }
-  main_window_->LoadURL(test_url);
   main_window_->web_contents()->GetRenderViewHost()->SetActive(true);
   main_window_->web_contents()->GetRenderViewHost()->Focus();
   if (!CommandLine::ForCurrentProcess()->HasSwitch(switches::kNoTimeout)) {
@@ -396,11 +400,6 @@ void WebKitTestController::Observe(int type,
       current_pid_ = base::GetProcId(render_process_host->GetHandle());
       break;
     }
-    case NOTIFICATION_NAV_ENTRY_PENDING: {
-      registrar_.Remove(this, NOTIFICATION_NAV_ENTRY_PENDING, source);
-      main_window_->web_contents()->GetController().PruneAllButActive();
-      break;
-    }
     default:
       NOTREACHED();
   }