Freeze sandbox attributes on creation
Created attachment 47451 [details] Patch
I thought there was a bug on this already, but I couldn't find it. Tests coming.
Created attachment 47993 [details] Patch
The test doesn't provide 100% coverage for this change, but it's better than nothing.
Comment on attachment 47993 [details] Patch Is it safe to have sandbox flags in the frame? Shouldn't we go further and remove them entirely? The changes in FrameLoader to call isSandboxed on the frame's document seem awkward. Wouldn't it be better to have a private isSandboxed function that did the right thing instead?
> Is it safe to have sandbox flags in the frame? Shouldn't we go further and > remove them entirely? I couldn't figure out how to read the allow-script bit out of the document yet, which is why there's a FIXME for that case. If I convert that call site to use the document's flags, we crash in Frame creation (for some reason the document is non-null and doesn't appear to be a fully initialized Document object). I suspect this is because of a tricky case during frame construction. My plan is to look at that call site in a second patch. Once we remove that, we can remove a bunch of code related to the flags on the frame. > The changes in FrameLoader to call isSandboxed on the frame's document seem > awkward. Wouldn't it be better to have a private isSandboxed function that did > the right thing instead? Sure.
Comment on attachment 47993 [details] Patch I think Darin meant to r- this patch. In any case, I need to address his feedback.
(In reply to comment #7) > I think Darin meant to r- this patch. In any case, I need to address his > feedback. For the record, I did r+ it on purpose. I reviewed it and also had some questions for followup. I don’t object, though, to you addressing more of those issues before landing something.
Ok. I'll address the remainder of your feedback and then land the patch. Thanks.
Committed r54587: <http://trac.webkit.org/changeset/54587>
Created attachment 51161 [details] Patch for landing
I seem to have forgotten to land the tests from this patch.
Comment on attachment 51161 [details] Patch for landing Clearing flags on attachment: 51161 Committed r56290: <http://trac.webkit.org/changeset/56290>
All reviewed patches have been landed. Closing bug.