Bug 25290 – REGRESSION(r41732): Crash when constructing XMLHttpRequest in a detached document

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore JavaScript

[Alexey Proskuryakov]

Now that JSXMLHttpRequestConstructor (and other constructors) are tied to a global object, not to a script execution context, we need to take care of edge cases where converting from ScriptExecutionContext to JSDOMGlobalObject doesn't work. Namely, both exist for a detached document, but toJSDOMGlobalObject(scriptExecutionContext) returns 0.

Patch forthcoming.

[Alexey Proskuryakov]

Created attachment 29621 [details]
proposed patch

[Darin Adler]

Comment on attachment 29621 [details]
proposed patch

> +    KURL url = this->url();
> +    NSURL* nsURL;
> +    if (m_responseContentDispositionEncodingFallbackArray.isEmpty())
> +        nsURL = url;
> +    else {
> +        CString urlString = TextEncoding(m_responseContentDispositionEncodingFallbackArray[0]).encode(url.string().characters(), url.string().length(), URLEncodedEntitiesForUnencodables);
> +        RetainPtr<CFURLRef> cfURL = CFURLCreateAbsoluteURLWithBytes(kCFAllocatorDefault, (const UInt8*)urlString.data(), urlString.length(), CFStringConvertIANACharSetNameToEncoding(m_responseContentDispositionEncodingFallbackArray[0].createCFString()), 0, false);
> +        nsURL = (NSURL*)cfURL.get();
> +        [[nsURL retain] autorelease];;
> +    }
> +
>      if (nsRequest)
> -        [nsRequest setURL:url()];
> +        [nsRequest setURL:nsURL];
>      else
> -        nsRequest = [[NSMutableURLRequest alloc] initWithURL:url()];
> +        nsRequest = [[NSMutableURLRequest alloc] initWithURL:nsURL];

I suggest making the new nsURL variable a RetainPtr<NSURL*>, and therefore avoiding autorelease.

Is there a way we could avoid 

There are double semicolons on the line that calls autorelease.

A local variable for the encoding name would make the expression easier to read.

This code leaks the string created by createCFString; probably best to fix that using a RetainPtr<CFStringRef>.

review- because of the leak

[Alexey Proskuryakov]

Created attachment 29624 [details]
proposed patch

> I suggest making the new nsURL variable a RetainPtr<NSURL*>, and therefore
> avoiding autorelease.

Oops! Sorry, this code is completely unrelated, and certainly not ready for review. Submitting a hopefully clean patch...

[Darin Adler]

Comment on attachment 29624 [details]
proposed patch

r=me

[Alexey Proskuryakov]

Committed <http://trac.webkit.org/changeset/42700>.