This can now typically implemented in terms of getOwnPropertySlot. Add a macro to PropertyDescriptor to define an implementation of GOPD in terms of GOPS. Switch over most classes in JSC & the WebCore bindings generator to use this.
Created attachment 209058 [details] Fix
Attachment 209058 [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/debugger/DebuggerActivation.cpp', u'Source/JavaScriptCore/runtime/Arguments.cpp', u'Source/JavaScriptCore/runtime/ArrayConstructor.cpp', u'Source/JavaScriptCore/runtime/ArrayPrototype.cpp', u'Source/JavaScriptCore/runtime/BooleanPrototype.cpp', u'Source/JavaScriptCore/runtime/DateConstructor.cpp', u'Source/JavaScriptCore/runtime/DatePrototype.cpp', u'Source/JavaScriptCore/runtime/ErrorPrototype.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/JSCell.cpp', u'Source/JavaScriptCore/runtime/JSDataView.cpp', u'Source/JavaScriptCore/runtime/JSDataViewPrototype.cpp', u'Source/JavaScriptCore/runtime/JSFunction.cpp', u'Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h', u'Source/JavaScriptCore/runtime/JSNotAnObject.cpp', u'Source/JavaScriptCore/runtime/JSONObject.cpp', u'Source/JavaScriptCore/runtime/JSObject.cpp', u'Source/JavaScriptCore/runtime/NamePrototype.cpp', u'Source/JavaScriptCore/runtime/NumberConstructor.cpp', u'Source/JavaScriptCore/runtime/NumberPrototype.cpp', u'Source/JavaScriptCore/runtime/ObjectConstructor.cpp', u'Source/JavaScriptCore/runtime/PropertyDescriptor.h', u'Source/JavaScriptCore/runtime/PropertySlot.h', u'Source/JavaScriptCore/runtime/RegExpConstructor.cpp', u'Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp', u'Source/JavaScriptCore/runtime/RegExpMatchesArray.h', u'Source/JavaScriptCore/runtime/RegExpObject.cpp', u'Source/JavaScriptCore/runtime/RegExpPrototype.cpp', u'Source/JavaScriptCore/runtime/StringConstructor.cpp', u'Source/JavaScriptCore/runtime/StringObject.cpp', u'Source/WebCore/ChangeLog', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/bindings/scripts/test/JS/JSTestActiveDOMObject.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestCustomNamedGetter.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestEventConstructor.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestEventTarget.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestException.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestInterface.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestMediaQueryListListener.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestNamedConstructor.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestNode.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestOverloadedConstructors.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestSerializedScriptValueInterface.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestTypedefs.cpp']" exit_code: 1 Source/JavaScriptCore/runtime/PropertyDescriptor.h:96: Else clause should never be on same line as else (use 2 lines) [whitespace/newline] [4] Source/JavaScriptCore/runtime/RegExpMatchesArray.h:87: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/RegExpMatchesArray.h:87: The parameter name "exec" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/RegExpMatchesArray.h:87: The parameter name "propertyName" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/RegExpMatchesArray.h:87: The parameter name "descriptor" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 5 in 50 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 209058 [details] Fix View in context: https://bugs.webkit.org/attachment.cgi?id=209058&action=review What remains? > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1902 > + push(@implContent, "GET_OWN_PROPERTY_DESCRIPTOR_IMPL(${className})\n"); I think an extra \n at the end would be be nicer.
Comment on attachment 209058 [details] Fix Attachment 209058 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1511487 New failing tests: http/tests/security/xss-DENIED-defineProperty.html
Created attachment 209077 [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 209118 [details] Updated patch with JSLocation fix Previously we blocked cross frame defineProperty by making GOPD hide the property altogether. Now allow it to reflect the presence of the property, just force the attributes to make it read-only, non-configurable.
(In reply to comment #3) > (From update of attachment 209058 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=209058&action=review > > What remains? Updated patch with bug fix, and a few more cases covered. It's pretty much just the window object left. > > Source/WebCore/bindings/scripts/CodeGeneratorJS.pm:1902 > > + push(@implContent, "GET_OWN_PROPERTY_DESCRIPTOR_IMPL(${className})\n"); > > I think an extra \n at the end would be be nicer. Done.
Attachment 209118 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/http/tests/security/xss-DENIED-defineProperty-expected.txt', u'Source/JavaScriptCore/API/JSCallbackObjectFunctions.h', u'Source/JavaScriptCore/ChangeLog', u'Source/JavaScriptCore/debugger/DebuggerActivation.cpp', u'Source/JavaScriptCore/runtime/Arguments.cpp', u'Source/JavaScriptCore/runtime/ArrayConstructor.cpp', u'Source/JavaScriptCore/runtime/ArrayPrototype.cpp', u'Source/JavaScriptCore/runtime/BooleanPrototype.cpp', u'Source/JavaScriptCore/runtime/DateConstructor.cpp', u'Source/JavaScriptCore/runtime/DatePrototype.cpp', u'Source/JavaScriptCore/runtime/ErrorPrototype.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/JSCell.cpp', u'Source/JavaScriptCore/runtime/JSDataView.cpp', u'Source/JavaScriptCore/runtime/JSDataViewPrototype.cpp', u'Source/JavaScriptCore/runtime/JSFunction.cpp', u'Source/JavaScriptCore/runtime/JSGenericTypedArrayViewInlines.h', u'Source/JavaScriptCore/runtime/JSNotAnObject.cpp', u'Source/JavaScriptCore/runtime/JSONObject.cpp', u'Source/JavaScriptCore/runtime/JSObject.cpp', u'Source/JavaScriptCore/runtime/NamePrototype.cpp', u'Source/JavaScriptCore/runtime/NumberConstructor.cpp', u'Source/JavaScriptCore/runtime/NumberPrototype.cpp', u'Source/JavaScriptCore/runtime/ObjectConstructor.cpp', u'Source/JavaScriptCore/runtime/PropertyDescriptor.h', u'Source/JavaScriptCore/runtime/PropertySlot.h', u'Source/JavaScriptCore/runtime/RegExpConstructor.cpp', u'Source/JavaScriptCore/runtime/RegExpMatchesArray.cpp', u'Source/JavaScriptCore/runtime/RegExpMatchesArray.h', u'Source/JavaScriptCore/runtime/RegExpObject.cpp', u'Source/JavaScriptCore/runtime/RegExpPrototype.cpp', u'Source/JavaScriptCore/runtime/StringConstructor.cpp', u'Source/JavaScriptCore/runtime/StringObject.cpp', u'Source/WebCore/ChangeLog', u'Source/WebCore/bindings/js/JSCSSStyleDeclarationCustom.cpp', u'Source/WebCore/bindings/js/JSHTMLAppletElementCustom.cpp', u'Source/WebCore/bindings/js/JSHTMLEmbedElementCustom.cpp', u'Source/WebCore/bindings/js/JSHTMLObjectElementCustom.cpp', u'Source/WebCore/bindings/js/JSHistoryCustom.cpp', u'Source/WebCore/bindings/js/JSLocationCustom.cpp', u'Source/WebCore/bindings/js/JSWorkerGlobalScopeCustom.cpp', u'Source/WebCore/bindings/scripts/CodeGeneratorJS.pm', u'Source/WebCore/bindings/scripts/test/JS/JSTestActiveDOMObject.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestCustomNamedGetter.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestEventConstructor.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestEventTarget.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestException.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestInterface.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestMediaQueryListListener.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestNamedConstructor.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestNode.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestObj.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestOverloadedConstructors.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestSerializedScriptValueInterface.cpp', u'Source/WebCore/bindings/scripts/test/JS/JSTestTypedefs.cpp', u'Source/WebCore/bridge/objc/objc_runtime.mm', u'Source/WebCore/bridge/runtime_array.cpp', u'Source/WebCore/bridge/runtime_method.cpp', u'Source/WebCore/bridge/runtime_object.cpp']" exit_code: 1 Source/JavaScriptCore/runtime/PropertyDescriptor.h:96: Else clause should never be on same line as else (use 2 lines) [whitespace/newline] [4] Source/JavaScriptCore/runtime/RegExpMatchesArray.h:87: The parameter name "object" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/RegExpMatchesArray.h:87: The parameter name "exec" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/RegExpMatchesArray.h:87: The parameter name "propertyName" adds no information, so it should be removed. [readability/parameter_name] [5] Source/JavaScriptCore/runtime/RegExpMatchesArray.h:87: The parameter name "descriptor" adds no information, so it should be removed. [readability/parameter_name] [5] Total errors found: 5 in 57 files If any of these errors are false positives, please file a bug against check-webkit-style.
Committed revision 154300.
Committed r154303: <http://trac.webkit.org/changeset/154303>
(In reply to comment #10) > Committed r154303: <http://trac.webkit.org/changeset/154303> Don't know how, but "webkit-patch land" thought that my change to https://bugs.webkit.org/show_bug.cgi?id=120015 was for this bug!
It seems something is wrong with the Qt port, because all dumpastext test started to "fail" after this change. The tests pass, but they generate rendertree dump instead of text only output incorrectly.
It seems like this patch caused http/tests/security/cross-frame-access-getOwnPropertyDescriptor.html to fail or crash: http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=http%2Ftests%2Fsecurity%2Fcross-frame-access-getOwnPropertyDescriptor.html
(In reply to comment #12) > It seems something is wrong with the Qt port, because all dumpastext test > started to "fail" after this change. The tests pass, but they generate > rendertree dump instead of text only output incorrectly. I think the but is somewhere in Qt implementation of DumpRenderTree::initJSObjects() . cc-ing Simon who touched this function last time - r145708 / bug112144
(In reply to comment #14) > (In reply to comment #12) > > It seems something is wrong with the Qt port, because all dumpastext test > > started to "fail" after this change. The tests pass, but they generate > > rendertree dump instead of text only output incorrectly. > > I think the but is somewhere in Qt implementation of > DumpRenderTree::initJSObjects() . > > cc-ing Simon who touched this function last time - r145708 / bug112144 Also it appears the tests are flaky, in that the only the first dumpAsText works. For instance: Tools/Scripts/new-run-webkit-tests --qt --iterations 10 http/tests/appcache/404-resource.html [2/10] http/tests/appcache/404-resource.html failed unexpectedly (text diff) [3/10] http/tests/appcache/404-resource.html failed unexpectedly (text diff) [4/10] http/tests/appcache/404-resource.html failed unexpectedly (text diff) [5/10] http/tests/appcache/404-resource.html failed unexpectedly (text diff) [6/10] http/tests/appcache/404-resource.html failed unexpectedly (text diff) [7/10] http/tests/appcache/404-resource.html failed unexpectedly (text diff) [8/10] http/tests/appcache/404-resource.html failed unexpectedly (text diff) [9/10] http/tests/appcache/404-resource.html failed unexpectedly (text diff) [10/10] http/tests/appcache/404-resource.html failed unexpectedly (text diff)
When a patch for bug A caused a regression bug B, B depends on A. i.e. A blocks B.
(In reply to comment #16) > When a patch for bug A caused a regression bug B, B depends on A. i.e. A blocks B. I don't agree. When a patch for bug A caused a regression bug B, it means that bug A isn't completely and properly fixed until bug B is fixed. So fixing bug A (original) depends on bug B (regression bug) == bug B blocks bug A. We always do it in this way. (irc bot does it for rollouts too) some historical example: - https://bugs.webkit.org/show_bug.cgi?id=119961 - https://bugs.webkit.org/show_bug.cgi?id=120023 But unfortunately in 95% of the cases there aren't any blocks/depends on relation added by folks only a comment in the original bug with the URL of the regression bug. But many times I can't find neither blocks/depends, nor comments. :(