Bug 59900 – StringImpl::endsWith has some insane code

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Adam Barth]

StringImpl::endsWith has some insane (and wrong) code

[Adam Barth]

Created attachment 91855 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 91855 [details]
Patch

How do we test this?

[Adam Barth]

@dave_levin: Is the WTF unit testing framework ready for use?

[Sam Weinig]

(In reply to comment #3)
> @dave_levin: Is the WTF unit testing framework ready for use?

The existing one in Tools/TestWebKitAPI works.  It will be transitioned to GTest at somepoint, but that should not stop people from using it now.

[Adam Barth]

WebKitAPITest != TestWebKitAPI :)

[Sam Weinig]

(In reply to comment #5)
> WebKitAPITest != TestWebKitAPI :)

Adam Roben and I like to confuse and befuddle.

[Adam Barth]

Sadly, that test harness can't handing anything the depends on String because the include paths are all screwed up.

[Adam Barth]

Comment on attachment 91855 [details]
Patch

I'm just going to land this patch.  It doesn't seem worthwhile to teach this test harness to include these files if we're just going to change the harness soon.

[WebKit Commit Bot]

Comment on attachment 91855 [details]
Patch

Rejecting attachment 91855 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-7', 'build-..." exit_code: 2

Last 500 characters of output:
led
..
animations/change-keyframes-name.html -> failed
.
animations/change-keyframes.html -> failed
.
animations/change-one-anim.html -> failed
.
animations/combo-transform-rotate+scale.html -> failed
.
animations/combo-transform-translate+scale.html -> failed
..
animations/dynamic-stylesheet-loading.html -> failed

Exiting early after 10 failures. 111 tests run.
38.93s total testing time

101 test cases (90%) succeeded
10 test cases (9%) had incorrect layout
2 test cases (1%) had stderr output

Full output: http://queues.webkit.org/results/8529503

[WebKit Commit Bot]

Created attachment 91878 [details]
Archive of layout-test-results from cr-jail-7

The attached test failures were seen while running run-webkit-tests on the commit-queue.
Bot: cr-jail-7  Port: Mac  Platform: Mac OS X 10.6.7

[Alexey Proskuryakov]

Comment on attachment 91855 [details]
Patch

+        return (caseSensitive ? find(matchString, start) == start : findIgnoringCase(matchString, start)) == start;

This is not right.

[Adam Barth]

> This is not right.

Apparently.  :)

[Adam Barth]

Created attachment 91898 [details]
Patch

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 91898 [details]:

http/tests/xmlhttprequest/access-control-sandboxed-iframe-allow.html bug 59940
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 91898 [details]
Patch

Clearing flags on attachment: 91898

Committed r85482: <http://trac.webkit.org/changeset/85482>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.