Refactor V8EventListenerHelper for speed and simplicity.

This refactor simplifies the header and puts the work into a
the source file, where the bulk of the work is in a file-local
template (which primarily exists to factor out the listener creation
logic).

This sets up for a number of follow-up CLs, which combined with
this one give a 20% perf improvement on a microbenchmark of adding
and removing event listeners.

Bug: 752127
Change-Id: I3d5b8cea1560833b6ec867d9d35d44318b6db190
Reviewed-on: https://chromium-review.googlesource.com/599427Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#492011}

third_party/WebKit/Source/bindings/core/v8/V8EventListenerHelper.cpp

@@ -31,29 +31,87 @@
 #include "bindings/core/v8/V8EventListenerHelper.h"
 
 #include "bindings/core/v8/V8BindingForCore.h"
+#include "bindings/core/v8/V8ErrorHandler.h"
+#include "bindings/core/v8/V8EventListener.h"
 #include "bindings/core/v8/V8Window.h"
 #include "bindings/core/v8/V8WorkerGlobalScopeEventListener.h"
+#include "platform/bindings/V8PrivateProperty.h"
 
 namespace blink {
+namespace {
 
-EventListener* V8EventListenerHelper::GetEventListener(
+template <typename ListenerType, typename ListenerFactory>
+ListenerType* GetEventListenerInternal(
+    v8::Isolate* isolate,
+    v8::Local<v8::Object> object,
+    const V8PrivateProperty::Symbol& listener_property,
+    ListenerLookupType lookup,
+    const ListenerFactory& listener_factory) {
+  DCHECK(isolate->InContext());
+  v8::Local<v8::Value> listener_value = listener_property.GetOrEmpty(object);
+  ListenerType* listener =
+      listener_value.IsEmpty()
+          ? nullptr
+          : static_cast<ListenerType*>(
+                listener_value.As<v8::External>()->Value());
+  if (listener || lookup == kListenerFindOnly)
+    return listener;
+
+  listener = listener_factory();
+  if (listener)
+    listener_property.Set(object, v8::External::New(isolate, listener));
+
+  return listener;
+}
+
+}  // namespace
+
+// static
+V8EventListener* V8EventListenerHelper::GetEventListener(
     ScriptState* script_state,
     v8::Local<v8::Value> value,
     bool is_attribute,
     ListenerLookupType lookup) {
   RUNTIME_CALL_TIMER_SCOPE(script_state->GetIsolate(),
                            RuntimeCallStats::CounterId::kGetEventListener);
-  if (lookup == kListenerFindOnly) {
-    // Used by EventTarget::removeEventListener, specifically
-    // EventTargetV8Internal::removeEventListenerMethod
-    DCHECK(!is_attribute);
-    return V8EventListenerHelper::ExistingEventListener(value, script_state);
-  }
-  if (ToLocalDOMWindow(script_state->GetContext()))
-    return V8EventListenerHelper::EnsureEventListener<V8EventListener>(
-        value, is_attribute, script_state);
-  return V8EventListenerHelper::EnsureEventListener<
-      V8WorkerGlobalScopeEventListener>(value, is_attribute, script_state);
+
+  if (!value->IsObject())
+    return nullptr;
+  v8::Local<v8::Object> object = value.As<v8::Object>();
+  v8::Isolate* isolate = script_state->GetIsolate();
+  V8PrivateProperty::Symbol listener_property =
+      is_attribute
+          ? V8PrivateProperty::GetV8EventListenerAttributeListener(isolate)
+          : V8PrivateProperty::GetV8EventListenerListener(isolate);
+
+  return GetEventListenerInternal<V8EventListener>(
+      isolate, object, listener_property, lookup,
+      [object, is_attribute, script_state]() {
+        return !ToLocalDOMWindow(script_state->GetContext())
+                   ? V8WorkerGlobalScopeEventListener::Create(
+                         object, is_attribute, script_state)
+                   : V8EventListener::Create(object, is_attribute,
+                                             script_state);
+      });
+}
+
+// static
+V8EventListener* V8EventListenerHelper::EnsureErrorHandler(
+    ScriptState* script_state,
+    v8::Local<v8::Value> value) {
+  if (!value->IsObject())
+    return nullptr;
+  v8::Local<v8::Object> object = value.As<v8::Object>();
+  v8::Isolate* isolate = script_state->GetIsolate();
+  V8PrivateProperty::Symbol listener_property =
+      V8PrivateProperty::GetV8EventListenerAttributeListener(isolate);
+
+  return GetEventListenerInternal<V8EventListener>(
+      isolate, object, listener_property, kListenerFindOrCreate,
+      [object, script_state]() {
+        const bool is_attribute = true;
+        return V8ErrorHandler::Create(object, is_attribute, script_state);
+      });
 }
 
 }  // namespace blink

third_party/WebKit/Source/bindings/core/v8/V8EventListenerHelper.h

@@ -34,12 +34,13 @@
 #include "bindings/core/v8/V8BindingForCore.h"
 #include "bindings/core/v8/V8EventListener.h"
 #include "core/CoreExport.h"
-#include "platform/bindings/V8PrivateProperty.h"
 #include "platform/wtf/Allocator.h"
 #include "v8/include/v8.h"
 
 namespace blink {
 
+class V8EventListener;
+
 enum ListenerLookupType {
   kListenerFindOnly,
   kListenerFindOrCreate,
@@ -51,76 +52,15 @@ class V8EventListenerHelper {
   STATIC_ONLY(V8EventListenerHelper);
 
  public:
-  static V8EventListener* ExistingEventListener(v8::Local<v8::Value> value,
-                                                ScriptState* script_state) {
-    DCHECK(script_state->GetIsolate()->InContext());
-    if (!value->IsObject())
-      return nullptr;
-
-    return FindEventListener(v8::Local<v8::Object>::Cast(value), false,
-                             script_state);
-  }
-
-  template <typename ListenerType>
-  static V8EventListener* EnsureEventListener(v8::Local<v8::Value>,
-                                              bool is_attribute,
-                                              ScriptState*);
-
-  CORE_EXPORT static EventListener* GetEventListener(ScriptState*,
-                                                     v8::Local<v8::Value>,
-                                                     bool is_attribute,
-                                                     ListenerLookupType);
-
- private:
-  static V8EventListener* FindEventListener(v8::Local<v8::Object> object,
-                                            bool is_attribute,
-                                            ScriptState* script_state) {
-    v8::Isolate* isolate = script_state->GetIsolate();
-    v8::HandleScope scope(isolate);
-    DCHECK(isolate->InContext());
-
-    v8::Local<v8::Value> listener =
-        ListenerProperty(isolate, is_attribute).GetOrEmpty(object);
-    if (listener.IsEmpty())
-      return nullptr;
-    return static_cast<V8EventListener*>(
-        v8::External::Cast(*listener)->Value());
-  }
+  CORE_EXPORT static V8EventListener* GetEventListener(ScriptState*,
+                                                       v8::Local<v8::Value>,
+                                                       bool is_attribute,
+                                                       ListenerLookupType);
 
-  static V8PrivateProperty::Symbol ListenerProperty(v8::Isolate* isolate,
-                                                    bool is_attribute) {
-    return is_attribute
-               ? V8PrivateProperty::GetV8EventListenerAttributeListener(isolate)
-               : V8PrivateProperty::GetV8EventListenerListener(isolate);
-  }
+  CORE_EXPORT static V8EventListener* EnsureErrorHandler(ScriptState*,
+                                                         v8::Local<v8::Value>);
 };
 
-template <typename ListenerType>
-V8EventListener* V8EventListenerHelper::EnsureEventListener(
-    v8::Local<v8::Value> value,
-    bool is_attribute,
-    ScriptState* script_state) {
-  v8::Isolate* isolate = script_state->GetIsolate();
-  DCHECK(isolate->InContext());
-  if (!value->IsObject())
-    return nullptr;
-
-  v8::Local<v8::Object> object = v8::Local<v8::Object>::Cast(value);
-
-  V8EventListener* listener =
-      FindEventListener(object, is_attribute, script_state);
-  if (listener)
-    return listener;
-
-  listener = ListenerType::Create(object, is_attribute, script_state);
-  if (listener) {
-    ListenerProperty(isolate, is_attribute)
-        .Set(object, v8::External::New(isolate, listener));
-  }
-
-  return listener;
-}
-
 }  // namespace blink
 
 #endif  // V8EventListenerHelper_h

third_party/WebKit/Source/bindings/scripts/v8_attributes.py

@@ -485,8 +485,8 @@ def setter_expression(interface, attribute, context):
                 attribute.name == 'onerror'):
             includes.add('bindings/core/v8/V8ErrorHandler.h')
             arguments.append(
-                'V8EventListenerHelper::EnsureEventListener<V8ErrorHandler>(' +
-                'v8Value, true, ScriptState::ForReceiverObject(info))')
+                'V8EventListenerHelper::EnsureErrorHandler(' +
+                'ScriptState::ForReceiverObject(info), v8Value)')
         else:
             arguments.append(
                 'V8EventListenerHelper::GetEventListener(' +