Modifies the Instant loader to listen for RENDER_VIEW_HOST_CHANGED notificati...

chrome/browser/instant/instant_loader.cc

 // Copyright (c) 2010 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 <algorithm>
 #include <utility>
 
 #include "app/l10n_util.h"
@@ skipping 12 matching lines @@
 #include "chrome/browser/search_engines/template_url.h"
 #include "chrome/browser/tab_contents/navigation_controller.h"
 #include "chrome/browser/tab_contents/navigation_entry.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
 #include "chrome/browser/tab_contents/tab_contents_delegate.h"
 #include "chrome/browser/tab_contents/tab_contents_view.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/notification_observer.h"
 #include "chrome/common/notification_registrar.h"
 #include "chrome/common/notification_service.h"
+#include "chrome/common/notification_type.h"
 #include "chrome/common/page_transition_types.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/common/renderer_preferences.h"
 #include "gfx/codec/png_codec.h"
 #include "ipc/ipc_message.h"
 
 namespace {
 // Number of ms to delay before updating the omnibox bounds. This is a bit long
 // as updating the bounds ends up being quite expensive.
 const int kUpdateBoundsDelayMS = 500;
@@ skipping 340 matching lines @@
 
 InstantLoader::InstantLoader(InstantLoaderDelegate* delegate, TemplateURLID id)
     : delegate_(delegate),
       template_url_id_(id),
       ready_(false),
       last_transition_type_(PageTransition::LINK) {
   preview_tab_contents_delegate_.reset(new TabContentsDelegateImpl(this));
 }
 
 InstantLoader::~InstantLoader() {
+  registrar_.RemoveAll();

[sky, 2010/11/16 17:41:05]

Is this needed?

[rohitrao (ping after 24h), 2010/11/16 18:33:59]

General paranoia, because we delete preview_contents_ here as well.  I'll remove
it, unless you think it's safer to keep it.

[sky, 2010/11/16 18:42:29]

You're right, we should keep the explicit remove.

+
   // Delete the TabContents before the delegate as the TabContents holds a
   // reference to the delegate.
   preview_contents_.reset(NULL);
 }
 
 void InstantLoader::Update(TabContents* tab_contents,
                            const TemplateURL* template_url,
                            const GURL& url,
                            PageTransition::Type transition_type,
                            const string16& user_text,
@@ skipping 127 matching lines @@
     if (type != INSTANT_COMMIT_DESTROY)
       preview_tab_contents_delegate_->CommitHistory();
     // Destroy the paint observer.
     // RenderWidgetHostView may be null during shutdown.
     if (preview_contents_->GetRenderWidgetHostView()) {
       preview_contents_->GetRenderWidgetHostView()->GetRenderWidgetHost()->
           set_paint_observer(NULL);
 #if defined(OS_MACOSX)
       preview_contents_->GetRenderWidgetHostView()->
           SetTakesFocusOnlyOnMouseDown(false);
+      registrar_.Remove(
+          this,
+          NotificationType::RENDER_VIEW_HOST_CHANGED,
+          Source<NavigationController>(&preview_contents_->controller()));
 #endif
     }
     preview_contents_->set_delegate(NULL);
     preview_tab_contents_delegate_->Reset();
     ready_ = false;
   }
   update_bounds_timer_.Stop();
   return preview_contents_.release();
 }
 
@@ skipping 52 matching lines @@
 }
 
 void InstantLoader::ShowPreview() {
   if (!ready_) {
     ready_ = true;
 #if defined(OS_MACOSX)
     if (preview_contents_->GetRenderWidgetHostView()) {
       preview_contents_->GetRenderWidgetHostView()->
           SetTakesFocusOnlyOnMouseDown(true);
     }
+    registrar_.Add(

[sky, 2010/11/16 17:41:05]

Any reason you put this here instead of when the preview_contents_ is created?

[rohitrao (ping after 24h), 2010/11/16 18:33:59]

Mostly because the notification is so closely associated with the
SetTakesFocusOnlyOnMouseDown() call a few lines up, and this way I have fewer
OS_MACOSX blocks.

I might be able to move this entire block up to be right after we create the
preview contents.  Let me try that.

+        this,
+        NotificationType::RENDER_VIEW_HOST_CHANGED,
+        Source<NavigationController>(&preview_contents_->controller()));
 #endif
     delegate_->ShowInstantLoader(this);
   }
 }
 
+void InstantLoader::Observe(NotificationType type,
+                            const NotificationSource& source,
+                            const NotificationDetails& details) {
+  switch (type.value) {
+#if defined(OS_MACOSX)
+    case NotificationType::RENDER_VIEW_HOST_CHANGED:
+      LOG(ERROR) << "New RWHV";

[sky, 2010/11/16 17:41:05]

Did you really want the LOG(ERROR)?

[rohitrao (ping after 24h), 2010/11/16 18:33:59]

No, no I don't =)

+      if (preview_contents_->GetRenderWidgetHostView()) {
+        preview_contents_->GetRenderWidgetHostView()->
+            SetTakesFocusOnlyOnMouseDown(true);
+      }
+      break;
+#endif
+    default:
+      NOTREACHED() << "Got a notification we didn't register for.";
+  }
+}
+
 void InstantLoader::PageFinishedLoading() {
   frame_load_observer_.reset();
   // Wait for the user input before showing, this way the page should be up to
   // date by the time we show it.
 }
 
 // TODO(tonyg): This method only fires when the omnibox bounds change. It also
 // needs to fire when the preview bounds change (e.g. open/close info bar).
 gfx::Rect InstantLoader::GetOmniboxBoundsInTermsOfPreview() {
   gfx::Rect preview_bounds(delegate_->GetInstantBounds());
@@ skipping 32 matching lines @@
   if (last_omnibox_bounds_ == omnibox_bounds_)
     return;
 
   last_omnibox_bounds_ = omnibox_bounds_;
   if (preview_contents_.get() && is_showing_instant() &&
       !is_waiting_for_load()) {
     preview_contents_->render_view_host()->SearchBoxResize(
         GetOmniboxBoundsInTermsOfPreview());
   }
 }