Changeset 126446 in webkit


Ignore:
Timestamp:
Aug 23, 2012 10:51:26 AM (12 years ago)
Author:
abarth@webkit.org
Message:

[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):

Location:
trunk/Source/WebCore
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r126445 r126446  
     12012-08-23  Adam Barth  <abarth@webkit.org>
     2
     3        [V8] ScheduledAction is ugly and needs a cleanup
     4        https://bugs.webkit.org/show_bug.cgi?id=94784
     5
     6        Reviewed by Eric Seidel.
     7
     8        This patch updates ScheduledAction to use modern WebKit machinery, like
     9        OwnHandle and Vector.
     10
     11        * bindings/v8/OwnHandle.h:
     12        (OwnHandle):
     13        * bindings/v8/ScheduledAction.cpp:
     14        (WebCore::ScheduledAction::ScheduledAction):
     15        (WebCore::ScheduledAction::~ScheduledAction):
     16        (WebCore::ScheduledAction::execute):
     17        * bindings/v8/ScheduledAction.h:
     18        (WebCore):
     19        (ScheduledAction):
     20        (WebCore::ScheduledAction::ScheduledAction):
     21
    1222012-08-23  Andrey Kosyakov  <caseq@chromium.org>
    223

  • trunk/Source/WebCore/bindings/v8/OwnHandle.h

    r126376 r126446  
    3333
    3434#include <v8.h>
     35#include <wtf/Noncopyable.h>
    3536
    3637namespace WebCore {
    … …  
    3839template<typename T>
    3940class OwnHandle {
     41    WTF_MAKE_NONCOPYABLE(OwnHandle);
    4042public:
    4143    OwnHandle() { }

  • trunk/Source/WebCore/bindings/v8/ScheduledAction.cpp

    r126399 r126446  
    5050namespace WebCore {
    5151
    52 ScheduledAction::ScheduledAction(v8::Handle<v8::Context> context, v8::Handle<v8::Function> func, int argc, v8::Handle<v8::Value> argv[])
     52ScheduledAction::ScheduledAction(v8::Handle<v8::Context> context, v8::Handle<v8::Function> function, int argc, v8::Handle<v8::Value> argv[])
    5353    : m_context(context)
     54    , m_function(function)
    5455    , m_code(String(), KURL(), TextPosition::belowRangePosition())
    5556{
    56     m_function = v8::Persistent<v8::Function>::New(func);
    57 
    58 #ifndef NDEBUG
    59     V8GCController::registerGlobalHandle(SCHEDULED_ACTION, this, m_function);
    60 #endif
    61 
    62     m_argc = argc;
    63     if (argc > 0) {
    64         m_argv = new v8::Persistent<v8::Value>[argc];
    65         for (int i = 0; i < argc; i++) {
    66             m_argv[i] = v8::Persistent<v8::Value>::New(argv[i]);
    67 
    68 #ifndef NDEBUG
    69     V8GCController::registerGlobalHandle(SCHEDULED_ACTION, this, m_argv[i]);
    70 #endif
    71         }
    72     } else
    73         m_argv = 0;
     57    m_args.reserveCapacity(argc);
     58    for (int i = 0; i < argc; ++i)
     59        m_args.append(v8::Persistent<v8::Value>::New(argv[i]));
    7460}
    7561
    7662ScheduledAction::~ScheduledAction()
    7763{
    78     if (m_function.IsEmpty())
    79         return;
    80 
    81 #ifndef NDEBUG
    82     V8GCController::unregisterGlobalHandle(this, m_function);
    83 #endif
    84     m_function.Dispose();
    85 
    86     for (int i = 0; i < m_argc; i++) {
    87 #ifndef NDEBUG
    88         V8GCController::unregisterGlobalHandle(this, m_argv[i]);
    89 #endif
    90         m_argv[i].Dispose();
    91     }
    92 
    93     if (m_argc > 0)
    94         delete[] m_argv;
     64    for (size_t i = 0; i < m_args.size(); ++i)
     65        m_args[i].Dispose();
    9566}
    9667
    … …  
    10374        if (!frame->script()->canExecuteScripts(AboutToExecuteScript))
    10475            return;
    105         execute(frame->script());
     76        execute(frame);
    10677    }
    10778#if ENABLE(WORKERS)
    … …  
    11384}
    11485
    115 void ScheduledAction::execute(ScriptController* script)
     86void ScheduledAction::execute(Frame* frame)
    11687{
    117     ASSERT(script->proxy());
     88    v8::HandleScope handleScope;
    11889
    119     v8::HandleScope handleScope;
    120     v8::Handle<v8::Context> v8Context = v8::Local<v8::Context>::New(m_context.get());
    121     if (v8Context.IsEmpty())
    122         return; // JS may not be enabled.
     90    v8::Handle<v8::Context> context = v8::Local<v8::Context>::New(m_context.get());
     91    if (context.IsEmpty())
     92        return;
     93    v8::Context::Scope scope(context);
    12394
    12495#if PLATFORM(CHROMIUM)
    … …  
    12697#endif
    12798
    128     v8::Context::Scope scope(v8Context);
     99    v8::Handle<v8::Function> function = m_function.get();
     100    if (!function.IsEmpty())
     101        frame->script()->callFunction(function, context->Global(), m_args.size(), m_args.data());
     102    else
     103        frame->script()->compileAndRunScript(m_code);
    129104
    130     // FIXME: Need to implement timeouts for preempting a long-running script.
    131     if (!m_function.IsEmpty() && m_function->IsFunction())
    132         script->callFunction(v8::Persistent<v8::Function>::Cast(m_function), v8Context->Global(), m_argc, m_argv);
    133     else
    134         script->compileAndRunScript(m_code);
    135 
    136     // The 'proxy' may be invalid at this point since JS could have released the owning Frame.
     105    // The frame might be invalid at this point because JavaScript could have released it.
    137106}
    138107
    139108#if ENABLE(WORKERS)
    140 void ScheduledAction::execute(WorkerContext* workerContext)
     109void ScheduledAction::execute(WorkerContext* worker)
    141110{
    142     // In a Worker, the execution should always happen on a worker thread.
    143     ASSERT(workerContext->thread()->threadID() == currentThread());
     111    ASSERT(worker->thread()->threadID() == currentThread());
    144112
    145     V8RecursionScope recursionScope(workerContext);
    146     WorkerScriptController* scriptController = workerContext->script();
     113    V8RecursionScope recursionScope(worker);
    147114
    148     if (!m_function.IsEmpty() && m_function->IsFunction()) {
     115    v8::Handle<v8::Function> function = m_function.get();
     116    if (!function.IsEmpty()) {
    149117        v8::HandleScope handleScope;
    150         v8::Handle<v8::Context> v8Context = v8::Local<v8::Context>::New(m_context.get());
    151         ASSERT(!v8Context.IsEmpty());
    152         v8::Context::Scope scope(v8Context);
    153         m_function->Call(v8Context->Global(), m_argc, m_argv);
     118
     119        v8::Handle<v8::Context> context = v8::Local<v8::Context>::New(m_context.get());
     120        ASSERT(!context.IsEmpty());
     121        v8::Context::Scope scope(context);
     122
     123        function->Call(context->Global(), m_args.size(), m_args.data());
    154124    } else
    155         scriptController->evaluate(m_code);
     125        worker->script()->evaluate(m_code);
    156126}
    157127#endif

  • trunk/Source/WebCore/bindings/v8/ScheduledAction.h

    r125998 r126446  
    3434#include "OwnHandle.h"
    3535#include "ScriptSourceCode.h"
    36 #include "V8GCController.h"
     36#include <v8.h>
    3737#include <wtf/Forward.h>
    38 
    39 #include <v8.h>
     38#include <wtf/Vector.h>
    4039
    4140namespace WebCore {
    4241
    43     class ScriptController;
    44     class ScriptExecutionContext;
    45     class WorkerContext;
     42class Frame;
     43class ScriptExecutionContext;
     44class WorkerContext;
    4645
    47     class ScheduledAction {
    48     public:
    49         ScheduledAction(v8::Handle<v8::Context>, v8::Handle<v8::Function>, int argc, v8::Handle<v8::Value> argv[]);
    50         explicit ScheduledAction(v8::Handle<v8::Context> context, const WTF::String& code, const KURL& url = KURL())
    51             : m_context(context)
    52             , m_argc(0)
    53             , m_argv(0)
    54             , m_code(code, url)
    55         {
    56         }
     46class ScheduledAction {
     47public:
     48    ScheduledAction(v8::Handle<v8::Context>, v8::Handle<v8::Function>, int argc, v8::Handle<v8::Value> argv[]);
    5749
    58         virtual ~ScheduledAction();
    59         virtual void execute(ScriptExecutionContext*);
     50    ScheduledAction(v8::Handle<v8::Context> context, const String& code, const KURL& url = KURL())
     51        : m_context(context)
     52        , m_code(code, url)
     53    {
     54    }
    6055
    61     private:
    62         void execute(ScriptController*);
     56    ~ScheduledAction();
     57    void execute(ScriptExecutionContext*);
     58
     59private:
     60    void execute(Frame*);
    6361#if ENABLE(WORKERS)
    64         void execute(WorkerContext*);
     62    void execute(WorkerContext*);
    6563#endif
    6664
    67         OwnHandle<v8::Context> m_context;
    68         v8::Persistent<v8::Function> m_function;
    69         int m_argc;
    70         v8::Persistent<v8::Value>* m_argv;
    71         ScriptSourceCode m_code;
    72     };
     65    OwnHandle<v8::Context> m_context;
     66    OwnHandle<v8::Function> m_function;
     67    Vector<v8::Persistent<v8::Value> > m_args;
     68    ScriptSourceCode m_code;
     69};
    7370
    7471} // namespace WebCore
Note: See TracChangeset for help on using the changeset viewer.