[Webkit-unassigned] [Bug 27628] [v8] Simplify weak handlers callback

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=27628





--- Comment #11 from anton muhin <antonm at chromium.org>  2009-07-24 04:16:10 PDT ---
(In reply to comment #9)
> (From update of attachment 33415 [details])
> This patch is still incorrect.
> 
> -    v8::HandleScope scope;
> 
> Don't we need a handle scope before talking to handles?

We don't create any handles and thus I don't think we need it, but, please,
double check.

> 
> -    ASSERT(v8Object->IsObject());
> 
> Why drop this assert?

I can easily revive it.  My thought was it was originally necessary due to the
cast below (which would check the type anyway, btw).  No I don't do that cast
and hence why ASSERT?

> 
> +        ASSERT(store->domData()->owningThread() == WTF::currentThread());
> 
> This assert is bogus.  Some of DOMDataStores belong to worker threads.
> 
> Here's what you should do:
> 
> 1) Acquire the mutex before iterating through the list because items might be
> inserted or removed on another thread.
> 
> 2) Move the ASSERT to after
> 
> +        if (it == domMapImpl.end() || it->second != *v8Object)
> 
> At that point, you've found a DOMDataStore that holds a Node and it must be on
> the main thread.

You're absolutely right, I wasn't attentive enough.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.