browser-plugin: Remove event handling code, and use CustomEvents in webkit instead.

content/renderer/browser_plugin/browser_plugin.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 "content/renderer/browser_plugin/browser_plugin.h"
 
 #include "base/message_loop.h"
 #include "base/string_util.h"
 #if defined (OS_WIN)
 #include "base/sys_info.h"
 #endif
 #include "base/utf_string_conversions.h"
 #include "content/common/browser_plugin_messages.h"
 #include "content/common/view_messages.h"
 #include "content/public/common/content_client.h"
 #include "content/public/renderer/content_renderer_client.h"
 #include "content/renderer/browser_plugin/browser_plugin_bindings.h"
 #include "content/renderer/browser_plugin/browser_plugin_manager.h"
 #include "content/renderer/render_process_impl.h"
 #include "content/renderer/render_thread_impl.h"
+#include "content/renderer/v8_value_converter_impl.h"
 #include "skia/ext/platform_canvas.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebBindings.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebDocument.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebElement.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebFrame.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebInputEvent.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebPluginContainer.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebPluginParams.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebRect.h"
+#include "third_party/WebKit/Source/WebKit/chromium/public/WebScriptSource.h"
 #include "webkit/plugins/sad_plugin.h"
 
 using WebKit::WebCanvas;
 using WebKit::WebPlugin;
 using WebKit::WebPluginContainer;
 using WebKit::WebPluginParams;
 using WebKit::WebPoint;
 using WebKit::WebString;
 using WebKit::WebRect;
 using WebKit::WebURL;
 using WebKit::WebVector;
 
 namespace content {
 
 namespace {
 const char kExitEventName[] = "exit";
 const char kIsTopLevel[] = "isTopLevel";
 const char kLoadAbortEventName[] = "loadabort";
 const char kLoadCommitEventName[] = "loadcommit";
 const char kLoadRedirectEventName[] = "loadredirect";
 const char kLoadStartEventName[] = "loadstart";
 const char kLoadStopEventName[] = "loadstop";
 const char kNewURL[] = "newUrl";
 const char kOldURL[] = "oldUrl";
 const char kPartitionAttribute[] = "partition";
 const char kPersistPrefix[] = "persist:";
 const char kProcessId[] = "processId";
 const char kSrcAttribute[] = "src";
-const char kType[] = "type";
+const char kReason[] = "reason";
 const char kURL[] = "url";
 
 static std::string TerminationStatusToString(base::TerminationStatus status) {
   switch (status) {
     case base::TERMINATION_STATUS_NORMAL_TERMINATION:
       return "normal";
     case base::TERMINATION_STATUS_ABNORMAL_TERMINATION:
       return "abnormal";
     case base::TERMINATION_STATUS_PROCESS_WAS_KILLED:
       return "killed";
@@ skipping 29 matching lines @@
       process_id_(-1),
       persist_storage_(false),
       content_window_routing_id_(MSG_ROUTING_NONE),
       focused_(false),
       visible_(true),
       current_nav_entry_index_(0),
       nav_entry_count_(0) {
   BrowserPluginManager::Get()->AddBrowserPlugin(instance_id, this);
   bindings_.reset(new BrowserPluginBindings(this));
 
-  InitializeEvents();
-
   ParseAttributes(params);
 }
 
 BrowserPlugin::~BrowserPlugin() {
   if (damage_buffer_)
     FreeDamageBuffer();
-  RemoveEventListeners();
   BrowserPluginManager::Get()->RemoveBrowserPlugin(instance_id_);
   BrowserPluginManager::Get()->Send(
       new BrowserPluginHostMsg_PluginDestroyed(
           render_view_routing_id_,
           instance_id_));
 }
 
 void BrowserPlugin::Cleanup() {
   if (damage_buffer_)
     FreeDamageBuffer();
@@ skipping 179 matching lines @@
   // true, which prevents changing the 'partition' attribute.
   SetSrcAttribute(src);
 }
 
 float BrowserPlugin::GetDeviceScaleFactor() const {
   if (!render_view_)
     return 1.0f;
   return render_view_->GetWebView()->deviceScaleFactor();
 }
 
-void BrowserPlugin::InitializeEvents() {
-  event_listener_map_[kExitEventName] = EventListeners();
-  event_listener_map_[kLoadAbortEventName] = EventListeners();
-  event_listener_map_[kLoadCommitEventName] = EventListeners();
-  event_listener_map_[kLoadRedirectEventName] = EventListeners();
-  event_listener_map_[kLoadStartEventName] = EventListeners();
-  event_listener_map_[kLoadStopEventName] = EventListeners();
-}
+void BrowserPlugin::TriggerEvent(const std::string& event_name,
+                                 std::map<std::string, base::Value*>* props) {
+  WebKit::WebElement plugin = container()->element();
+  v8::HandleScope handle_scope;
+  v8::Local<v8::Context> context =
+      plugin.document().frame()->mainWorldScriptContext();

[abarth-chromium, 2012/11/05 19:07:15]

Why is the main world the correct world?

[sadrul, 2012/11/06 03:23:34]

The current code uses the main-world context, so I am using the same here. But
should this be different?

+  v8::Context::Scope context_scope(context);
+  std::string script = base::StringPrintf("new CustomEvent('%s', {"
+                                          "  'bubbles': false, "
+                                          "  'cancelable': false"
+                                          "});", event_name.c_str());

[abarth-chromium, 2012/11/05 19:07:15]

Where does event_name come from?  Could this lead to XSS?

[sadrul, 2012/11/06 03:23:34]

I have changed this code to use WebDOMCustomEvent instead. But if you think
taking an enum as an input-parameter and deciding the event-name from that would
still be better, I can make that change.

+  v8::Local<v8::Value> value = v8::Local<v8::Value>::New(
+      plugin.document().frame()->executeScriptAndReturnValue(
+        WebKit::WebScriptSource(WebKit::WebString::fromUTF8(script.c_str()))));
+  v8::Local<v8::Object> event = value->ToObject();

[abarth-chromium, 2012/11/05 19:07:15]

Why not use the WebKit API to create a CustomEvent?  It seems like a hack to use
eval to do it.

[sadrul, 2012/11/06 03:23:34]

Done: https://bugs.webkit.org/show_bug.cgi?id=101293

 
-void BrowserPlugin::RemoveEventListeners() {
-  EventListenerMap::iterator event_listener_map_iter =
-      event_listener_map_.begin();
-  for (; event_listener_map_iter != event_listener_map_.end();
-       ++event_listener_map_iter) {
-    EventListeners& listeners =
-        event_listener_map_[event_listener_map_iter->first];
-    EventListeners::iterator it = listeners.begin();
-    for (; it != listeners.end(); ++it) {
-      it->Dispose();
+  if (props) {
+    V8ValueConverterImpl converter;
+    for (std::map<std::string, base::Value*>::iterator iter = props->begin(),
+        end = props->end(); iter != end; ++iter) {
+      event->Set(v8::String::New(iter->first.data(), iter->first.size()),
+          converter.ToV8Value(iter->second, context),
+          v8::ReadOnly);

[abarth-chromium, 2012/11/05 19:07:15]

This operation can trigger JavaScript setters, which can do arbitrary things
like delete the BrowserPlugin.

     }
-  }
-  event_listener_map_.clear();
-}
-
-bool BrowserPlugin::IsValidEvent(const std::string& event_name) {
-  return event_listener_map_.find(event_name) != event_listener_map_.end();
-}
-
-void BrowserPlugin::TriggerEvent(const std::string& event_name,
-                                 v8::Local<v8::Object>* event) {
-  WebKit::WebElement plugin = container()->element();
-
-  const EventListeners& listeners = event_listener_map_[event_name.c_str()];
-  // A v8::Local copy of the listeners is created from the v8::Persistent
-  // listeners before firing them. This is to ensure that if one of these
-  // listeners mutate the list of listeners (by calling
-  // addEventListener/removeEventListener), this local copy is not affected.
-  // This means if you mutate the list of listeners for an event X while event X
-  // is firing, the mutation is deferred until all current listeners for X have
-  // fired.
-  EventListenersLocal listeners_local;
-  listeners_local.reserve(listeners.size());
-  for (EventListeners::const_iterator it = listeners.begin();
-       it != listeners.end();
-       ++it) {
-    listeners_local.push_back(v8::Local<v8::Function>::New(*it));
+    STLDeleteValues(props);
   }
 
-  (*event)->Set(v8::String::New("name"),
-                v8::String::New(event_name.c_str(), event_name.size()),
-                v8::ReadOnly);
-  v8::Local<v8::Value> argv[] = { *event };
-  for (EventListenersLocal::const_iterator it = listeners_local.begin();
-       it != listeners_local.end();
-       ++it) {
-    WebKit::WebFrame* frame = plugin.document().frame();
-    if (!frame)
-      break;
-    frame->callFunctionEvenIfScriptDisabled(*it, v8::Object::New(), 1, argv);
-  }
+  NPVariant npevent, npresult;
+  WebKit::WebBindings::toNPVariant(event, NULL, &npevent);
+
+  NPIdentifier method =
+      WebKit::WebBindings::getStringIdentifier("dispatchEvent");
+  WebKit::WebBindings::invoke(NULL, container()->scriptableObjectForElement(),
+      method, &npevent, 1, &npresult);
+
+  WebKit::WebBindings::releaseVariantValue(&npevent);
+  WebKit::WebBindings::releaseVariantValue(&npresult);
 }
 
 void BrowserPlugin::Back() {
   if (!navigate_src_sent_)
     return;
   BrowserPluginManager::Get()->Send(
       new BrowserPluginHostMsg_Go(render_view_routing_id_,
                                   instance_id_, -1));
 }
 
@@ skipping 82 matching lines @@
   BrowserPluginManager::Get()->Send(new BrowserPluginHostMsg_UpdateRect_ACK(
       render_view_routing_id_,
       instance_id_,
       message_id,
       gfx::Size()));
 }
 
 void BrowserPlugin::GuestGone(int process_id, base::TerminationStatus status) {
   // We fire the event listeners before painting the sad graphic to give the
   // developer an opportunity to display an alternative overlay image on crash.
-  if (HasListeners(kExitEventName)) {
-    WebKit::WebElement plugin = container()->element();
-    v8::HandleScope handle_scope;
-    v8::Context::Scope context_scope(
-        plugin.document().frame()->mainWorldScriptContext());
+  std::string termination_status = TerminationStatusToString(status);
+  std::map<std::string, base::Value*> props;
+  props[kProcessId] = base::Value::CreateIntegerValue(process_id);
+  props[kReason] = base::Value::CreateStringValue(termination_status);
 
-    // Construct the exit event object.
-    v8::Local<v8::Object> event = v8::Object::New();
-    event->Set(v8::String::New(kProcessId, sizeof(kProcessId) - 1),
-               v8::Integer::New(process_id),
-               v8::ReadOnly);
-    std::string termination_status = TerminationStatusToString(status);
-    event->Set(v8::String::New(kType, sizeof(kType) - 1),
-               v8::String::New(termination_status.data(),
-                               termination_status.size()),
-               v8::ReadOnly);
-    // Event listeners may remove the BrowserPlugin from the document. If that
-    // happens, the BrowserPlugin will be scheduled for later deletion (see
-    // BrowserPlugin::destroy()). That will clear the container_ reference,
-    // but leave other member variables valid below.
-    TriggerEvent(kExitEventName, &event);
-  }
+  // Event listeners may remove the BrowserPlugin from the document. If that
+  // happens, the BrowserPlugin will be scheduled for later deletion (see
+  // BrowserPlugin::destroy()). That will clear the container_ reference,
+  // but leave other member variables valid below.
+  TriggerEvent(kExitEventName, &props);

[abarth-chromium, 2012/11/05 19:07:15]

It looks like you're using constants.  Can we pass the name of the event as an
enum value and then have the callee look up the string so that we know it is
always something that won't cause XSS?

+
   guest_crashed_ = true;
   // We won't paint the contents of the current backing store again so we might
   // as well toss it out and save memory.
   backing_store_.reset();
   // If the BrowserPlugin is scheduled to be deleted, then container_ will be
   // NULL so we shouldn't attempt to access it.
   if (container_)
     container_->invalidate();
 }
 
 void BrowserPlugin::LoadStart(const GURL& url, bool is_top_level) {
-  if (!HasListeners(kLoadStartEventName))
-    return;
+  std::map<std::string, base::Value*> props;
+  props[kURL] = base::Value::CreateStringValue(url.spec());
+  props[kIsTopLevel] = base::Value::CreateBooleanValue(is_top_level);
 
-  WebKit::WebElement plugin = container()->element();
-  v8::HandleScope handle_scope;
-  v8::Context::Scope context_scope(
-      plugin.document().frame()->mainWorldScriptContext());
-
-  // Construct the loadStart event object.
-  v8::Local<v8::Object> event = v8::Object::New();
-  event->Set(v8::String::New(kURL, sizeof(kURL) - 1),
-             v8::String::New(url.spec().data(), url.spec().size()),
-             v8::ReadOnly);
-  event->Set(v8::String::New(kIsTopLevel, sizeof(kIsTopLevel) - 1),
-             v8::Boolean::New(is_top_level),
-             v8::ReadOnly);
-  TriggerEvent(kLoadStartEventName, &event);
+  TriggerEvent(kLoadStartEventName, &props);
 }
 
 void BrowserPlugin::LoadCommit(
     const BrowserPluginMsg_LoadCommit_Params& params) {
   // If the guest has just committed a new navigation then it is no longer
   // crashed.
   guest_crashed_ = false;
   src_ = params.url.spec();
   process_id_ = params.process_id;
   current_nav_entry_index_ = params.current_entry_index;
   nav_entry_count_ = params.entry_count;
 
-  if (!HasListeners(kLoadCommitEventName))
-    return;
-
-  WebKit::WebElement plugin = container()->element();
-  v8::HandleScope handle_scope;
-  v8::Context::Scope context_scope(
-      plugin.document().frame()->mainWorldScriptContext());
-
-  // Construct the loadCommit event object.
-  v8::Local<v8::Object> event = v8::Object::New();
-  event->Set(v8::String::New(kURL, sizeof(kURL) - 1),
-             v8::String::New(src_.data(), src_.size()),
-             v8::ReadOnly);
-  event->Set(v8::String::New(kIsTopLevel, sizeof(kIsTopLevel) - 1),
-             v8::Boolean::New(params.is_top_level),
-             v8::ReadOnly);
-
-  TriggerEvent(kLoadCommitEventName, &event);
+  std::map<std::string, base::Value*> props;
+  props[kURL] = base::Value::CreateStringValue(src_);
+  props[kIsTopLevel] = base::Value::CreateBooleanValue(params.is_top_level);
+  TriggerEvent(kLoadCommitEventName, &props);
 }
 
 void BrowserPlugin::LoadStop() {
-  if (!HasListeners(kLoadStopEventName))
-    return;
-
-  WebKit::WebElement plugin = container()->element();
-  v8::HandleScope handle_scope;
-  v8::Context::Scope context_scope(
-      plugin.document().frame()->mainWorldScriptContext());
-
   // Construct the loadStop event object.
-  v8::Local<v8::Object> event = v8::Object::New();
-  TriggerEvent(kLoadStopEventName, &event);
+  TriggerEvent(kLoadStopEventName, NULL);
 }
 
 void BrowserPlugin::LoadAbort(const GURL& url,
                               bool is_top_level,
                               const std::string& type) {
-  if (!HasListeners(kLoadAbortEventName))
-    return;
-
-  WebKit::WebElement plugin = container()->element();
-  v8::HandleScope handle_scope;
-  v8::Context::Scope context_scope(
-      plugin.document().frame()->mainWorldScriptContext());
-
-  // Construct the loadAbort event object.
-  v8::Local<v8::Object> event = v8::Object::New();
-  event->Set(v8::String::New(kURL, sizeof(kURL) - 1),
-             v8::String::New(url.spec().data(), url.spec().size()),
-             v8::ReadOnly);
-  event->Set(v8::String::New(kIsTopLevel, sizeof(kIsTopLevel) - 1),
-             v8::Boolean::New(is_top_level),
-             v8::ReadOnly);
-  event->Set(v8::String::New(kType, sizeof(kType) - 1),
-             v8::String::New(type.data(), type.size()),
-             v8::ReadOnly);
-
-  TriggerEvent(kLoadAbortEventName, &event);
+  std::map<std::string, base::Value*> props;
+  props[kURL] = base::Value::CreateStringValue(url.spec());
+  props[kIsTopLevel] = base::Value::CreateBooleanValue(is_top_level);
+  props[kReason] = base::Value::CreateStringValue(type);
+  TriggerEvent(kLoadAbortEventName, &props);
 }
 
 void BrowserPlugin::LoadRedirect(const GURL& old_url,
                                  const GURL& new_url,
                                  bool is_top_level) {
-  if (!HasListeners(kLoadRedirectEventName))
-    return;
-
-  WebKit::WebElement plugin = container()->element();
-  v8::HandleScope handle_scope;
-  v8::Context::Scope context_scope(
-      plugin.document().frame()->mainWorldScriptContext());
-
-  // Construct the loadRedirect event object.
-  v8::Local<v8::Object> event = v8::Object::New();
-  event->Set(v8::String::New(kOldURL, sizeof(kOldURL) - 1),
-             v8::String::New(old_url.spec().data(), old_url.spec().size()),
-             v8::ReadOnly);
-  event->Set(v8::String::New(kNewURL, sizeof(kNewURL) - 1),
-             v8::String::New(new_url.spec().data(), new_url.spec().size()),
-             v8::ReadOnly);
-  event->Set(v8::String::New(kIsTopLevel, sizeof(kIsTopLevel) - 1),
-             v8::Boolean::New(is_top_level),
-             v8::ReadOnly);
-
-  TriggerEvent(kLoadRedirectEventName, &event);
+  std::map<std::string, base::Value*> props;
+  props[kOldURL] = base::Value::CreateStringValue(old_url.spec());
+  props[kNewURL] = base::Value::CreateStringValue(new_url.spec());
+  props[kIsTopLevel] = base::Value::CreateBooleanValue(is_top_level);
+  TriggerEvent(kLoadRedirectEventName, &props);
 }
 
 void BrowserPlugin::AdvanceFocus(bool reverse) {
   // We do not have a RenderView when we are testing.
   if (render_view_)
     render_view_->GetWebView()->advanceFocus(reverse);
 }
 
 void BrowserPlugin::GuestContentWindowReady(int content_window_routing_id) {
   DCHECK(content_window_routing_id != MSG_ROUTING_NONE);
   content_window_routing_id_ = content_window_routing_id;
 }
 
 void BrowserPlugin::SetAcceptTouchEvents(bool accept) {
   if (container())
     container()->setIsAcceptingTouchEvents(accept);
 }
 
-bool BrowserPlugin::HasListeners(const std::string& event_name) {
-  return IsValidEvent(event_name) &&
-         !event_listener_map_[event_name].empty();
-}
-
-bool BrowserPlugin::AddEventListener(const std::string& event_name,
-                                     v8::Local<v8::Function> function) {
-  if (!IsValidEvent(event_name))
-    return false;
-  EventListeners& listeners = event_listener_map_[event_name];
-  for (unsigned int i = 0; i < listeners.size(); ++i) {
-    if (listeners[i] == function)
-      return false;
-  }
-  v8::Persistent<v8::Function> persistent_function =
-      v8::Persistent<v8::Function>::New(function);
-  listeners.push_back(persistent_function);
-  return true;
-}
-
-bool BrowserPlugin::RemoveEventListener(const std::string& event_name,
-                                        v8::Local<v8::Function> function) {
-  if (!HasListeners(event_name))
-    return false;
-
-  EventListeners& listeners = event_listener_map_[event_name];
-  EventListeners::iterator it = listeners.begin();
-  for (; it != listeners.end(); ++it) {
-    if (*it == function) {
-      it->Dispose();
-      listeners.erase(it);
-      return true;
-    }
-  }
-  return false;
-}
-
 WebKit::WebPluginContainer* BrowserPlugin::container() const {
   return container_;
 }
 
 bool BrowserPlugin::initialize(WebPluginContainer* container) {
   container_ = container;
   return true;
 }
 
 void BrowserPlugin::destroy() {
@@ skipping 265 matching lines @@
                                                  void* notify_data) {
 }
 
 void BrowserPlugin::didFailLoadingFrameRequest(
     const WebKit::WebURL& url,
     void* notify_data,
     const WebKit::WebURLError& error) {
 }
 
 }  // namespace content