[V8] Fix issues with [DoNotCheckSecurity] which depended on non ES5 compliant behavior
Created attachment 157341 [details] Patch
Comment on attachment 157341 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=157341&action=review > Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:800 > + if (!BindingSecurity::shouldAllowAccessToFrame(BindingState::instance(), imp->frame(), ReportSecurityError)) ReportSecurityError is the default. You don't need to supply it explicitly.
Comment on attachment 157341 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=157341&action=review > Source/WebCore/ChangeLog:13 > + No new tests. No change in behavior. Does this fix tests that fail when the ES5 flag is flipped?
Comment on attachment 157341 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=157341&action=review > Source/WebCore/bindings/scripts/CodeGeneratorV8.pm:777 > + v8::Local<v8::Value> hiddenValue = info.This()->GetHiddenValue(name); GetHiddenValue() and SetHiddenValue() are slow. Isn't there any performance concern? If this code would not be a bottleneck in practical world, that's fine.
(In reply to comment #4) > GetHiddenValue() and SetHiddenValue() are slow. Isn't there any performance concern? If this code would not be a bottleneck in practical world, that's fine. They are slow compared to doing nothing but they are not slow slow. I don't have a better idea how to fix this.
(In reply to comment #3) > (From update of attachment 157341 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=157341&action=review > > > Source/WebCore/ChangeLog:13 > > + No new tests. No change in behavior. > > Does this fix tests that fail when the ES5 flag is flipped? Yes. That is the point of this patch. I can add that to the ChangeLog.
Ok. LGTM, but I'd like Kentaro to do the official review, if he's willing.
(In reply to comment #5) > They are slow compared to doing nothing but they are not slow slow. I don't have a better idea how to fix this. I think, for correctness, the slowdown is acceptable if the slowdown is not in hot call paths in practical. Would you confirm that this patch doesn't regress performance of PerformanceTests/Dromaeo/ and PerformanceTests/Bindings/?
Created attachment 157541 [details] Perf comparison
The perf results show no regression as far as I can tell. Some tests are faster, some are slower. The results are just really flaky so it is hard to tell.
Comment on attachment 157341 [details] Patch Thanks. LGTM.
Created attachment 157590 [details] Patch for landing
Comment on attachment 157590 [details] Patch for landing Clearing flags on attachment: 157590 Committed r125232: <http://trac.webkit.org/changeset/125232>
All reviewed patches have been landed. Closing bug.