Bug 24755

Summary:

LayoutTests/http/tests/misc/url-in-utf16le.html regression

Product:

WebKit

Reporter:

Nico Weber <nicolasweber>

Component:

New Bugs

Assignee:

Darin Adler <darin>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ddkilzer, koivisto, nicolasweber

Priority:

Version:

528+ (Nightly build)

Hardware:

Mac

OS:

OS X 10.5

Attachments:

Description	Flags
Patch.	none
Patch with better ChangeLog.	none
Patch with better changelog and no tabs.	darin: review-
patch that fixes regression tests without reintroducing the performance problem	koivisto: review+

Nico Weber

Reported 2009-03-22 22:00:12 PDT

WebKit change 41727 regressed query param handling for utf16 files. These fail: WebKitTools/Scripts/run-webkit-tests --debug LayoutTests/http/tests/misc/url-in-utf16le.html WebKitTools/Scripts/run-webkit-tests --debug LayoutTests/http/tests/misc/url-in-utf16be.html

Attachments
Patch. (1.25 KB, patch) 2009-03-22 22:04 PDT, Nico Weber	no flags	Details Formatted Diff Diff
Patch with better ChangeLog. (1.32 KB, patch) 2009-03-22 22:07 PDT, Nico Weber	no flags	Details Formatted Diff Diff
Patch with better changelog and no tabs. (1.32 KB, patch) 2009-03-22 22:09 PDT, Nico Weber	darin: review-	Details Formatted Diff Diff
patch that fixes regression tests without reintroducing the performance problem (4.58 KB, patch) 2009-03-23 09:17 PDT, Darin Adler	koivisto: review+	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Nico Weber

Comment 1 2009-03-22 22:04:48 PDT

Created attachment 28845 [details] Patch. Revert change 41727. Also fixes http://crbug.com/9067 .

Nico Weber

Comment 2 2009-03-22 22:05:33 PDT

With this patch, WebKitTools/Scripts/run-webkit-tests --debug LayoutTests/http/tests/misc/url-in-utf16le.html works fine again.

Nico Weber

Comment 3 2009-03-22 22:07:48 PDT

Created attachment 28846 [details] Patch with better ChangeLog.

Nico Weber

Comment 4 2009-03-22 22:09:44 PDT

Created attachment 28847 [details] Patch with better changelog and no tabs. Sorry for the noise. It's my first time.

Nico Weber

Comment 5 2009-03-22 22:16:16 PDT

Some comments on the change: If `this` is an UTF16-encoding, change 41727 caused wrong results. Since `isNonByteBasedEncoding()` checks `noExtendedTextEncodingNameUsed()` already, and boolean operators are lazy, reverting 41727 does not cause any harm but fixes the regression.

David Kilzer (:ddkilzer)

Comment 6 2009-03-23 09:08:39 PDT

http://trac.webkit.org/changeset/41727

Darin Adler

Comment 7 2009-03-23 09:08:48 PDT

Comment on attachment 28847 [details] Patch with better changelog and no tabs. We can't just roll out this optimization, which was fixing a significant performance problem. We need a new fix rather than just rolling out the bad one. Let me take a look at this.

Darin Adler

Comment 8 2009-03-23 09:10:34 PDT

Comment on attachment 28847 [details] Patch with better changelog and no tabs. I'm about to post an improved patch for this.

Darin Adler

Comment 9 2009-03-23 09:17:12 PDT

Created attachment 28854 [details] patch that fixes regression tests without reintroducing the performance problem

Antti Koivisto

Comment 10 2009-03-23 11:12:55 PDT

r=me

Darin Adler

Comment 11 2009-03-23 16:50:28 PDT

http://trac.webkit.org/changeset/41916

Note You need to log in before you can comment on or make changes to this bug.