[Webkit-unassigned] [Bug 100235] [V8] Remove ScriptController::windowShell()

[bugzilla-daemon at webkit.org]

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


Adam Barth <abarth at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #170370|                            |review+
               Flag|                            |




--- Comment #4 from Adam Barth <abarth at webkit.org>  2012-10-24 09:00:11 PST ---
(From update of attachment 170370)
View in context: https://bugs.webkit.org/attachment.cgi?id=170370&action=review

This looks great!  I'm not sure what's going on with the test failure, but you might want to investigate that before landing.

> Source/WebCore/bindings/v8/DOMWrapperWorld.cpp:70
> +    for (WorldMap::iterator i = isolatedWorlds.begin(), e = isolatedWorlds.end(); i != e; ++i)

style nit: we usually use "it" rather than "i" for iterators.  Also, there's no reason to store "e" in a variable.  You can just call end() each time in the loop condition.

> Source/WebCore/bindings/v8/ScriptController.cpp:492
> +    v8::Handle<v8::Context> v8Context = m_windowShell->context();
>      v8Context->AllowCodeGenerationFromStrings(true);

I would combine these lines.

> Source/WebCore/bindings/v8/ScriptController.h:176
>      // Dummy method to avoid a bunch of ifdef's in WebCore.

Is this comment still accurate?

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