Bug 7989 – Win32: User Agent shouldn't be hardcoded in TransferJobWin

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit API

[Justin Haygood]

Since Frame will return a userAgent, TransferJobWin should use that instead of hardcoding Spinneret. Fairly trivial fix, requires changes to TransferJobWin and Spinneret though.

Patches coming soon.

[Justin Haygood]

Created attachment 7302 [details]
WebCore part of fix... 

WebCore part of fix.

Requires Spinneret part if you don't want Spinneret to crash.. coming soon

[Justin Haygood]

Created attachment 7303 [details]
Spinneret part of fix

Spinneret part of fix.

Also contains a one liner fix to set the main frame of a page, which crashes Spinneret on alot of pages.

[Eric Seidel (no email)]

Comment on attachment 7303 [details]
Spinneret part of fix

This patch fails to follow style guidelines, has tabs, is missing a copyright header, and changelog.

It also looks like it leaks a DocLoader on every request.

Otherwise the code itself looks sane.

[Eric Seidel (no email)]

Comment on attachment 7302 [details]
WebCore part of fix... 

Ah!  So this is why maps.google doesn't work!

This still has tabs, and strange spacing between lines.

It's also missing your copyright and a changelog.

[Justin Haygood]

Created attachment 7335 [details]
Follows guidelines, adds copyright and changelog

[Justin Haygood]

Created attachment 7336 [details]
Spinneret part of fix.. updated to follow guidelines

Still contains the crasher fix.

[Eric Seidel (no email)]

Comment on attachment 7335 [details]
Follows guidelines, adds copyright and changelog

Looks good.  Still has tabs in changelog, but I'll fix those when I land.  r=me.

[Eric Seidel (no email)]

Comment on attachment 7336 [details]
Spinneret part of fix.. updated to follow guidelines

This is the wrong place for this code.

The WebView should be the one setting the main frame on the page.

Also the Document owns a doc loader, you shouldn't create one in Frame like that.  Also deleting the DocLoader right after calling start() will just cause things to crash.

[Darin Adler]

Comment on attachment 7335 [details]
Follows guidelines, adds copyright and changelog

This uses String::ascii, but this is what PlatformString.h says:

#ifndef NDEBUG
    // For debugging only, leaks memory.
    const char *ascii() const;
#endif

So this won't even compile when NDEBUG is set. We need a solution that doesn't involve the ascii() function.

A short term hack would be to use DeprecatedString's ascii() function. A better fix would be to add an appropriate conversion for String for this case; perhaps a way to convert to an ASCII string that doesn't leak memory or a way to convert to a Windows wchar_t* so we can use InternetOpen rather than InternetOpenA?

[Eric Seidel (no email)]

InternetOpenW is not implemented.  We learned that the hard way when originally writing this code.  so string.deprecatedString().ascii() is a fine option here.

[Eric Seidel (no email)]

I fixed this in a slightly different way in TOT.