[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 #173219|review?                     |review+
               Flag|                            |




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

> Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:3410
> -        if (frame->script()->windowShell()->context().IsEmpty() && frame->script()->windowShell()->initializeIfNeeded()) {
> -            // initializeIfNeeded may have created a wrapper for the object, retry from the start.
> +        if (frame->script()->initializeMainWorld()) {
> +            // initializeMainWorld may have created a wrapper for the object, retry from the start.

You need to run-bindings-tests --reset-results to avoid turning the bot red.

> Source/WebCore/bindings/v8/ScriptController.cpp:348
> -    return iter == m_isolatedWorlds.end() ? 0 : iter->value;
> +    return iter == m_isolatedWorlds.end() ? 0 : iter->value->isContextInitialized() ? iter->value : 0;

I would probably have broken this down into multiple statements.  Nested ternary operators can be slightly hard to read.

> Source/WebCore/bindings/v8/ScriptController.cpp:369
> +    if (!shell->isContextInitialized()) {
> +        if (shell->initializeIfNeeded()) {

These lines can be combined.  Do we really need to call isContextInitialized?  I would think that initializeIfNeeded would check isContextInitialized because it says "if needed".

> Source/WebCore/bindings/v8/ScriptController.cpp:727
> +    if ((!m_windowShell->isContextInitialized() || !m_windowShell->isGlobalInitialized()) && m_frame->loader()->stateMachine()->creatingInitialEmptyDocument())

This line still seems strange, but I think we should worry about it later.

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