Make BlockedPlugin implement RenderViewObserver so that RenderView doesn't ha...

chrome/renderer/blocked_plugin.cc

Index: chrome/renderer/blocked_plugin.cc
===================================================================
--- chrome/renderer/blocked_plugin.cc	(revision 71581)
+++ chrome/renderer/blocked_plugin.cc	(working copy)
@@ -9,6 +9,7 @@
 #include "base/string_piece.h"
 #include "base/values.h"
 #include "chrome/common/jstemplate_builder.h"
+#include "chrome/common/render_messages.h"
 #include "chrome/renderer/render_view.h"
 #include "grit/generated_resources.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebContextMenuData.h"
@@ -49,9 +50,10 @@
                              const WebPreferences& preferences,
                              int template_id,
                              const string16& message)
-    : render_view_(render_view),
+    : RenderViewObserver(render_view),
       frame_(frame),
-      plugin_params_(params) {
+      plugin_params_(params),
+      custom_menu_showing_(false) {
   const base::StringPiece template_html(
       ResourceBundle::GetSharedInstance().GetRawDataResource(template_id));
 
@@ -71,13 +73,9 @@
                                                  preferences,
                                                  html_data,
                                                  GURL(kBlockedPluginDataURL));
-
-  render_view_->RegisterBlockedPlugin(this);
 }
 
 BlockedPlugin::~BlockedPlugin() {
-  render_view_->CustomMenuListenerDestroyed(this);
-  render_view_->UnregisterBlockedPlugin(this);
 }
 
 void BlockedPlugin::BindWebFrame(WebFrame* frame) {
@@ -118,11 +116,25 @@
 
   menu_data.customItems.swap(custom_items);
   menu_data.mousePosition = WebPoint(event.windowX, event.windowY);
-  render_view_->showContextMenu(NULL, menu_data);
-  render_view_->CustomMenuListenerInstall(this);
+  render_view()->showContextMenu(NULL, menu_data);

[Chris Evans, 2011/01/17 13:50:30]

I don't see where this ever gets set to false? Doesn't that mean that
::OnMessageReceived() will start swallowing messages inappropriately?

[jam, 2011/01/17 18:45:19]

This is the same behavior as before, where CustomMenuListenerInstall was called
after showContextMenu, and it would keep swallowing messages forever.

My intent in this change is to clean things up, fixing possible existing bugs is
better to be done in separate changes, since that really helps with tracking
down regressions (i.e. having them be done in different cls).

[jam, 2011/01/17 18:47:46]

oh, wait, nm, I think I understand what you meant now.  before showContextMenu
would reset the listener each time showContextMenu is closed.  let me look into
it, thanks for bringing it up

+  custom_menu_showing_ = true;
 }
 
-void BlockedPlugin::MenuItemSelected(unsigned id) {
+bool BlockedPlugin::OnMessageReceived(const IPC::Message& message) {
+  if (custom_menu_showing_ &&
+      message.type() == ViewMsg_CustomContextMenuAction::ID) {
+    ViewMsg_CustomContextMenuAction::Dispatch(
+        &message, this, this, &BlockedPlugin::OnMenuItemSelected);

[Chris Evans, 2011/01/17 13:50:30]

Perhaps we could guarantee unique IDs for custom menu items, and then only
"grab" this event if it's for one of the IDs we know about? Would avoid worrying
about maintaining correct state for "custom_menu_showing_".

+    return true;
+  }
+
+  if (message.type() == ViewMsg_LoadBlockedPlugins::ID)
+    LoadPlugin();
+
+  return false;  // Want other BlockedPlugin objects to see LoadBlockedPlugins.
+}
+
+void BlockedPlugin::OnMenuItemSelected(unsigned id) {
   if (id == kMenuActionLoad) {
     LoadPlugin();
   } else if (id == kMenuActionRemove) {
@@ -136,8 +148,7 @@
   CHECK(plugin_);
   WebPluginContainer* container = plugin_->container();
   WebPlugin* new_plugin =
-      render_view_->CreatePluginNoCheck(frame_,
-                                        plugin_params_);
+      render_view()->CreatePluginNoCheck(frame_, plugin_params_);
   if (new_plugin && new_plugin->initialize(container)) {
     container->setPlugin(new_plugin);
     container->invalidate();