Bug 197693

Created attachment 369392 [details] Patch

Comment on attachment 369392 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=369392&action=review This looks identical to a bug Michael was looking into that caused other issues. Might be worth verifying with what the other issues were. > Source/JavaScriptCore/runtime/JSObject.cpp:3448 > + ASSERT(!scope.exception() || !result); EXCEPTION_ASSERT instead

Comment on attachment 369392 [details] Patch Attachment 369392 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/12134379 New failing tests: http/tests/security/cross-frame-access-getOwnPropertyDescriptor.html

Created attachment 369399 [details] Archive of layout-test-results from ews114 for mac-highsierra The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-highsierra Platform: Mac OS X 10.13.6

Comment on attachment 369392 [details] Patch Attachment 369392 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/12135051 New failing tests: http/tests/css/filters-on-iframes.html

Created attachment 369407 [details] Archive of layout-test-results from ews212 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews212 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

Created attachment 369421 [details] Patch

(In reply to Saam Barati from comment #2) > Comment on attachment 369392 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=369392&action=review > > This looks identical to a bug Michael was looking into that caused other > issues. Might be worth verifying with what the other issues were. Yep, looks like the same issue and the exact same fix, which explains the test failures on my patch. I think it should be correct now.

Comment on attachment 369421 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=369421&action=review > Source/JavaScriptCore/runtime/JSObject.cpp:3447 > + bool result = methodTable(vm)->getOwnPropertySlot(this, exec, propertyName, slot); Our policy was if this throws, it must return false. Is that no longer true? Who broke that? We can remove a branch if this is the case.

Comment on attachment 369421 [details] Patch Attachment 369421 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/12138241 New failing tests: svg/repaint/remove-border-property-on-root.html

Created attachment 369450 [details] Archive of layout-test-results from ews213 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews213 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

(In reply to Saam Barati from comment #9) > Comment on attachment 369421 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=369421&action=review > > > Source/JavaScriptCore/runtime/JSObject.cpp:3447 > > + bool result = methodTable(vm)->getOwnPropertySlot(this, exec, propertyName, slot); > > Our policy was if this throws, it must return false. Is that no longer true? > Who broke that? We can remove a branch if this is the case. That's no longer true. JSLocation::getOwnPropertySlot returns true in case of exception. I guess it would be better to just fix that instead? I'll update the patch.

(In reply to Tadeu Zagallo from comment #12) > (In reply to Saam Barati from comment #9) > > Comment on attachment 369421 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=369421&action=review > > > > > Source/JavaScriptCore/runtime/JSObject.cpp:3447 > > > + bool result = methodTable(vm)->getOwnPropertySlot(this, exec, propertyName, slot); > > > > Our policy was if this throws, it must return false. Is that no longer true? > > Who broke that? We can remove a branch if this is the case. > > That's no longer true. JSLocation::getOwnPropertySlot returns true in case > of exception. I guess it would be better to just fix that instead? I'll > update the patch. I think that would be better.

Created attachment 369719 [details] Patch

Comment on attachment 369719 [details] Patch r=me

Comment on attachment 369719 [details] Patch Clearing flags on attachment: 369719 Committed r245249: <https://trac.webkit.org/changeset/245249>

All reviewed patches have been landed. Closing bug.

After changes in https://trac.webkit.org/changeset/245249 We are seeing 32 failures on the Debug JSC queue ** The following JSC stress test failures have been introduced: stress/proxy-delete.js.bytecode-cache stress/proxy-delete.js.default stress/proxy-delete.js.dfg-eager stress/proxy-delete.js.dfg-eager-no-cjit-validate stress/proxy-delete.js.dfg-maximal-flush-validate-no-cjit stress/proxy-delete.js.ftl-eager stress/proxy-delete.js.ftl-eager-no-cjit stress/proxy-delete.js.ftl-eager-no-cjit-b3o1 stress/proxy-delete.js.ftl-no-cjit-b3o0 stress/proxy-delete.js.ftl-no-cjit-no-inline-validate stress/proxy-delete.js.ftl-no-cjit-no-put-stack-validate stress/proxy-delete.js.ftl-no-cjit-small-pool stress/proxy-delete.js.ftl-no-cjit-validate-sampling-profiler stress/proxy-delete.js.no-cjit-validate-phases stress/proxy-delete.js.no-ftl stress/proxy-delete.js.no-llint stress/proxy-property-descriptor.js.bytecode-cache stress/proxy-property-descriptor.js.default stress/proxy-property-descriptor.js.dfg-eager stress/proxy-property-descriptor.js.dfg-eager-no-cjit-validate stress/proxy-property-descriptor.js.dfg-maximal-flush-validate-no-cjit stress/proxy-property-descriptor.js.ftl-eager stress/proxy-property-descriptor.js.ftl-eager-no-cjit stress/proxy-property-descriptor.js.ftl-eager-no-cjit-b3o1 stress/proxy-property-descriptor.js.ftl-no-cjit-b3o0 stress/proxy-property-descriptor.js.ftl-no-cjit-no-inline-validate stress/proxy-property-descriptor.js.ftl-no-cjit-no-put-stack-validate stress/proxy-property-descriptor.js.ftl-no-cjit-small-pool stress/proxy-property-descriptor.js.ftl-no-cjit-validate-sampling-profiler stress/proxy-property-descriptor.js.no-cjit-validate-phases stress/proxy-property-descriptor.js.no-ftl stress/proxy-property-descriptor.js.no-llint https://build.webkit.org/builders/Apple%20High%20Sierra%20Debug%20JSC%20%28Tests%29/builds/2788/steps/jscore-test/logs/stdio

*** Bug 197485 has been marked as a duplicate of this bug. ***