Remove observer when removing delegate, in Instant.

chrome/browser/instant/instant_loader.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/instant/instant_loader.h"
 
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "chrome/browser/history/history_types.h"
 #include "chrome/browser/instant/instant_loader_delegate.h"
 #include "chrome/browser/profiles/profile.h"
@@ skipping 20 matching lines @@
       public CoreTabHelperDelegate,
       public content::WebContentsDelegate,
       public content::WebContentsObserver {
  public:
   explicit WebContentsDelegateImpl(InstantLoader* loader);
 
   bool is_pointer_down_from_activate() const {
     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;
 
   // CoreTabHelperDelegate:
   virtual void SwapTabContents(TabContents* old_tc,
                                TabContents* new_tc) OVERRIDE;
 
   // content::WebContentsDelegate:
   virtual bool ShouldSuppressDialogs() OVERRIDE;
   virtual bool ShouldFocusPageAfterCrash() OVERRIDE;
@@ skipping 35 matching lines @@
   InstantLoader* const loader_;
 
   // True if the mouse or a touch pointer is down from an activate.
   bool is_pointer_down_from_activate_;
 
   DISALLOW_COPY_AND_ASSIGN(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
   // to the constrained window.
   return false;
 }
 
 void InstantLoader::WebContentsDelegateImpl::SwapTabContents(
     TabContents* old_tc,
     TabContents* new_tc) {
@@ skipping 82 matching lines @@
                         OnInstantSupportDetermined)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
 }
 
 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()) {
     MaybeSetAndNotifyInstantSupportDetermined(true);
     loader_->loader_delegate_->SetSuggestions(loader_, suggestions, behavior);
   }
 }
 
 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())
     MaybeSetAndNotifyInstantSupportDetermined(result);
 }
 
 
 void InstantLoader::WebContentsDelegateImpl
     ::CommitFromPointerReleaseIfNecessary() {
   if (is_pointer_down_from_activate_) {
     is_pointer_down_from_activate_ = false;
     loader_->loader_delegate_->CommitInstantLoader(loader_);
   }
 }
 
 void InstantLoader::WebContentsDelegateImpl
     ::MaybeSetAndNotifyInstantSupportDetermined(bool supports_instant) {
   // If we already determined that the loader supports Instant, nothing to do.
   if (loader_->supports_instant_)
     return;
 
   loader_->supports_instant_ = supports_instant;
   loader_->loader_delegate_->InstantSupportDetermined(loader_,
                                                       supports_instant);
 
   // 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 ---------------------------------------------------------------
 
 InstantLoader::InstantLoader(InstantLoaderDelegate* delegate,
                              const std::string& instant_url,
                              const TabContents* tab_contents)
     : loader_delegate_(delegate),
       preview_contents_(new TabContents(content::WebContents::Create(
           tab_contents->profile(), NULL, MSG_ROUTING_NONE,
           tab_contents->web_contents(),
           tab_contents->web_contents()->GetController().
                                         GetSessionStorageNamespace()))),
       preview_delegate_(new WebContentsDelegateImpl(
           ALLOW_THIS_IN_INITIALIZER_LIST(this))),
       supports_instant_(false),
       instant_url_(instant_url) {
 }
 
 InstantLoader::~InstantLoader() {
-  if (preview_contents())
+  if (preview_contents()) {
     preview_contents_->web_contents()->SetDelegate(NULL);
+    preview_delegate_->ObserveContents(NULL);
+  }
 }
 
 void InstantLoader::Init() {
   SetupPreviewContents();
   // This HTTP header and value are set on loads that originate from instant.
   const char* const kInstantHeader = "X-Purpose: Instant";
   preview_contents_->web_contents()->GetController().LoadURL(GURL(instant_url_),
       content::Referrer(), content::PAGE_TRANSITION_GENERATED, kInstantHeader);
   preview_contents_->web_contents()->WasHidden();
 }
@@ skipping 42 matching lines @@
     return;
   }
   NOTREACHED();
 #endif
 }
 
 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).
   preview_contents_->blocked_content_tab_helper()->SetAllContentsBlocked(true);
   preview_contents_->constrained_window_tab_helper()->set_delegate(
                                                           new_delegate);
   preview_contents_->content_settings()->SetPopupsBlocked(true);
   preview_contents_->core_tab_helper()->set_delegate(new_delegate);
   if (ThumbnailGenerator* tg = preview_contents_->thumbnail_generator())
     tg->set_enabled(false);
 
 #if defined(OS_MACOSX)
   // If |preview_contents_| does not currently have a RWHV, we will call
   // SetTakesFocusOnlyOnMouseDown() as a result of the RENDER_VIEW_HOST_CHANGED
   // notification.
   if (content::RenderWidgetHostView* rwhv =
           new_contents->GetRenderWidgetHostView()) {
     rwhv->SetTakesFocusOnlyOnMouseDown(true);
   }
   registrar_.Add(this, content::NOTIFICATION_RENDER_VIEW_HOST_CHANGED,
                  content::Source<content::NavigationController>(
                      &new_contents->GetController()));
 #endif
 }
 
 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);
   preview_contents_->content_settings()->SetPopupsBlocked(false);
   preview_contents_->core_tab_helper()->set_delegate(NULL);
   if (ThumbnailGenerator* tg = preview_contents_->thumbnail_generator())
     tg->set_enabled(true);
 
 #if defined(OS_MACOSX)
   if (content::RenderWidgetHostView* rwhv =
@@ skipping 10 matching lines @@
                                            TabContents* new_tc) {
   DCHECK(old_tc == preview_contents_);
   CleanupPreviewContents();
   // We release here without deleting so that the caller still has the
   // responsibility for deleting the TabContents.
   ignore_result(preview_contents_.release());
   preview_contents_.reset(new_tc);
   SetupPreviewContents();
   loader_delegate_->SwappedTabContents(this);
 }