Bug 25290 - REGRESSION(r41732): Crash when constructing XMLHttpRequest in a detached document
Summary: REGRESSION(r41732): Crash when constructing XMLHttpRequest in a detached docu...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P1 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords: Regression
Depends on:
Blocks:
 
Reported: 2009-04-20 01:51 PDT by Alexey Proskuryakov
Modified: 2009-04-21 01:26 PDT (History)
0 users

See Also:


Attachments
proposed patch (15.86 KB, patch)
2009-04-20 12:58 PDT, Alexey Proskuryakov
darin: review-
Details | Formatted Diff | Diff
proposed patch (14.29 KB, patch)
2009-04-20 14:05 PDT, Alexey Proskuryakov
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2009-04-20 01:51:09 PDT
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.
Comment 1 Alexey Proskuryakov 2009-04-20 12:58:17 PDT
Created attachment 29621 [details]
proposed patch
Comment 2 Darin Adler 2009-04-20 13:55:36 PDT
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
Comment 3 Alexey Proskuryakov 2009-04-20 14:05:21 PDT
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...
Comment 4 Darin Adler 2009-04-20 14:18:44 PDT
Comment on attachment 29624 [details]
proposed patch

r=me
Comment 5 Alexey Proskuryakov 2009-04-21 01:26:32 PDT
Committed <http://trac.webkit.org/changeset/42700>.