Summary: | Reduce use of Deprecated::ScriptXXX classes | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Darin Adler <darin> | ||||||||||||||
Component: | WebCore JavaScript | Assignee: | Darin Adler <darin> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | beidson, buildbot, cdumez, ggaren, rniwa, sam | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
Attachments: |
|
Description
Darin Adler
2016-04-15 11:40:58 PDT
Created attachment 276494 [details]
Patch
Looks like I missed a null check somewhere. 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 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
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 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
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 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
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 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
Created attachment 276514 [details]
Patch
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. Committed r199619: <http://trac.webkit.org/changeset/199619> 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. |