[Webkit-unassigned] [Bug 24689] Upstream V8 bindings for Workers proxies.
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Mar 19 10:25:32 PDT 2009
https://bugs.webkit.org/show_bug.cgi?id=24689
dglazkov at chromium.org changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #28742|review? |review-
Flag| |
------- Comment #3 from dglazkov at chromium.org 2009-03-19 10:25 PDT -------
(From update of attachment 28742)
> +#include "v8Binding.h"
V8Binding
> +#include "v8Proxy.h"
V8Proxy
> +#include "v8_index.h"
Ideally, we should create V8Index.h that forwards to v8_index, just like I did
for V8Proxy and friends. But this include isn't needed, right? V8Proxy includes
v8_index.
> + v8::Handle<v8::String> implicit_proto_string = v8::String::New("__proto__");
implicitProtoString?
this method needs to be scrubbed for local var naming.
> + v8::TryCatch try_catch;
tryCatch.
> +v8::Handle<v8::Value> WorkerContextExecutionProxy::ToV8Object(V8ClassIndex::V8WrapperType type, void* impl)
toV8Object
> + if (!impl)
> + return v8::Null();
> +
> + if (type == V8ClassIndex::WORKERCONTEXT)
> + return WorkerContextToV8Object(static_cast<WorkerContext*>(impl));
> +
> + // Non DOM node
> + v8::Persistent<v8::Object> result = GetDOMObjectMap().get(impl);
Perhaps it would be good to add a domObjectMap() helper to V8Proxy, which for
now wraps around GetDOMObjectMap?
> + if (result.IsEmpty()) {
> + v8::Local<v8::Object> v8obj = instantiateV8Object(type, type, impl);
probably just object in this context.
> + v8::Handle<v8::Object> result = instantiateV8Object(type, V8ClassIndex::EVENT, event);
can I recommend renaming instantiateV8Object to toV8? It's a convention I've
seen in JSC (toJS), and it's less verbose.
> + m_listeners.push_back(listener.get());
push_back?! This smells like std::list -- and it is!! We should use Vector
here.
> +#include "v8.h"
<v8.h>
> +#include "v8_index.h"
Really need V8Index.h, then.
> + // Returns a local handle of the context.
> + v8::Local<v8::Context> GetContext() { return v8::Local<v8::Context>::New(m_context); }
context();
> +
> + // Returns the dom constructor function for the given node type.
> + v8::Local<v8::Function> GetConstructor(V8ClassIndex::V8WrapperType);
constructor();
> +
> + // Finds or creates an event listener;
> + PassRefPtr<V8EventListener> FindOrCreateEventListener(v8::Local<v8::Value> listener, bool isInline, bool findOnly);
findOrCreateEventListener
> +
> + // Removes an event listener;
> + void RemoveEventListener(V8EventListener*);
removeEventListener
> +
> + // 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*);
trackEvent
> + // Returns the JS wrapper of object.
> + static v8::Handle<v8::Value> ToV8Object(V8ClassIndex::V8WrapperType type, void* impl);
> + static v8::Handle<v8::Value> EventToV8Object(Event* event);
> + static v8::Handle<v8::Value> EventTargetToV8Object(EventTarget* target);
> + static v8::Handle<v8::Value> WorkerContextToV8Object(WorkerContext* wc);
camelCase for all these.
> + typedef std::list<V8EventListener*> EventListenerList;
Vector<V8EventListener*>, and typedefs typically go on top of all decls.
> diff --git a/WebCore/bindings/v8/WorkerScriptController.cpp b/WebCore/bindings/v8/WorkerScriptController.cpp
> +#include "v8.h"
<v8.h>
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
More information about the webkit-unassigned
mailing list