Modified: trunk/Source/WebCore/ChangeLog (126445 => 126446)
--- trunk/Source/WebCore/ChangeLog 2012-08-23 17:45:59 UTC (rev 126445)
+++ trunk/Source/WebCore/ChangeLog 2012-08-23 17:51:26 UTC (rev 126446)
@@ -1,3 +1,24 @@
+2012-08-23 Adam Barth <aba...@webkit.org>
+
+ [V8] ScheduledAction is ugly and needs a cleanup
+ https://bugs.webkit.org/show_bug.cgi?id=94784
+
+ Reviewed by Eric Seidel.
+
+ This patch updates ScheduledAction to use modern WebKit machinery, like
+ OwnHandle and Vector.
+
+ * bindings/v8/OwnHandle.h:
+ (OwnHandle):
+ * bindings/v8/ScheduledAction.cpp:
+ (WebCore::ScheduledAction::ScheduledAction):
+ (WebCore::ScheduledAction::~ScheduledAction):
+ (WebCore::ScheduledAction::execute):
+ * bindings/v8/ScheduledAction.h:
+ (WebCore):
+ (ScheduledAction):
+ (WebCore::ScheduledAction::ScheduledAction):
+
2012-08-23 Andrey Kosyakov <ca...@chromium.org>
Web Inspector: dblclick on Timeline overview no longer selects entire timeline range
Modified: trunk/Source/WebCore/bindings/v8/OwnHandle.h (126445 => 126446)
--- trunk/Source/WebCore/bindings/v8/OwnHandle.h 2012-08-23 17:45:59 UTC (rev 126445)
+++ trunk/Source/WebCore/bindings/v8/OwnHandle.h 2012-08-23 17:51:26 UTC (rev 126446)
@@ -32,11 +32,13 @@
#define OwnHandle_h
#include <v8.h>
+#include <wtf/Noncopyable.h>
namespace WebCore {
template<typename T>
class OwnHandle {
+ WTF_MAKE_NONCOPYABLE(OwnHandle);
public:
OwnHandle() { }
Modified: trunk/Source/WebCore/bindings/v8/ScheduledAction.cpp (126445 => 126446)
--- trunk/Source/WebCore/bindings/v8/ScheduledAction.cpp 2012-08-23 17:45:59 UTC (rev 126445)
+++ trunk/Source/WebCore/bindings/v8/ScheduledAction.cpp 2012-08-23 17:51:26 UTC (rev 126446)
@@ -49,49 +49,20 @@
namespace WebCore {
-ScheduledAction::ScheduledAction(v8::Handle<v8::Context> context, v8::Handle<v8::Function> func, int argc, v8::Handle<v8::Value> argv[])
+ScheduledAction::ScheduledAction(v8::Handle<v8::Context> context, v8::Handle<v8::Function> function, int argc, v8::Handle<v8::Value> argv[])
: m_context(context)
+ , m_function(function)
, m_code(String(), KURL(), TextPosition::belowRangePosition())
{
- m_function = v8::Persistent<v8::Function>::New(func);
-
-#ifndef NDEBUG
- V8GCController::registerGlobalHandle(SCHEDULED_ACTION, this, m_function);
-#endif
-
- m_argc = argc;
- if (argc > 0) {
- m_argv = new v8::Persistent<v8::Value>[argc];
- for (int i = 0; i < argc; i++) {
- m_argv[i] = v8::Persistent<v8::Value>::New(argv[i]);
-
-#ifndef NDEBUG
- V8GCController::registerGlobalHandle(SCHEDULED_ACTION, this, m_argv[i]);
-#endif
- }
- } else
- m_argv = 0;
+ m_args.reserveCapacity(argc);
+ for (int i = 0; i < argc; ++i)
+ m_args.append(v8::Persistent<v8::Value>::New(argv[i]));
}
ScheduledAction::~ScheduledAction()
{
- if (m_function.IsEmpty())
- return;
-
-#ifndef NDEBUG
- V8GCController::unregisterGlobalHandle(this, m_function);
-#endif
- m_function.Dispose();
-
- for (int i = 0; i < m_argc; i++) {
-#ifndef NDEBUG
- V8GCController::unregisterGlobalHandle(this, m_argv[i]);
-#endif
- m_argv[i].Dispose();
- }
-
- if (m_argc > 0)
- delete[] m_argv;
+ for (size_t i = 0; i < m_args.size(); ++i)
+ m_args[i].Dispose();
}
void ScheduledAction::execute(ScriptExecutionContext* context)
@@ -102,7 +73,7 @@
return;
if (!frame->script()->canExecuteScripts(AboutToExecuteScript))
return;
- execute(frame->script());
+ execute(frame);
}
#if ENABLE(WORKERS)
else {
@@ -112,47 +83,46 @@
#endif
}
-void ScheduledAction::execute(ScriptController* script)
+void ScheduledAction::execute(Frame* frame)
{
- ASSERT(script->proxy());
-
v8::HandleScope handleScope;
- v8::Handle<v8::Context> v8Context = v8::Local<v8::Context>::New(m_context.get());
- if (v8Context.IsEmpty())
- return; // JS may not be enabled.
+ v8::Handle<v8::Context> context = v8::Local<v8::Context>::New(m_context.get());
+ if (context.IsEmpty())
+ return;
+ v8::Context::Scope scope(context);
+
#if PLATFORM(CHROMIUM)
TRACE_EVENT0("v8", "ScheduledAction::execute");
#endif
- v8::Context::Scope scope(v8Context);
-
- // FIXME: Need to implement timeouts for preempting a long-running script.
- if (!m_function.IsEmpty() && m_function->IsFunction())
- script->callFunction(v8::Persistent<v8::Function>::Cast(m_function), v8Context->Global(), m_argc, m_argv);
+ v8::Handle<v8::Function> function = m_function.get();
+ if (!function.IsEmpty())
+ frame->script()->callFunction(function, context->Global(), m_args.size(), m_args.data());
else
- script->compileAndRunScript(m_code);
+ frame->script()->compileAndRunScript(m_code);
- // The 'proxy' may be invalid at this point since JS could have released the owning Frame.
+ // The frame might be invalid at this point because _javascript_ could have released it.
}
#if ENABLE(WORKERS)
-void ScheduledAction::execute(WorkerContext* workerContext)
+void ScheduledAction::execute(WorkerContext* worker)
{
- // In a Worker, the execution should always happen on a worker thread.
- ASSERT(workerContext->thread()->threadID() == currentThread());
+ ASSERT(worker->thread()->threadID() == currentThread());
- V8RecursionScope recursionScope(workerContext);
- WorkerScriptController* scriptController = workerContext->script();
+ V8RecursionScope recursionScope(worker);
- if (!m_function.IsEmpty() && m_function->IsFunction()) {
+ v8::Handle<v8::Function> function = m_function.get();
+ if (!function.IsEmpty()) {
v8::HandleScope handleScope;
- v8::Handle<v8::Context> v8Context = v8::Local<v8::Context>::New(m_context.get());
- ASSERT(!v8Context.IsEmpty());
- v8::Context::Scope scope(v8Context);
- m_function->Call(v8Context->Global(), m_argc, m_argv);
+
+ v8::Handle<v8::Context> context = v8::Local<v8::Context>::New(m_context.get());
+ ASSERT(!context.IsEmpty());
+ v8::Context::Scope scope(context);
+
+ function->Call(context->Global(), m_args.size(), m_args.data());
} else
- scriptController->evaluate(m_code);
+ worker->script()->evaluate(m_code);
}
#endif
Modified: trunk/Source/WebCore/bindings/v8/ScheduledAction.h (126445 => 126446)
--- trunk/Source/WebCore/bindings/v8/ScheduledAction.h 2012-08-23 17:45:59 UTC (rev 126445)
+++ trunk/Source/WebCore/bindings/v8/ScheduledAction.h 2012-08-23 17:51:26 UTC (rev 126446)
@@ -33,43 +33,40 @@
#include "OwnHandle.h"
#include "ScriptSourceCode.h"
-#include "V8GCController.h"
+#include <v8.h>
#include <wtf/Forward.h>
+#include <wtf/Vector.h>
-#include <v8.h>
-
namespace WebCore {
- class ScriptController;
- class ScriptExecutionContext;
- class WorkerContext;
+class Frame;
+class ScriptExecutionContext;
+class WorkerContext;
- class ScheduledAction {
- public:
- ScheduledAction(v8::Handle<v8::Context>, v8::Handle<v8::Function>, int argc, v8::Handle<v8::Value> argv[]);
- explicit ScheduledAction(v8::Handle<v8::Context> context, const WTF::String& code, const KURL& url = ""
- : m_context(context)
- , m_argc(0)
- , m_argv(0)
- , m_code(code, url)
- {
- }
+class ScheduledAction {
+public:
+ ScheduledAction(v8::Handle<v8::Context>, v8::Handle<v8::Function>, int argc, v8::Handle<v8::Value> argv[]);
- virtual ~ScheduledAction();
- virtual void execute(ScriptExecutionContext*);
+ ScheduledAction(v8::Handle<v8::Context> context, const String& code, const KURL& url = ""
+ : m_context(context)
+ , m_code(code, url)
+ {
+ }
- private:
- void execute(ScriptController*);
+ ~ScheduledAction();
+ void execute(ScriptExecutionContext*);
+
+private:
+ void execute(Frame*);
#if ENABLE(WORKERS)
- void execute(WorkerContext*);
+ void execute(WorkerContext*);
#endif
- OwnHandle<v8::Context> m_context;
- v8::Persistent<v8::Function> m_function;
- int m_argc;
- v8::Persistent<v8::Value>* m_argv;
- ScriptSourceCode m_code;
- };
+ OwnHandle<v8::Context> m_context;
+ OwnHandle<v8::Function> m_function;
+ Vector<v8::Persistent<v8::Value> > m_args;
+ ScriptSourceCode m_code;
+};
} // namespace WebCore
