[Webkit-unassigned] [Bug 98108] Correct detection of context type in WorldContextHandle

[bugzilla-daemon at webkit.org]

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





--- Comment #8 from Dan Carney <dcarney at google.com>  2012-10-03 06:14:06 PST ---
(From update of attachment 166596)
View in context: https://bugs.webkit.org/attachment.cgi?id=166596&action=review

>> Source/WebCore/ChangeLog:9
>> +        contexts.
> 
> Can you write a test that demonstrates this issue?

I'm not sure there's a test that can hit this in a predictable manner, but the issue came up because of http://code.google.com/p/chromium/issues/detail?id=150737

>> Source/WebCore/bindings/v8/V8DOMWrapper.cpp:180
>> +WrapperTypeInfo* V8DOMWrapper::domWrapperTypeSlow(v8::Handle<v8::Value> value)
> 
> Adding this function seems like the wrong approach.  We shouldn't be calling this function with a random object.  We're starting from a v8::Context.  We should know how to find the right JS object to call domWrapperType on.

Fair enough. For the case this patch is trying to fix, we want to know if it's V8SharedWorkerContext, V8DedicatedWorkerContext or V8DOMWindow or something else, because the context is some unknown context and I didn't want to call isWrapperOfType multiple times.  But probably, yes, the fix for that particular issue is to use the correct context for IndexedDB deserialization.  I also wanted to use this function in https://bugs.webkit.org/show_bug.cgi?id=96637, where the context is totally unknown, and I've hit assertions using isWrapperOfType.  I was hoping to replace isWrapperOfType with this function.

>> Source/WebCore/bindings/v8/WorldContextHandle.cpp:52
>> +        return;
> 
> This seems to say that we'll always use the UseMainWorld if we call this function while JavaScript is on the stack.

Yeah, that should be !v8::Context::InContext() - I'm on vacation on with a very slow editing environment.  Returning UseMainWorld is the current (albeit unusual) behaviour.

>>> Source/WebCore/bindings/v8/WorldContextHandle.cpp:60
>>> +    if (UNLIKELY(typeInfo == &V8DedicatedWorkerContext::info)) {
>> 
>> UNLIKELY() would not be helpful in .cpp. You can remove it.
> 
> Actually, UNLIKELY and LIKELY don't do anything on the compilers and architectures we use.  We might want to just remove them from the project.

okay

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