Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1442)

Unified Diff: chrome/browser/instant/instant_unload_handler.cc

Issue 12256029: Use scoped_ptr to document ownership in InstantUnloadHandler. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: More correct ownership semantics Created 7 years, 10 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/instant/instant_unload_handler.h ('k') | chrome/browser/ui/browser_instant_controller.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/instant/instant_unload_handler.cc
diff --git a/chrome/browser/instant/instant_unload_handler.cc b/chrome/browser/instant/instant_unload_handler.cc
index c7b580fd2c80966c323473d6cc1c58c2f4bf53d1..fa64ed1e13894fd15485de712c4eb5a3e1e61397 100644
--- a/chrome/browser/instant/instant_unload_handler.cc
+++ b/chrome/browser/instant/instant_unload_handler.cc
@@ -12,42 +12,43 @@
#include "content/public/browser/web_contents.h"
#include "content/public/browser/web_contents_delegate.h"
-// WebContentsDelegate implementation. This owns the WebContents supplied to the
-// constructor.
class InstantUnloadHandler::WebContentsDelegateImpl
: public content::WebContentsDelegate {
public:
WebContentsDelegateImpl(InstantUnloadHandler* handler,
- content::WebContents* contents,
+ scoped_ptr<content::WebContents> contents,
int index)
: handler_(handler),
- contents_(contents),
+ contents_(contents.Pass()),
index_(index) {
- contents->SetDelegate(this);
+ contents_->SetDelegate(this);
+ contents_->GetRenderViewHost()->FirePageBeforeUnload(false);
}
- // content::WebContentsDelegate overrides:
- virtual void WillRunBeforeUnloadConfirm() OVERRIDE {
- content::WebContents* contents = contents_.release();
- contents->SetDelegate(NULL);
- handler_->Activate(this, contents, index_);
+ // Overridden from content::WebContentsDelegate:
+ virtual void CloseContents(content::WebContents* source) OVERRIDE {
+ DCHECK_EQ(contents_, source);
+ // Remove ourselves as the delegate, so that CloseContents() won't be
+ // called twice, leading to double deletion (http://crbug.com/155848).
+ contents_->SetDelegate(NULL);
+ handler_->Destroy(this);
}
- virtual bool ShouldSuppressDialogs() OVERRIDE {
- return true; // Return true so dialogs are suppressed.
+ virtual void WillRunBeforeUnloadConfirm() OVERRIDE {
+ contents_->SetDelegate(NULL);
+ handler_->Activate(this, contents_.Pass(), index_);
}
- virtual void CloseContents(content::WebContents* source) OVERRIDE {
- contents_->SetDelegate(NULL);
- handler_->Destroy(this);
+ virtual bool ShouldSuppressDialogs() OVERRIDE {
+ return true;
}
private:
InstantUnloadHandler* const handler_;
scoped_ptr<content::WebContents> contents_;
- // The index |contents_| was originally at. If we add the tab back we add it
- // at this index.
+ // The tab strip index |contents_| was originally at. If we add the tab back
+ // to the tabstrip, we add it at this index.
const int index_;
DISALLOW_COPY_AND_ASSIGN(WebContentsDelegateImpl);
@@ -61,7 +62,7 @@ InstantUnloadHandler::~InstantUnloadHandler() {
}
void InstantUnloadHandler::RunUnloadListenersOrDestroy(
- content::WebContents* contents,
+ scoped_ptr<content::WebContents> contents,
int index) {
DCHECK(!contents->GetDelegate());
@@ -71,27 +72,25 @@ void InstantUnloadHandler::RunUnloadListenersOrDestroy(
// get here from BrowserInstantController::TabDeactivated, other tab
// observers may still expect to interact with the tab before the event has
// finished propagating.
- MessageLoop::current()->DeleteSoon(FROM_HERE, contents);
+ MessageLoop::current()->DeleteSoon(FROM_HERE, contents.release());
return;
}
- // Tab has before unload listener. Install a delegate and fire the before
- // unload listener.
- delegates_.push_back(new WebContentsDelegateImpl(this, contents, index));
- contents->GetRenderViewHost()->FirePageBeforeUnload(false);
+ // Tab has beforeunload listeners. Install a delegate to run them.
+ delegates_.push_back(
+ new WebContentsDelegateImpl(this, contents.Pass(), index));
}
void InstantUnloadHandler::Activate(WebContentsDelegateImpl* delegate,
- content::WebContents* contents,
+ scoped_ptr<content::WebContents> contents,
int index) {
- chrome::NavigateParams params(browser_, contents);
- params.disposition = NEW_FOREGROUND_TAB;
- params.tabstrip_index = index;
-
// Remove (and delete) the delegate.
Destroy(delegate);
// Add the tab back in.
+ chrome::NavigateParams params(browser_, contents.release());
+ params.disposition = NEW_FOREGROUND_TAB;
+ params.tabstrip_index = index;
chrome::Navigate(&params);
}
« no previous file with comments | « chrome/browser/instant/instant_unload_handler.h ('k') | chrome/browser/ui/browser_instant_controller.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698