Bug 113003

Summary: Remove 2 bad branches from StringHash::equal() and CaseFoldingHash::equal()
Product: WebKit Reporter: Benjamin Poulain <benjamin>
Component: New BugsAssignee: Benjamin Poulain <benjamin>
Status: RESOLVED FIXED    
Severity: Normal CC: cmarcelo, eric, esprehn+autocc, ojan.autocc, rniwa, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch eric: review+

Benjamin Poulain
Reported 2013-03-21 22:12:42 PDT
Remove 2 bad branches from StringHash::equal() and CaseFoldingHash::equal()
Attachments
Patch (10.46 KB, patch)
2013-03-21 22:35 PDT, Benjamin Poulain 		no flags
DetailsFormatted DiffDiff
Patch (12.53 KB, patch)
2013-03-22 15:30 PDT, Benjamin Poulain 		eric: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Benjamin Poulain
Comment 1 2013-03-21 22:35:27 PDT
Created attachment 194443 [details] Patch
Benjamin Poulain
Comment 2 2013-03-22 15:30:57 PDT
Created attachment 194645 [details] Patch
Benjamin Poulain
Comment 3 2013-03-22 15:33:37 PDT
*** Bug 111892 has been marked as a duplicate of this bug. ***
Eric Seidel (no email)
Comment 4 2013-03-22 16:26:00 PDT
Comment on attachment 194645 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=194645&action=review LGTM! > Source/WebCore/html/parser/HTMLTreeBuilderSimulator.cpp:98 > + return equalIgnoringCase(token.data().asStringImpl(), SVGNames::foreignObjectTag.localName().impl()); These are known to never be null, btw.
Benjamin Poulain
Comment 5 2013-03-22 16:30:20 PDT
> > Source/WebCore/html/parser/HTMLTreeBuilderSimulator.cpp:98 > > + return equalIgnoringCase(token.data().asStringImpl(), SVGNames::foreignObjectTag.localName().impl()); > > These are known to never be null, btw. I was not sure so I took the safe option :). I will change to the non-null form before landing. Thank you for the review.
Eric Seidel (no email)
Comment 6 2013-03-22 16:31:58 PDT
I believe the HTMLParser basically never has any null strings ever. :) Everything is empty at most. In the case of a tag name, the parser will tread <> as a characters in a character token, not as a tag with empty name. Thanks for fixing!
Benjamin Poulain
Comment 7 2013-03-22 18:38:08 PDT
Committed r146702: <http://trac.webkit.org/changeset/146702>
Ryosuke Niwa
Comment 8 2013-03-22 19:44:13 PDT
This patch broke Windows build: http://build.webkit.org/builders/Apple%20Win%20Release%20%28Build%29/builds/46465/steps/compile-webkit/logs/stdio 7>WebKitSystemInterface.lib(WebKitSystemInterface.obj) : MSIL .netmodule or module compiled with /GL found; restarting link with /LTCG; add /LTCG to the link command line to improve linker performance 7> Creating library C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\lib\WebKit.lib and object C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\lib\WebKit.exp 7>WebKit.exp : error LNK2001: unresolved external symbol "bool __cdecl WTF::equalIgnoringCase(class WTF::StringImpl *,unsigned char const *)" (?equalIgnoringCase@WTF@@YA_NPAVStringImpl@1@PBE@Z) 7>C:\cygwin\home\buildbot\slave\win-release\build\WebKitBuild\Release\bin\WebKit.dll : fatal error LNK1120: 1 unresolved externals
Benjamin Poulain
Comment 9 2013-03-22 20:32:57 PDT
(In reply to comment #8) > This patch broke Windows build: I am on it.
Benjamin Poulain
Comment 11 2013-03-22 20:39:02 PDT
(In reply to comment #10) > Fixed in http://trac.webkit.org/changeset/146705 and http://trac.webkit.org/changeset/146710. Thanks for fixing this.
Note You need to log in before you can comment on or make changes to this bug.