Bug 113780 – Extract URL that doesn't inherit a parent's SecurityOrigin out into a constant.

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Mike West]

We're scheduling navigations to 'data:text/html,' in two places in order to end up on a page that doesn't inherit its parent's SecurityOrigin. We should be more explicit about what we're doing.

[Mike West]

Created attachment 196133 [details]
Patch

[Mike West]

Created attachment 196134 [details]
Patch

[Mike West]

Mind taking a look, Jochen? Should be pretty much what we discussed (I went with the method, as I didn't see too many occurrences of a bare static string in the codebase).

Comment on attachment 196134 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=196134&action=review

> Source/WebCore/page/SecurityOrigin.cpp:597
> +    DEFINE_STATIC_LOCAL(const String, uniqueSecurityOriginURL, (ASCIILiteral("data:text/html,")));

is the mime type even important at that point?

[Mike West]

(In reply to comment #4)
> (From update of attachment 196134 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=196134&action=review
> 
> > Source/WebCore/page/SecurityOrigin.cpp:597
> > +    DEFINE_STATIC_LOCAL(const String, uniqueSecurityOriginURL, (ASCIILiteral("data:text/html,")));
> 
> is the mime type even important at that point?

No idea. I'm not sure what WebKit does with a blank MIME type. I suppose I can test it. I'll do that before landing.

[Darin Adler]

Comment on attachment 196134 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=196134&action=review

> Source/WebCore/page/SecurityOrigin.h:213
> +    static const String urlWithUniqueSecurityOrigin();

The const here is not needed or helpful. The return type should be just String.

[Alexey Proskuryakov]

Comment on attachment 196134 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=196134&action=review

> Source/WebCore/page/SecurityOrigin.cpp:595
> +const String SecurityOrigin::urlWithUniqueSecurityOrigin()

Can we assert isMainThread() here?

[Mike West]

Thanks, Alexey and Darin. I'll land this shortly with your tweaks.

[Mike West]

Created attachment 196292 [details]
Patch

[Mike West]

Comment on attachment 196292 [details]
Patch

The x-frame-options bug got reverted; adjusting this patch to drop that bit. I'll fold it into the other patch once this lands.

[WebKit Review Bot]

Comment on attachment 196292 [details]
Patch

Clearing flags on attachment: 196292

Committed r147526: <http://trac.webkit.org/changeset/147526>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.