Fix crash when swapping in a prerendered page for a page which has an unload handler.

Fix crash when swapping in a prerendered page for a page which has an unload handler. 

***
Note: this is the fix without a browser test, for expediency. A CL with the test is in http://codereview.chromium.org/7046053/
***

The root cause PrerenderManager::OnCloseTabContentsDeleter was never unregistering itself as a TabContentsDelegate in CloseContents. 

r87606 exposed this by changing the TabContents destructor to deference the delegate if it's non-NULL. 

BUG=85350
TEST=None

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=88478

[cbentzel, 2011-06-09 01:53:23]

The browser test did flake on release builds. I want to get this fix in ASAP
since it is _the_ top crasher, and will continue working on a non-flaky browser
test.

[mmenke, 2011-06-09 01:57:15]

LGTM

On 2011/06/09 01:53:23, cbentzel wrote:
> The browser test did flake on release builds. I want to get this fix in ASAP
> since it is _the_ top crasher, and will continue working on a non-flaky
browser
> test.

[dominich, 2011-06-09 01:59:52]

LGTM

Comments on code are nittish so you don't need another LG from me.

The flake doesn't surprise me. We have an issue with wanting to call things
after/during navigate and the message loop quitting in the TestPrerenderContents
destructor /and/ elsewhere is messing us up. This is hitting Shishir's
window.opener tests, my fragment tests, and I think Timo's favicon tests. I'm
working on a way to change the framework for the browser tests to both simplify
the logic and fix this, but it's non-trivial (read: I'm stuck).

http://codereview.chromium.org/7108032/diff/1/chrome/browser/prerender/preren...
File chrome/browser/prerender/prerender_manager.cc (right):

http://codereview.chromium.org/7108032/diff/1/chrome/browser/prerender/preren...
chrome/browser/prerender/prerender_manager.cc:82: virtual void
CloseContents(TabContents* source) OVERRIDE {
if |source| should be |tab_->tab_contents()| here, and I'm not sure that it is,
please CHECK.

http://codereview.chromium.org/7108032/diff/1/chrome/browser/prerender/preren...
chrome/browser/prerender/prerender_manager.cc:83:
tab_->tab_contents()->set_delegate(NULL);
maybe CHECK that |tab_->tab_contents()| is not NULL before calling this.

[cbentzel, 2011-06-09 02:08:54]

Thanks. I agree that there's overall flakiness, although this one surprised me
since the quit was outside of the main browser framework

http://codereview.chromium.org/7108032/diff/1/chrome/browser/prerender/preren...
File chrome/browser/prerender/prerender_manager.cc (right):

http://codereview.chromium.org/7108032/diff/1/chrome/browser/prerender/preren...
chrome/browser/prerender/prerender_manager.cc:82: virtual void
CloseContents(TabContents* source) OVERRIDE {
On 2011/06/09 01:59:52, dominich wrote:
> if |source| should be |tab_->tab_contents()| here, and I'm not sure that it
is,
> please CHECK.

Yes, it should be. Other delegates aren't necessarily restricted to a single
TabContents, but this one is.

[commit-bot: I haz the power, 2011-06-09 03:01:19]

Change committed as 88478