Remove observer when removing delegate, in Instant.

chrome/browser/instant/instant_loader.cc

Index: chrome/browser/instant/instant_loader.cc
diff --git a/chrome/browser/instant/instant_loader.cc b/chrome/browser/instant/instant_loader.cc
index 1f088885ff6f6e188e9f563851934cb0fb8c2210..5ca8877677c08f072d31e8113f0f8e135a7008be 100644
--- a/chrome/browser/instant/instant_loader.cc
+++ b/chrome/browser/instant/instant_loader.cc
@@ -38,6 +38,10 @@ class InstantLoader::WebContentsDelegateImpl
     return is_pointer_down_from_activate_;
   }
 
+  // Start observing the given |web_contents| instead of whatever is currently
+  // being observed. If |web_contents| is NULL, effectively stops observing.
+  void ObserveContents(content::WebContents* web_contents);
+
   // ConstrainedWindowTabHelperDelegate:
   virtual bool ShouldFocusConstrainedWindow() OVERRIDE;
 
@@ -93,11 +97,15 @@ class InstantLoader::WebContentsDelegateImpl
 
 InstantLoader::WebContentsDelegateImpl::WebContentsDelegateImpl(
     InstantLoader* loader)
-    : content::WebContentsObserver(loader->preview_contents_->web_contents()),
-      loader_(loader),
+    : loader_(loader),
       is_pointer_down_from_activate_(false) {
 }
 
+void InstantLoader::WebContentsDelegateImpl::ObserveContents(
+    content::WebContents* web_contents) {
+  Observe(web_contents);
+}
+
 bool InstantLoader::WebContentsDelegateImpl::ShouldFocusConstrainedWindow() {
   // Return false so that constrained windows are not initially focused. If we
   // did otherwise the preview would prematurely get committed when focus goes
@@ -200,6 +208,13 @@ void InstantLoader::WebContentsDelegateImpl::OnSetSuggestions(
     int page_id,
     const std::vector<string16>& suggestions,
     InstantCompleteBehavior behavior) {
+  DCHECK(loader_->preview_contents() &&
+         loader_->preview_contents_->web_contents());
+  // TODO(sreeram): Remove this 'if' bandaid once bug 141875 is confirmed fixed.
+  if (!loader_->preview_contents() ||
+      !loader_->preview_contents_->web_contents()) {
+    return;
+  }
   content::NavigationEntry* entry = loader_->preview_contents_->web_contents()->
                                         GetController().GetActiveEntry();
   if (entry && page_id == entry->GetPageID()) {
@@ -211,6 +226,13 @@ void InstantLoader::WebContentsDelegateImpl::OnSetSuggestions(
 void InstantLoader::WebContentsDelegateImpl::OnInstantSupportDetermined(
     int page_id,
     bool result) {
+  DCHECK(loader_->preview_contents() &&
+         loader_->preview_contents_->web_contents());
+  // TODO(sreeram): Remove this 'if' bandaid once bug 141875 is confirmed fixed.
+  if (!loader_->preview_contents() ||

[Shishir, 2012/08/10 22:11:15]

How will you know what is fixing the issue: this if or the observer change?

[sreeram, 2012/08/10 22:13:54]

I'll know when debug builds don't crash (the DCHECK should never fire). Of
course, nobody runs debug builds, but perhaps on a try bot...

+      !loader_->preview_contents_->web_contents()) {
+    return;
+  }
   content::NavigationEntry* entry = loader_->preview_contents_->web_contents()->
                                         GetController().GetActiveEntry();
   if (entry && page_id == entry->GetPageID())
@@ -239,8 +261,10 @@ void InstantLoader::WebContentsDelegateImpl
   // If the page doesn't support the Instant API, InstantController schedules
   // the loader for destruction. Stop sending the controller any more messages,
   // by severing the connection from the WebContents to us (its delegate).
-  if (!supports_instant)
+  if (!supports_instant) {
     loader_->preview_contents_->web_contents()->SetDelegate(NULL);
+    ObserveContents(NULL);
+  }
 }
 
 // InstantLoader ---------------------------------------------------------------
@@ -261,8 +285,10 @@ InstantLoader::InstantLoader(InstantLoaderDelegate* delegate,
 }
 
 InstantLoader::~InstantLoader() {
-  if (preview_contents())
+  if (preview_contents()) {
     preview_contents_->web_contents()->SetDelegate(NULL);
+    preview_delegate_->ObserveContents(NULL);
+  }
 }
 
 void InstantLoader::Init() {
@@ -325,6 +351,7 @@ void InstantLoader::SetupPreviewContents() {
   content::WebContents* new_contents = preview_contents_->web_contents();
   WebContentsDelegateImpl* new_delegate = preview_delegate_.get();
   new_contents->SetDelegate(new_delegate);
+  new_delegate->ObserveContents(new_contents);
 
   // Disable popups and such (mainly to avoid losing focus and reverting the
   // preview prematurely).
@@ -353,6 +380,7 @@ void InstantLoader::SetupPreviewContents() {
 void InstantLoader::CleanupPreviewContents() {
   content::WebContents* old_contents = preview_contents_->web_contents();
   old_contents->SetDelegate(NULL);
+  preview_delegate_->ObserveContents(NULL);
 
   preview_contents_->blocked_content_tab_helper()->SetAllContentsBlocked(false);
   preview_contents_->constrained_window_tab_helper()->set_delegate(NULL);