Title: [132417] trunk/Source/WebCore
Revision
132417
Author
[email protected]
Date
2012-10-24 16:17:36 -0700 (Wed, 24 Oct 2012)

Log Message

[V8] WorkerContextExecutionProxy doesn't need to track events
https://bugs.webkit.org/show_bug.cgi?id=100295

Reviewed by Eric Seidel.

This code was added when this code was originally upstreamed as part of
the Chromium port. There doesn't appear to be any reason why
WorkerContextExecutionProxy needs to keep a Vector of raw event
pointers. Those points are likely to become dangling, making the rest
of what this code does very sketchy.

* bindings/v8/V8WorkerContextEventListener.cpp:
(WebCore::V8WorkerContextEventListener::callListenerFunction):
* bindings/v8/WorkerContextExecutionProxy.cpp:
(WebCore::WorkerContextExecutionProxy::dispose):
* bindings/v8/WorkerContextExecutionProxy.h:
(WebCore::WorkerContextExecutionState::WorkerContextExecutionState):
(WorkerContextExecutionProxy):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (132416 => 132417)


--- trunk/Source/WebCore/ChangeLog	2012-10-24 23:01:30 UTC (rev 132416)
+++ trunk/Source/WebCore/ChangeLog	2012-10-24 23:17:36 UTC (rev 132417)
@@ -1,3 +1,24 @@
+2012-10-24  Adam Barth  <[email protected]>
+
+        [V8] WorkerContextExecutionProxy doesn't need to track events
+        https://bugs.webkit.org/show_bug.cgi?id=100295
+
+        Reviewed by Eric Seidel.
+
+        This code was added when this code was originally upstreamed as part of
+        the Chromium port. There doesn't appear to be any reason why
+        WorkerContextExecutionProxy needs to keep a Vector of raw event
+        pointers. Those points are likely to become dangling, making the rest
+        of what this code does very sketchy.
+
+        * bindings/v8/V8WorkerContextEventListener.cpp:
+        (WebCore::V8WorkerContextEventListener::callListenerFunction):
+        * bindings/v8/WorkerContextExecutionProxy.cpp:
+        (WebCore::WorkerContextExecutionProxy::dispose):
+        * bindings/v8/WorkerContextExecutionProxy.h:
+        (WebCore::WorkerContextExecutionState::WorkerContextExecutionState):
+        (WorkerContextExecutionProxy):
+
 2012-10-24  Max Vujovic  <[email protected]>
 
         [CSS Shaders] The mesh should be specified using <column, row> order

Modified: trunk/Source/WebCore/bindings/v8/V8WorkerContextEventListener.cpp (132416 => 132417)


--- trunk/Source/WebCore/bindings/v8/V8WorkerContextEventListener.cpp	2012-10-24 23:01:30 UTC (rev 132416)
+++ trunk/Source/WebCore/bindings/v8/V8WorkerContextEventListener.cpp	2012-10-24 23:17:36 UTC (rev 132417)
@@ -110,9 +110,6 @@
     V8RecursionScope recursionScope(context);
     v8::Local<v8::Value> result = handlerFunction->Call(receiver, 1, parameters);
 
-    if (WorkerContextExecutionProxy* proxy = workerProxy(context))
-        proxy->trackEvent(event);
-
     InspectorInstrumentation::didCallFunction(cookie);
 
     return result;

Modified: trunk/Source/WebCore/bindings/v8/WorkerContextExecutionProxy.cpp (132416 => 132417)


--- trunk/Source/WebCore/bindings/v8/WorkerContextExecutionProxy.cpp	2012-10-24 23:01:30 UTC (rev 132416)
+++ trunk/Source/WebCore/bindings/v8/WorkerContextExecutionProxy.cpp	2012-10-24 23:17:36 UTC (rev 132417)
@@ -99,14 +99,6 @@
 
 void WorkerContextExecutionProxy::dispose()
 {
-    // Detach all events from their JS wrappers.
-    for (size_t eventIndex = 0; eventIndex < m_events.size(); ++eventIndex) {
-        Event* event = m_events[eventIndex];
-        if (forgetV8EventObject(event))
-          event->deref();
-    }
-    m_events.clear();
-
     m_perContextData.clear();
 
     // Dispose the context.
@@ -195,15 +187,6 @@
     return true;
 }
 
-bool WorkerContextExecutionProxy::forgetV8EventObject(Event* event)
-{
-    if (getDOMObjectMap().contains(event)) {
-        getDOMObjectMap().forget(event);
-        return true;
-    }
-    return false;
-}
-
 ScriptValue WorkerContextExecutionProxy::evaluate(const String& script, const String& fileName, const TextPosition& scriptStartPosition, WorkerContextExecutionState* state)
 {
     V8GCController::checkMemoryUsage();
@@ -258,11 +241,6 @@
     m_disableEvalPending = enable ? String() : errorMessage;
 }
 
-void WorkerContextExecutionProxy::trackEvent(Event* event)
-{
-    m_events.append(event);
-}
-
 } // namespace WebCore
 
 #endif // ENABLE(WORKERS)

Modified: trunk/Source/WebCore/bindings/v8/WorkerContextExecutionProxy.h (132416 => 132417)


--- trunk/Source/WebCore/bindings/v8/WorkerContextExecutionProxy.h	2012-10-24 23:01:30 UTC (rev 132416)
+++ trunk/Source/WebCore/bindings/v8/WorkerContextExecutionProxy.h	2012-10-24 23:17:36 UTC (rev 132417)
@@ -49,7 +49,11 @@
     struct WrapperTypeInfo;
 
     struct WorkerContextExecutionState {
-        WorkerContextExecutionState() : hadException(false), lineNumber(0) { }
+        WorkerContextExecutionState()
+            : hadException(false)
+            , lineNumber(0)
+        {
+        }
 
         bool hadException;
         ScriptValue exception;
@@ -60,14 +64,9 @@
 
     class WorkerContextExecutionProxy {
     public:
-        WorkerContextExecutionProxy(WorkerContext*);
+        explicit WorkerContextExecutionProxy(WorkerContext*);
         ~WorkerContextExecutionProxy();
 
-        // Track the event so that we can detach it from the JS wrapper when a worker
-        // terminates. This is needed because we need to be able to dispose these
-        // events and releases references to their event targets: WorkerContext.
-        void trackEvent(Event*);
-
         // Alow use of eval() and is equivalents in scripts.
         void setEvalAllowed(bool enable, const String& errorMessage);
 
@@ -82,17 +81,11 @@
         bool initializeIfNeeded();
         void dispose();
 
-        static bool forgetV8EventObject(Event*);
-
         static const int kWorkerMaxStackSize = 500 * 1024;
 
         WorkerContext* m_workerContext;
         v8::Persistent<v8::Context> m_context;
-
-        Vector<Event*> m_events;
-
         OwnPtr<V8PerContextData> m_perContextData;
-
         String m_disableEvalPending;
     };
 
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to