This is a followup to bug 21288, which concerned the implementation of the HTML5 iframe sandbox attribute. I'm curious whether it would be possible to use more defensive initial values for the sandbox flags than the current default of "SandboxNone" (indicating no sandboxing at all). This defensive technique is, to me, primarily about finding current bugs and preventing future ones. It is a design challenge rather than a functional bug, so I don't have a test case to share. Three classes contain such sandbox flags: HTMLFrameOwnerElement, FrameLoader and SecurityOrigin. I imagine three steps: 1. On object instantiation, the sandbox flags are set to a defensive default value (SandboxAll, possibly including a special flag indicating an illegal value). 2. At some point between 1 and 3, we know of a better value for the sandbox flags, and assign that value to the flags. 3. We now start making decisions based on the sandbox flags. To ensure that step 2 actually happened, we could ASSERT that the flag for an illegal value is not set. The design challenge is to ensure step 2 always happens before step 3. It was straight-forward for the FrameLoader (that part was included in the landed patch for bug 21288), but not for HTMLFrameOwnerElement and SecurityOrigin. I wrote down some observations on this in the thread for bug 21288, comment #58: https://bugs.webkit.org/show_bug.cgi?id=21288#c58
I think for HTMLFrameOwnerElement this issue probably doesn't apply. Using the DOM you can create a frame element a step at a time, and you can start using it at any point, so there is no real "illegal value" time for the DOM elements. For SecurityOrigin I think we might be able to make it happen, though.
Created attachment 46230 [details] Work in progress
Created attachment 46242 [details] Patch
Comment on attachment 46242 [details] Patch > if (shouldTreatURLSchemeAsNoAccess(m_protocol)) > m_isUnique = true; > > - // If this ASSERT becomes false in the future, please consider the impact > - // of m_sandoxFlags on m_isUnique. > - ASSERT(m_sandboxFlags == SandboxNone); > + if (isSandboxed(SandboxOrigin)) > + m_isUnique = true; I suggest initializing m_unique to the correct value from the outset. m_isUnique(shouldTreatURLSchemeAsNoAccess(m_protocol) || isSandboxed(SandboxOrigin)) To some the if statements may be slightly clearer, so it seems OK to leave it this way if you think that way. > + static PassRefPtr<SecurityOrigin> create(const KURL&, SandboxFlags sandboxFlags = SandboxNone); The argument name isn't needed here because the type speaks for itself.
> > + static PassRefPtr<SecurityOrigin> create(const KURL&, SandboxFlags sandboxFlags = SandboxNone); > > The argument name isn't needed here because the type speaks for itself. Oh, I didn't know you could omit the argument name and provide a default value. Thanks for the review.
Comments addressed and landed in http://trac.webkit.org/changeset/53056