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

chrome/renderer/blocked_plugin.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/renderer/blocked_plugin.h"
 
 #include "app/l10n_util.h"
 #include "app/resource_bundle.h"
 #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"
 #include "third_party/WebKit/WebKit/chromium/public/WebData.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebFrame.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebMenuItemInfo.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebPluginContainer.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebPoint.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebRegularExpression.h"
 #include "third_party/WebKit/WebKit/chromium/public/WebTextCaseSensitivity.h"
@@ skipping 20 matching lines @@
 static const unsigned kMenuActionLoad = 1;
 static const unsigned kMenuActionRemove = 2;
 
 BlockedPlugin::BlockedPlugin(RenderView* render_view,
                              WebFrame* frame,
                              const webkit::npapi::PluginGroup& info,
                              const WebPluginParams& params,
                              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));
 
   DCHECK(!template_html.empty()) << "unable to load template. ID: "
                                  << template_id;
 
   DictionaryValue values;
   values.SetString("message", message);
   name_ = info.GetGroupName();
   values.SetString("name", name_);
 
   // "t" is the id of the templates root node.
   std::string html_data = jstemplate_builder::GetTemplatesHtml(
       template_html, &values, "t");
 
   plugin_ = webkit::npapi::WebViewPlugin::Create(this,
                                                  preferences,
                                                  html_data,
                                                  GURL(kBlockedPluginDataURL));
-
-  render_view_->RegisterBlockedPlugin(this);
 }
 
 BlockedPlugin::~BlockedPlugin() {
-  render_view_->CustomMenuListenerDestroyed(this);
-  render_view_->UnregisterBlockedPlugin(this);
 }
 
 void BlockedPlugin::BindWebFrame(WebFrame* frame) {
   BindToJavascript(frame, "plugin");
   BindMethod("load", &BlockedPlugin::Load);
   BindMethod("hide", &BlockedPlugin::Hide);
 }
 
 void BlockedPlugin::WillDestroyPlugin() {
   delete this;
@@ skipping 20 matching lines @@
 
   WebMenuItemInfo hide_item;
   hide_item.action = kMenuActionRemove;
   hide_item.enabled = true;
   hide_item.label = WebString::fromUTF8(
       l10n_util::GetStringUTF8(IDS_CONTENT_CONTEXT_PLUGIN_HIDE).c_str());
   custom_items[3] = hide_item;
 
   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) {
     HidePlugin();
   } else {
     NOTREACHED();
   }
 }
 
 void BlockedPlugin::LoadPlugin() {
   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();
     container->reportGeometry();
     plugin_->ReplayReceivedData(new_plugin);
     plugin_->destroy();
   }
 }
 
 void BlockedPlugin::Load(const CppArgumentList& args, CppVariant* result) {
@@ skipping 39 matching lines @@
       if (element.hasAttribute("style")) {
         WebString style_str = element.getAttribute("style");
         if (width_regex.match(style_str) >= 0 &&
             height_regex.match(style_str) >= 0)
           element.setAttribute("style", "display: none;");
       }
     }
   }
 }