Commit e348af70 authored by mmenke@chromium.org's avatar mmenke@chromium.org

Prevent DCHECK (And later double deletion) when

PrerenderContents::Destroy() is called in response to
TabContents destruction.

Re-enables PrerenderBrowserTest.PrerenderHttpAuthentication,
which ran into the issue on ChromeOS.

BUG=82835,82913
TEST=PrerenderBrowserTest.PrerenderHttpAuthentication on ChromeOS

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@86339 0039d316-1c4b-4281-b951-d872f2087c98
parent ec7bf890
...@@ -89,7 +89,7 @@ class TestPrerenderContents : public PrerenderContents { ...@@ -89,7 +89,7 @@ class TestPrerenderContents : public PrerenderContents {
" when testing URL " << prerender_url().path(); " when testing URL " << prerender_url().path();
// Prerendering RenderViewHosts should be hidden before the first // Prerendering RenderViewHosts should be hidden before the first
// navigation, so this should be happen for every PrerenderContents for // navigation, so this should be happen for every PrerenderContents for
// while a RenderViewHost is created, regardless of whether or not it's // which a RenderViewHost is created, regardless of whether or not it's
// used. // used.
if (new_render_view_host_) { if (new_render_view_host_) {
EXPECT_TRUE(was_hidden_); EXPECT_TRUE(was_hidden_);
...@@ -519,9 +519,9 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderIframeDelayLoadPlugin) { ...@@ -519,9 +519,9 @@ IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, PrerenderIframeDelayLoadPlugin) {
// Renders a page that contains a prerender link to a page that contains an // Renders a page that contains a prerender link to a page that contains an
// iframe with a source that requires http authentication. This should not // iframe with a source that requires http authentication. This should not
// prerender successfully. // prerender successfully.
// Flaky, and crbug.com was down when discovered, so no crbug entry. // Flaky: http://crbug.com/82913.
IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest, IN_PROC_BROWSER_TEST_F(PrerenderBrowserTest,
DISABLED_PrerenderHttpAuthentication) { FLAKY_PrerenderHttpAuthentication) {
PrerenderTestURL("files/prerender/prerender_http_auth_container.html", PrerenderTestURL("files/prerender/prerender_http_auth_container.html",
FINAL_STATUS_AUTH_NEEDED, FINAL_STATUS_AUTH_NEEDED,
1); 1);
......
...@@ -137,6 +137,7 @@ PrerenderContents::PrerenderContents(PrerenderManager* prerender_manager, ...@@ -137,6 +137,7 @@ PrerenderContents::PrerenderContents(PrerenderManager* prerender_manager,
has_stopped_loading_(false), has_stopped_loading_(false),
final_status_(FINAL_STATUS_MAX), final_status_(FINAL_STATUS_MAX),
prerendering_has_started_(false), prerendering_has_started_(false),
prerendering_has_been_cancelled_(false),
child_id_(-1), child_id_(-1),
route_id_(-1), route_id_(-1),
starting_page_id_(-1) { starting_page_id_(-1) {
...@@ -366,6 +367,9 @@ FinalStatus PrerenderContents::final_status() const { ...@@ -366,6 +367,9 @@ FinalStatus PrerenderContents::final_status() const {
PrerenderContents::~PrerenderContents() { PrerenderContents::~PrerenderContents() {
DCHECK(final_status_ != FINAL_STATUS_MAX); DCHECK(final_status_ != FINAL_STATUS_MAX);
DCHECK(prerendering_has_been_cancelled_ ||
final_status_ == FINAL_STATUS_USED ||
final_status_ == FINAL_STATUS_CONTROL_GROUP);
// If we haven't even started prerendering, we were just in the control // If we haven't even started prerendering, we were just in the control
// group, which means we do not want to record the status. // group, which means we do not want to record the status.
...@@ -712,9 +716,12 @@ void PrerenderContents::DidStopLoading() { ...@@ -712,9 +716,12 @@ void PrerenderContents::DidStopLoading() {
} }
void PrerenderContents::Destroy(FinalStatus final_status) { void PrerenderContents::Destroy(FinalStatus final_status) {
if (prerender_manager_->IsPendingDelete(this)) if (prerendering_has_been_cancelled_)
return; return;
prerendering_has_been_cancelled_ = true;
prerender_manager_->MoveEntryToPendingDelete(this);
if (child_id_ != -1 && route_id_ != -1) { if (child_id_ != -1 && route_id_ != -1) {
// Cancel the prerender in the PrerenderTracker. This is needed // Cancel the prerender in the PrerenderTracker. This is needed
// because destroy may be called directly from the UI thread without calling // because destroy may be called directly from the UI thread without calling
...@@ -732,9 +739,8 @@ void PrerenderContents::Destroy(FinalStatus final_status) { ...@@ -732,9 +739,8 @@ void PrerenderContents::Destroy(FinalStatus final_status) {
NOTREACHED(); NOTREACHED();
} }
} }
prerender_manager_->MoveEntryToPendingDelete(this);
set_final_status(final_status); set_final_status(final_status);
// We may destroy the PrerenderContents before we have initialized the // We may destroy the PrerenderContents before we have initialized the
// RenderViewHost. Otherwise set the Observer's PrerenderContents to NULL to // RenderViewHost. Otherwise set the Observer's PrerenderContents to NULL to
// avoid any more messages being sent. // avoid any more messages being sent.
......
...@@ -339,6 +339,10 @@ class PrerenderContents : public RenderViewHostDelegate, ...@@ -339,6 +339,10 @@ class PrerenderContents : public RenderViewHostDelegate,
bool prerendering_has_started_; bool prerendering_has_started_;
// Tracks whether or not prerendering has been cancelled by calling Destroy.
// Used solely to prevent double deletion.
bool prerendering_has_been_cancelled_;
// Time at which we started to load the URL. This is used to compute // Time at which we started to load the URL. This is used to compute
// the time elapsed from initiating a prerender until the time the // the time elapsed from initiating a prerender until the time the
// (potentially only partially) prerendered page is shown to the user. // (potentially only partially) prerendered page is shown to the user.
......
...@@ -66,7 +66,7 @@ class TestPrerenderManager : public PrerenderManager { ...@@ -66,7 +66,7 @@ class TestPrerenderManager : public PrerenderManager {
virtual ~TestPrerenderManager() { virtual ~TestPrerenderManager() {
if (next_prerender_contents()) { if (next_prerender_contents()) {
next_prerender_contents()->set_final_status( next_prerender_contents_.release()->Destroy(
FINAL_STATUS_MANAGER_SHUTDOWN); FINAL_STATUS_MANAGER_SHUTDOWN);
} }
// Set the final status for all PrerenderContents with an expected final // Set the final status for all PrerenderContents with an expected final
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment