|Product:||WebKit||Reporter:||Nate Chapin <japhet>|
|Component:||WebCore Misc.||Assignee:||Nate Chapin <japhet>|
|Version:||528+ (Nightly build)|
|OS:||OS X 10.5|
|Bug Depends on:|
Description Nate Chapin 2010-01-04 15:10:06 PST
Ideally, these security checks would be in some common part of the custom bindings that all descendants of V8Element had access to. Unfortunately, there doesn't seem to be such a place, and V8AttrCustom.cpp needs to have access to these checks as well. Therefore, these should probably go in V8BindingSecurity (which will need to be expanded to something more than a typedef).
Comment 2 WebKit Review Bot 2010-01-04 15:52:55 PST
Attachment 45842 [details] did not pass style-queue: Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1 WebCore/ChangeLog:7: Line contains tab character. [whitespace/tab]  WebCore/bindings/v8/V8BindingSecurity.h:68: Could not find a newline character at the end of the file. [whitespace/ending_newline]  Total errors found: 2
Comment 3 Nate Chapin 2010-01-04 15:58:47 PST
Created attachment 45843 [details] patch minus style errors
Comment 4 WebKit Review Bot 2010-01-04 16:03:22 PST
style-queue ran check-webkit-style on attachment 45843 [details] without any errors.
Comment 5 Adam Barth 2010-01-04 17:27:28 PST
Comment on attachment 45843 [details] patch minus style errors Sorry, I think i steered you slightly wrong here. This code can go in the generic BindingSecurity object since it's not dependent on V8: http://trac.webkit.org/browser/trunk/WebCore/bindings/BindingSecurity.h That way the code will be shared with JSC eventually.
Comment 6 Nate Chapin 2010-01-04 19:15:34 PST
(In reply to comment #5) > (From update of attachment 45843 [details]) > Sorry, I think i steered you slightly wrong here. This code can go in the > generic BindingSecurity object since it's not dependent on V8: > > http://trac.webkit.org/browser/trunk/WebCore/bindings/BindingSecurity.h > > That way the code will be shared with JSC eventually. That'll require a State<Binding> parameter then, right? Is that ok? I don't have a great idea of how the State is used.
Comment 7 Nate Chapin 2010-01-05 10:16:33 PST
Created attachment 45902 [details] Functions are now in BindingSecurity.h. I'm not sure whether these functions belong in BindingSecurity.h or whether we should add BindingSecurity.cpp, so I defaulted to not adding a new file.
Comment 8 WebKit Review Bot 2010-01-05 10:17:26 PST
style-queue ran check-webkit-style on attachment 45902 [details] without any errors.
Comment 9 Adam Barth 2010-01-05 11:01:26 PST
Comment on attachment 45902 [details] Functions are now in BindingSecurity.h. This looks great. Unfortunately, these functions need to be in the header because of the template parameter. I'm about to land a change that moves this file, so we're going to conflict. I can land this if you'd rather not deal with the merge conflict.