For all JSC types, this makes getOwnPropertyDescriptor redundant. There will be a bit more hacking required in WebCore to remove GOPD whilst maintaining current behaviour. (Current behaviour is in many ways broken, particularly in that GOPD & GOPS are inconsistent, but we should fix incrementally).
Created attachment 209020 [details] Fix
Attachment 209020 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSCallbackObjectFunctions.h', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/runtime/Arguments.cpp', u'Source/JavaScriptCore/runtime/JSActivation.cpp', u'Source/JavaScriptCore/runtime/JSArray.cpp', u'Source/JavaScriptCore/runtime/JSArrayBuffer.cpp', u'Source/JavaScriptCore/runtime/JSArrayBufferView.cpp', u'Source/JavaScriptCore/runtime/JSDataView.cpp', u'Source/JavaScriptCore/runtime/JSFunction.cpp', u'Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h', u'Source/JavaScriptCore/runtime/JSObject.cpp', u'Source/JavaScriptCore/runtime/JSObject.h', u'Source/JavaScriptCore/runtime/JSString.h', u'Source/JavaScriptCore/runtime/JSSymbolTableObject.h', u'Source/JavaScriptCore/runtime/Lookup.cpp', u'Source/JavaScriptCore/runtime/Lookup.h', u'Source/JavaScriptCore/runtime/PropertySlot.h', u'Source/JavaScriptCore/runtime/RegExpObject.cpp', u'Source/JavaScriptCore/runtime/SparseArrayValueMap.cpp', u'Source/WebCore/ChangeLog', u'Source/WebCore/bindings/js/JSCSSStyleDeclarationCustom.cpp', u'Source/WebCore/bindings/js/JSDOMWindowCustom.cpp', u'Source/WebCore/bindings/js/JSHistoryCustom.cpp', u'Source/WebCore/bindings/js/JSLocationCustom.cpp', u'Source/WebCore/bindings/js/JSPluginElementFunctions.cpp', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/bridge/runtime_array.cpp', u'Source/WebCore/bridge/runtime_method.cpp', u'Source/WebCore/bridge/runtime_object.cpp', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/WebProcess/Plugins/Netscape/JSNPObject.cpp']" exit_code: 1 Source/JavaScriptCore/runtime/PropertySlot.h:40: One space before end of line comments [whitespace/comments] [5] Source/JavaScriptCore/runtime/PropertySlot.h:41: One space before end of line comments [whitespace/comments] [5] Source/JavaScriptCore/runtime/PropertySlot.h:42: One space before end of line comments [whitespace/comments] [5] Source/JavaScriptCore/runtime/PropertySlot.h:43: One space before end of line comments [whitespace/comments] [5] Source/JavaScriptCore/runtime/PropertySlot.h:44: One space before end of line comments [whitespace/comments] [5] Total errors found: 5 in 31 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 209020 [details] Fix Attachment 209020 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1470748 New failing tests: http/tests/security/cross-frame-access-history-get.html http/tests/history/cross-origin-replace-history-object-child.html
Created attachment 209021 [details] Archive of layout-test-results from webkit-ews-05 for mac-mountainlion The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: webkit-ews-05 Port: mac-mountainlion Platform: Mac OS X 10.8.4
Comment on attachment 209020 [details] Fix Attachment 209020 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1478394 New failing tests: http/tests/history/cross-origin-replace-history-object-child.html
Created attachment 209024 [details] Archive of layout-test-results from webkit-ews-16 for mac-mountainlion-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: webkit-ews-16 Port: mac-mountainlion-wk2 Platform: Mac OS X 10.8.4
Created attachment 209036 [details] Fix for JSHistory bug - accidental use of null entry!
Attachment 209036 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/JavaScriptCore/API/JSCallbackObjectFunctions.h', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/runtime/Arguments.cpp', u'Source/JavaScriptCore/runtime/JSActivation.cpp', u'Source/JavaScriptCore/runtime/JSArray.cpp', u'Source/JavaScriptCore/runtime/JSArrayBuffer.cpp', u'Source/JavaScriptCore/runtime/JSArrayBufferView.cpp', u'Source/JavaScriptCore/runtime/JSDataView.cpp', u'Source/JavaScriptCore/runtime/JSFunction.cpp', u'Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h', u'Source/JavaScriptCore/runtime/JSObject.cpp', u'Source/JavaScriptCore/runtime/JSObject.h', u'Source/JavaScriptCore/runtime/JSString.h', u'Source/JavaScriptCore/runtime/JSSymbolTableObject.h', u'Source/JavaScriptCore/runtime/Lookup.cpp', u'Source/JavaScriptCore/runtime/Lookup.h', u'Source/JavaScriptCore/runtime/PropertySlot.h', u'Source/JavaScriptCore/runtime/RegExpObject.cpp', u'Source/JavaScriptCore/runtime/SparseArrayValueMap.cpp', u'Source/WebCore/ChangeLog', u'Source/WebCore/bindings/js/JSCSSStyleDeclarationCustom.cpp', u'Source/WebCore/bindings/js/JSDOMWindowCustom.cpp', u'Source/WebCore/bindings/js/JSHistoryCustom.cpp', u'Source/WebCore/bindings/js/JSLocationCustom.cpp', u'Source/WebCore/bindings/js/JSPluginElementFunctions.cpp', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/bridge/runtime_array.cpp', u'Source/WebCore/bridge/runtime_method.cpp', u'Source/WebCore/bridge/runtime_object.cpp', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/WebProcess/Plugins/Netscape/JSNPObject.cpp']" exit_code: 1 Source/JavaScriptCore/runtime/PropertySlot.h:40: One space before end of line comments [whitespace/comments] [5] Source/JavaScriptCore/runtime/PropertySlot.h:41: One space before end of line comments [whitespace/comments] [5] Source/JavaScriptCore/runtime/PropertySlot.h:42: One space before end of line comments [whitespace/comments] [5] Source/JavaScriptCore/runtime/PropertySlot.h:43: One space before end of line comments [whitespace/comments] [5] Source/JavaScriptCore/runtime/PropertySlot.h:44: One space before end of line comments [whitespace/comments] [5] Total errors found: 5 in 31 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 209036 [details] Fix for JSHistory bug - accidental use of null entry! View in context: https://bugs.webkit.org/attachment.cgi?id=209036&action=review Hard to tell by reading whether this is correct, but I approve of this approach, so r=me. > Source/JavaScriptCore/API/JSCallbackObjectFunctions.h:168 > JSValue value = thisObject->getStaticValue(exec, propertyName); > if (value) { > - slot.setValue(thisObject, value); > + slot.setValue(thisObject, ReadOnly | DontEnum, value); > return true; I think static values have their own attributes, no? > Source/JavaScriptCore/API/JSCallbackObjectFunctions.h:177 > if (OpaqueJSClassStaticFunctionsTable* staticFunctions = jsClass->staticFunctions(exec)) { > if (staticFunctions->contains(name)) { > - slot.setCustom(thisObject, staticFunctionGetter); > + slot.setCustom(thisObject, ReadOnly | DontEnum, staticFunctionGetter); > return true; > } Ditto for static functions.
(In reply to comment #9) > Hard to tell by reading whether this is correct, but I approve of this approach, so r=me. Oh - should have mentioned - I tested by adding an assert in objectConstructorGetOwnPropertyDescriptor to ensure GOPS/GOPD return the same results. Results match for all objects other than some WebCore ones (particularly the window object) where the GOPS/GOPD behaviour does not currently match (I'll work on aligning that next). > > Source/JavaScriptCore/API/JSCallbackObjectFunctions.h:168 > > JSValue value = thisObject->getStaticValue(exec, propertyName); > I think static values have their own attributes, no? > > > Source/JavaScriptCore/API/JSCallbackObjectFunctions.h:177 > > if (OpaqueJSClassStaticFunctionsTable* staticFunctions = jsClass->staticFunctions(exec)) { > Ditto for static functions. Yes – but I intend to leave these as is for now. I'd like to separate the refactoring of unifying GOPS/GOPD from any changes in behavior, and whilst you're right that this information does exist for static values, GOPD just ignores it. I'd like to make GOPS do the same for now, and then once I've removed GOPD I'll follow up with a patch to fix the attributes.
Committed revision 154253.