Bug 156632 – Reduce use of Deprecated::ScriptXXX classes

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Darin Adler]

Reduce use of Deprecated::ScriptXXX classes

[Darin Adler]

Created attachment 276494 [details]
Patch

[Darin Adler]

Looks like I missed a null check somewhere.

[Build Bot]

Comment on attachment 276494 [details]
Patch

Attachment 276494 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1164770

New failing tests:
fast/dom/javascript-url-exception-isolation.html
fast/frames/sandboxed-iframe-scripting-03.html

[Build Bot]

Created attachment 276499 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 276494 [details]
Patch

Attachment 276494 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.webkit.org/results/1164765

New failing tests:
fast/dom/javascript-url-exception-isolation.html
fast/frames/sandboxed-iframe-scripting-03.html

[Build Bot]

Created attachment 276500 [details]
Archive of layout-test-results from ews104 for mac-yosemite-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-yosemite-wk2  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 276494 [details]
Patch

Attachment 276494 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/1164777

New failing tests:
fast/dom/javascript-url-exception-isolation.html
fast/frames/sandboxed-iframe-scripting-03.html

[Build Bot]

Created attachment 276502 [details]
Archive of layout-test-results from ews114 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 276494 [details]
Patch

Attachment 276494 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/1164830

New failing tests:
fast/dom/javascript-url-exception-isolation.html
fast/frames/sandboxed-iframe-scripting-03.html

[Build Bot]

Created attachment 276503 [details]
Archive of layout-test-results from ews123 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews123  Port: ios-simulator-wk2  Platform: Mac OS X 10.10.5

[Darin Adler]

Created attachment 276514 [details]
Patch

[Alex Christensen]

Comment on attachment 276514 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=276514&action=review

> Source/JavaScriptCore/bindings/ScriptValue.cpp:169
>  RefPtr<InspectorValue> ScriptValue::toInspectorValue(ExecState* scriptState) const
>  {
>      JSLockHolder holder(scriptState);
> -    return jsToInspectorValue(scriptState, m_value.get(), InspectorValue::maxDepth);
> +    return jsToInspectorValue(*scriptState, m_value.get(), InspectorValue::maxDepth);
>  }

There are a lot of places where ExecStates and VMs are passed around as pointers that are assumed to be non-null.  Why don't we use more ExecState&?

> Source/WebKit2/WebProcess/WebPage/WebInspectorUI.cpp:87
> -    ScriptGlobalObject::set(execStateFromPage(mainThreadNormalWorld(), m_page.corePage()), ASCIILiteral("InspectorFrontendHost"), m_frontendHost.get());
> +    ScriptGlobalObject::set(*execStateFromPage(mainThreadNormalWorld(), m_page.corePage()), ASCIILiteral("InspectorFrontendHost"), *m_frontendHost);

This will be dereferencing a null pointer if corePage() returns null.

[Darin Adler]

Committed r199619: <http://trac.webkit.org/changeset/199619>

[Darin Adler]

Comment on attachment 276514 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=276514&action=review

>> Source/JavaScriptCore/bindings/ScriptValue.cpp:169
>>  }
> 
> There are a lot of places where ExecStates and VMs are passed around as pointers that are assumed to be non-null.  Why don't we use more ExecState&?

Yes, there are. We should use more ExecState&. It takes a long time to change all the old code.

>> Source/WebKit2/WebProcess/WebPage/WebInspectorUI.cpp:87
>> +    ScriptGlobalObject::set(*execStateFromPage(mainThreadNormalWorld(), m_page.corePage()), ASCIILiteral("InspectorFrontendHost"), *m_frontendHost);
> 
> This will be dereferencing a null pointer if corePage() returns null.

Yes, was true before, the dereferencing was inside ScriptGlobalObject::set, and it is equally true now.