V8AbstractEventListener should hold a ScriptState

Source/bindings/v8/V8AbstractEventListener.cpp

Index: Source/bindings/v8/V8AbstractEventListener.cpp
diff --git a/Source/bindings/v8/V8AbstractEventListener.cpp b/Source/bindings/v8/V8AbstractEventListener.cpp
index d9b99058814ff9c6dcf7343f6aa549099ac6f7e2..dcd9b00a878e0c017183f632bb89bfa67536bc16 100644
--- a/Source/bindings/v8/V8AbstractEventListener.cpp
+++ b/Source/bindings/v8/V8AbstractEventListener.cpp
@@ -43,10 +43,20 @@
 
 namespace WebCore {
 
-V8AbstractEventListener::V8AbstractEventListener(bool isAttribute, DOMWrapperWorld& world, v8::Isolate* isolate)
+V8AbstractEventListener::V8AbstractEventListener(bool isAttribute, ScriptState* scriptState)
     : EventListener(JSEventListenerType)
     , m_isAttribute(isAttribute)
-    , m_world(world)
+    , m_scriptState(scriptState)
+    , m_isolate(scriptState->isolate())
+{
+    if (isMainThread())
+        InspectorCounters::incrementCounter(InspectorCounters::JSEventListenerCounter);
+}
+
+V8AbstractEventListener::V8AbstractEventListener(bool isAttribute, v8::Isolate* isolate)
+    : EventListener(JSEventListenerType)
+    , m_isAttribute(isAttribute)
+    , m_scriptState(nullptr)
     , m_isolate(isolate)
 {
     if (isMainThread())
@@ -57,7 +67,7 @@ V8AbstractEventListener::~V8AbstractEventListener()
 {
     if (!m_listener.isEmpty()) {
         v8::HandleScope scope(m_isolate);
-        V8EventListenerList::clearWrapper(m_listener.newLocal(m_isolate), m_isAttribute, m_isolate);
+        V8EventListenerList::clearWrapper(m_listener.newLocal(isolate()), m_isAttribute, isolate());
     }
     if (isMainThread())
         InspectorCounters::decrementCounter(InspectorCounters::JSEventListenerCounter);
@@ -75,40 +85,29 @@ void V8AbstractEventListener::handleEvent(ExecutionContext* context, Event* even
     // See issue 889829.
     RefPtr<V8AbstractEventListener> protect(this);
 
-    v8::HandleScope handleScope(m_isolate);
-
-    v8::Local<v8::Context> v8Context = toV8Context(context, world());
-    if (v8Context.IsEmpty())
+    if (scriptState()->contextIsEmpty())
         return;
-
-    // Enter the V8 context in which to perform the event handling.
-    v8::Context::Scope scope(v8Context);
+    ScriptState::Scope scope(scriptState());
 
     // Get the V8 wrapper for the event object.
-    v8::Isolate* isolate = v8Context->GetIsolate();
-    v8::Handle<v8::Value> jsEvent = toV8(event, v8::Handle<v8::Object>(), isolate);
+    v8::Handle<v8::Value> jsEvent = toV8(event, scriptState()->context()->Global(), isolate());
     if (jsEvent.IsEmpty())
         return;
-    invokeEventHandler(context, event, v8::Local<v8::Value>::New(isolate, jsEvent));
+    invokeEventHandler(event, v8::Local<v8::Value>::New(isolate(), jsEvent));
 }
 
 void V8AbstractEventListener::setListenerObject(v8::Handle<v8::Object> listener)
 {
-    m_listener.set(m_isolate, listener);
+    m_listener.set(isolate(), listener);
     m_listener.setWeak(this, &setWeakCallback);
 }
 
-void V8AbstractEventListener::invokeEventHandler(ExecutionContext* context, Event* event, v8::Local<v8::Value> jsEvent)
+void V8AbstractEventListener::invokeEventHandler(Event* event, v8::Local<v8::Value> jsEvent)
 {
     // If jsEvent is empty, attempt to set it as a hidden value would crash v8.
     if (jsEvent.IsEmpty())
         return;
 
-    v8::Local<v8::Context> v8Context = toV8Context(context, world());
-    if (v8Context.IsEmpty())

[adamk, 2014/05/21 10:51:06]

I take it you removed this check because you've updated the callers to do it
instead? Can you then add an ASSERT(!scriptState()->contextIsEmpty())?

[haraken, 2014/05/21 11:15:31]

Yes. Added the ASSERT.

-        return;
-    v8::Isolate* isolate = v8Context->GetIsolate();
-
     v8::Local<v8::Value> returnValue;
     {
         // Catch exceptions thrown in the event handler so they do not propagate to javascript code that caused the event to fire.
@@ -116,29 +115,29 @@ void V8AbstractEventListener::invokeEventHandler(ExecutionContext* context, Even
         tryCatch.SetVerbose(true);
 
         // Save the old 'event' property so we can restore it later.
-        v8::Local<v8::Value> savedEvent = V8HiddenValue::getHiddenValue(v8Context->GetIsolate(), v8Context->Global(), V8HiddenValue::event(isolate));
+        v8::Local<v8::Value> savedEvent = V8HiddenValue::getHiddenValue(isolate(), scriptState()->context()->Global(), V8HiddenValue::event(isolate()));
         tryCatch.Reset();
 
         // Make the event available in the global object, so DOMWindow can expose it.
-        V8HiddenValue::setHiddenValue(v8Context->GetIsolate(), v8Context->Global(), V8HiddenValue::event(isolate), jsEvent);
+        V8HiddenValue::setHiddenValue(isolate(), scriptState()->context()->Global(), V8HiddenValue::event(isolate()), jsEvent);
         tryCatch.Reset();
 
-        returnValue = callListenerFunction(context, jsEvent, event);
+        returnValue = callListenerFunction(scriptState()->executionContext(), jsEvent, event);
         if (tryCatch.HasCaught())
             event->target()->uncaughtExceptionInEventHandler();
 
         if (!tryCatch.CanContinue()) { // Result of TerminateExecution().
-            if (context->isWorkerGlobalScope())
-                toWorkerGlobalScope(context)->script()->forbidExecution();
+            if (scriptState()->executionContext()->isWorkerGlobalScope())
+                toWorkerGlobalScope(scriptState()->executionContext())->script()->forbidExecution();
             return;
         }
         tryCatch.Reset();
 
         // Restore the old event. This must be done for all exit paths through this method.
         if (savedEvent.IsEmpty())
-            V8HiddenValue::setHiddenValue(v8Context->GetIsolate(), v8Context->Global(), V8HiddenValue::event(isolate), v8::Undefined(v8Context->GetIsolate()));
+            V8HiddenValue::setHiddenValue(isolate(), scriptState()->context()->Global(), V8HiddenValue::event(isolate()), v8::Undefined(isolate()));
         else
-            V8HiddenValue::setHiddenValue(v8Context->GetIsolate(), v8Context->Global(), V8HiddenValue::event(isolate), savedEvent);
+            V8HiddenValue::setHiddenValue(isolate(), scriptState()->context()->Global(), V8HiddenValue::event(isolate()), savedEvent);
         tryCatch.Reset();
     }
 
@@ -161,23 +160,22 @@ bool V8AbstractEventListener::shouldPreventDefault(v8::Local<v8::Value> returnVa
     return returnValue->IsBoolean() && !returnValue->BooleanValue();
 }
 
-v8::Local<v8::Object> V8AbstractEventListener::getReceiverObject(ExecutionContext* context, Event* event)
+v8::Local<v8::Object> V8AbstractEventListener::getReceiverObject(Event* event)
 {
-    v8::Isolate* isolate = toV8Context(context, world())->GetIsolate();
-    v8::Local<v8::Object> listener = m_listener.newLocal(isolate);
+    v8::Local<v8::Object> listener = m_listener.newLocal(isolate());
     if (!m_listener.isEmpty() && !listener->IsFunction())
         return listener;
 
     EventTarget* target = event->currentTarget();
-    v8::Handle<v8::Value> value = toV8(target, v8::Handle<v8::Object>(), isolate);
+    v8::Handle<v8::Value> value = toV8(target, scriptState()->context()->Global(), isolate());
     if (value.IsEmpty())
         return v8::Local<v8::Object>();
-    return v8::Local<v8::Object>::New(isolate, v8::Handle<v8::Object>::Cast(value));
+    return v8::Local<v8::Object>::New(isolate(), v8::Handle<v8::Object>::Cast(value));
 }
 
 bool V8AbstractEventListener::belongsToTheCurrentWorld() const
 {
-    return m_isolate->InContext() && m_world == &DOMWrapperWorld::current(m_isolate);
+    return isolate()->InContext() && &world() == &DOMWrapperWorld::current(isolate());
 }
 
 void V8AbstractEventListener::setWeakCallback(const v8::WeakCallbackData<v8::Object, V8AbstractEventListener> &data)