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

chrome/browser/instant/instant_loader.cc

Index: chrome/browser/instant/instant_loader.cc
===================================================================
--- chrome/browser/instant/instant_loader.cc	(revision 66159)
+++ chrome/browser/instant/instant_loader.cc	(working copy)
@@ -30,6 +30,7 @@
 #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"
@@ -390,6 +391,8 @@
 }
 
 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);
@@ -537,6 +540,10 @@
 #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);
@@ -609,11 +616,33 @@
       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